[GISel/DAG] Getting a TableGen pattern to match both G_ADD and G_PTR_ADD in GISel (original) (raw)

August 12, 2022, 10:03am 1

Hello!

I am currently trying to make the following instruction selection pattern work on the G_PTR_ADD gMIR instruction:

class ThreeOp_i32_Pats <SDPatternOperator op1, SDPatternOperator op2, Instruction inst> : GCNPat <
  (ThreeOpFrag<op1, op2> i32:$src0, i32:$src1, i32:$src2),
  (inst VSrc_b32:$src0, VSrc_b32:$src1, VSrc_b32:$src2)
>;

def : ThreeOp_i32_Pats<add, add, V_ADD3_U32_e64>;

The goal is to fold instructions like G_PTR_ADD (G_PTR_ADD %ptr, %off1) %off2) into V_ADD3_U32_e64 %ptr, %off1, %off2.

If I understand correctly, in order to match the above pattern for both G_ADD and G_PTR_ADD, there are a couple of issues I need to overcome due to the fact that the pattern is written for the DAGISel, but is translated into GISel matching logic.

• Opcode matching: As far as I can see, there is no way to express a desire to match G_PTR_ADD. add maps to G_ADD and no DAG instruction currently matches G_PTR_ADD as far as I can see.
• Types: The DAG has no pointer type, GISel has pointer types. Moreover, the SDTypeProfile of add requires all types to be identical and to be integers, so we cannot have an add DAG node with types such as (p0, i32) -> p0 (which is what a G_PTR_ADD would look like).

My current attempt (⚙ D131254 [AMDGPU][GISel] Enable Selection of ADD3 for G_PTR_ADD) revolves around adding some fields to Pattern and SDNodeEquiv to enable the generated G_ADD pattern to match either opcode, and not do strict type matching but instead just check that the size of the type match (so i32 can match both i32 and a pointer for instance). It works, but it feels awfully hacky and has a few other bugs.

Before investing more effort into that diff, I would like to gather feedback on my current attempt at a fix, and on how such problems are usually dealt with:

• Is there a simple solution to rewrite the pattern in a way that’ll match both G_PTR_ADD and G_ADD, and that I completely missed?
  • I saw something like “wip_match_opcode” in other TableGen files, could it be used here?
• Is this a bigger problem with how GISel/DAGSel fit together, and the proper fix will not be trivial?
  • Then maybe the best course of action is a small, non-intrusive hack to make the pattern match? e.g. transform G_PTR_ADD into G_ADD (G_PTRTOINT) pre instruction selection in our backend so the pattern can match. What do you think?

Thank you very much for your feedback.