AMDGPU/GlobalISel: Add tablegen operator that looks through copies

[Petar.Avramovic]

Summary

Introduce g_maybe_cross_reg_bank_copy, operator that can be used in patterns to ignore cross register bank copies.
Operator will be recognized by global-isel emitter and ignored by sdag emitter (sdag uses types not register banks).
Generates GIM_CheckOpcodeIgnoreCopies instead of GIM_CheckOpcode.
Does not affect complexity of the pattern.

[foad]

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

396 ↗

(On Diff #286821)

Can't this still be inside the class? I.e. move it down two or three lines?

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2855–2856 ↗

(On Diff #286821)

Why not call AdjustInstrPostInstrSelectionBase on each MI in OutMIs?

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4535 ↗

(On Diff #286821)

Forcing a function to be shared that refers to an SDNode is also pretty awkward

4538 ↗

(On Diff #286821)

I was trying to avoid doing things this way. This is selecting instructions incorrectly, and then relying on something else to fix them up. I think we should select trivially correct instructions, and a follow on pass could do a better job figuring out the optimal SGPR operands. This is why RegBankSelect is forcing everything to VGPRs for VALU instructions. The later pass will have an easier time if everything is expected to consistently be in VGPRs, rather than having to figure out if some operands are already scalar and moving them back to VGPRs if it decided another copy was better

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4538 ↗

(On Diff #286821)

We're also incorrectly letting some constant bus violations through with ThreeOp_i32_Pats, which do need a complex predicate to avoid the constant bus violations. Technically the later folding pass could form these cases, but it would be nice to keep these in patterns. I haven't thought of a good solution for this yet

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4538 ↗

(On Diff #286821)

How do we "select trivially correct instructions" if regbankselect has already run, and it has no foreknowledge of what complex patterns we might want to match as part of instruction selection? Some of the current failures to select v_lshl_add are because the inputs to the G_SHL are uniformn (or constant) and regbankselect doesn't know that we might want to combine it into a G_ADD whose other operand is divergent.

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4538 ↗

(On Diff #286821)

It's trivial if you look at a single instruction at a time. The constant bus violations only start showing up when you start looking through copies when pattern matching multiple input operations.

The patterns have to make sure they don't violate the constant bus restriction. As I mentioned, ThreeOp_i32_Pats is broken and really should fail to import. ThreeOpFrag needs a non-trivial predicate that uses PredicateCodeUsesOperands, but GlobalISelEmitter doesn't implement it (or even check if it's in use).

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4538 ↗

(On Diff #286821)

It's trivial if you look at a single instruction at a time. The constant bus violations only start showing up when you start looking through copies when pattern matching multiple input operations.

The patterns have to make sure they don't violate the constant bus restriction. As I mentioned, ThreeOp_i32_Pats is broken and really should fail to import. ThreeOpFrag needs a non-trivial predicate that uses PredicateCodeUsesOperands, but GlobalISelEmitter doesn't implement it (or even check if it's in use).

4538 ↗

(On Diff #286821)

OK, makes sense. We can look at implementing PredicateCodeUsesOperands for GlobalISel in tablegen.

[Petar.Avramovic]

Attempt to teach emitter to collect operands that predicate with 'let PredicateCodeUsesOperands = 1' requires.

[Petar.Avramovic]

Move support for predicate code that uses operands to a separate patch.

[arsenm]

I think copies that can be ignored should be taken care of by a combiner pass, and the selector should not have to do this

[foad]

In D86294#2261887, @arsenm wrote:

I think copies that can be ignored should be taken care of by a combiner pass, and the selector should not have to do this

Can you explain how you think this should work for selecting e.g. v_add_lshl where some of the operands can be sgprs? A combiner pass can't remove cross-register-bank copies, can it?

[arsenm]

In D86294#2262832, @foad wrote:

In D86294#2261887, @arsenm wrote:

I think copies that can be ignored should be taken care of by a combiner pass, and the selector should not have to do this

Can you explain how you think this should work for selecting e.g. v_add_lshl where some of the operands can be sgprs? A combiner pass can't remove cross-register-bank copies, can it?

A combiner could, but under the current scheme it should not. I want to minimize the amount of places/code that need to worry about the constant bus restriction. The pattern for these instructions should search through the copies if it can. We may need a new tablegen operator or something to look through copies

[Petar.Avramovic]

Allow patterns to explicitly ask to look through copies.

[arsenm]

llvm/include/llvm/Target/TargetSelectionDAG.td

313 ↗

(On Diff #292800)

I think the name needs work. I don't think it should have a g_ prefix. Maybe ignore_copies?

[Pierre-vh]

@Petar.Avramovic this could be relevant for https://discourse.llvm.org/t/cleanly-addressing-globalisel-dag-pattern-matching-differences/64817/14, do you think you have time to resume work on this patch & get it landed, or may I take over it?
Thank you

[Petar.Avramovic]

You can take over. I am not so sure this will be useful as is. Just looking through copies will break constant bus limit. Then there are constants and their register banks.
I didn't like g_maybe_cross_reg_bank_copy it would be better to have some field in PatFrag recognized by the global-isel emitter (it would generate GIM_CheckOpcodeIgnoreCopies instead of GIM_CheckOpcode for all opcode checks in pattern).
But that can break constant bus limit. There is a mechanism to collect operands with
PredicateCodeUsesOperands = 1
Maybe you could add another field in PatFrag that will be a signal to generate jump-table entries that will go through collected operands and check constant bus limit for collected operands.
Just to clarify by have some field in PatFrag recognized by the global-isel emitter I meant something like HasNoUse but sdag emitter will ignore it.

[Pierre-vh]

D136234 landed and is an alternative implementation of this

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

case GIM_CheckOpcodeIsEither: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t Expected0 = MatchTable[CurrentIdx++];
       int64_t Expected1 = -1;
       if (MatcherOpcode == GIM_CheckOpcodeIsEither)
         Expected1 = MatchTable[CurrentIdx++];
 
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
       unsigned Opcode = State.MIs[InsnID]->getOpcode();
+      if (Opcode == TargetOpcode::COPY) {
+        Register CopySrc = State.MIs[InsnID]->getOperand(1).getReg();
+        if (!CopySrc.isPhysical()) {
+          auto MI2 = State.MIs[InsnID] = getDefIgnoringCopies(CopySrc, MRI);
+          if (MI2) {
+            State.MIs[InsnID] = MI2;
+            Opcode = MI2->getOpcode();
+          }
+        }
+      }
 
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
         dbgs() << CurrentIdx << ": GIM_CheckOpcode(MIs[" << InsnID
         << "], ExpectedOpcode=" << Expected0;
         if (MatcherOpcode == GIM_CheckOpcodeIsEither)
           dbgs() << " || " << Expected1;
         dbgs() << ") // Got=" << Opcode << "\n";
       );

llvm/test/CodeGen/AMDGPU/GlobalISel/add.v2i16.ll

 ; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX9-NEXT:    v_mov_b32_e32 v1, 0xffc0ffc0
 ; GFX9-NEXT:    v_pk_add_u16 v0, v0, v1
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX8-LABEL: v_add_v2i16_neg_inline_imm_splat:
 ; GFX8:       ; %bb.0:
 ; GFX8-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX8-NEXT:    s_movk_i32 s4, 0xffc0
-; GFX8-NEXT:    v_mov_b32_e32 v2, s4
-; GFX8-NEXT:    v_add_u16_e32 v1, s4, v0
-; GFX8-NEXT:    v_add_u16_sdwa v0, v0, v2 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD
+; GFX8-NEXT:    v_mov_b32_e32 v2, 64
+; GFX8-NEXT:    v_subrev_u16_e32 v1, 64, v0
+; GFX8-NEXT:    v_sub_u16_sdwa v0, v0, v2 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD
 ; GFX8-NEXT:    v_or_b32_e32 v0, v1, v0
 ; GFX8-NEXT:    s_setpc_b64 s[30:31]
   %add = add <2 x i16> %a, <i16 -64, i16 -64>
   ret <2 x i16> %add
 }
 
 define <2 x i16> @v_add_v2i16_neg_inline_imm_lo(<2 x i16> %a) {
 ; GFX9-LABEL: v_add_v2i16_neg_inline_imm_lo:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX9-NEXT:    v_mov_b32_e32 v1, 0x4ffc0
 ; GFX9-NEXT:    v_pk_add_u16 v0, v0, v1
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX8-LABEL: v_add_v2i16_neg_inline_imm_lo:
 ; GFX8:       ; %bb.0:
 ; GFX8-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX8-NEXT:    v_mov_b32_e32 v2, 4
-; GFX8-NEXT:    v_add_u16_e32 v1, 0xffc0, v0
+; GFX8-NEXT:    v_subrev_u16_e32 v1, 64, v0
 ; GFX8-NEXT:    v_add_u16_sdwa v0, v0, v2 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD
 ; GFX8-NEXT:    v_or_b32_e32 v0, v1, v0
 ; GFX8-NEXT:    s_setpc_b64 s[30:31]
   %add = add <2 x i16> %a, <i16 -64, i16 4>
   ret <2 x i16> %add
 }
 
 define <2 x i16> @v_add_v2i16_neg_inline_imm_hi(<2 x i16> %a) {
 ; GFX9-LABEL: v_add_v2i16_neg_inline_imm_hi:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX9-NEXT:    v_mov_b32_e32 v1, 0xffc00004
 ; GFX9-NEXT:    v_pk_add_u16 v0, v0, v1
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX8-LABEL: v_add_v2i16_neg_inline_imm_hi:
 ; GFX8:       ; %bb.0:
 ; GFX8-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX8-NEXT:    v_mov_b32_e32 v1, 0xffffffc0
-; GFX8-NEXT:    v_add_u16_e32 v2, 4, v0
-; GFX8-NEXT:    v_add_u16_sdwa v0, v0, v1 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD
-; GFX8-NEXT:    v_or_b32_e32 v0, v2, v0
+; GFX8-NEXT:    v_mov_b32_e32 v2, 64
+; GFX8-NEXT:    v_add_u16_e32 v1, 4, v0
+; GFX8-NEXT:    v_sub_u16_sdwa v0, v0, v2 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD
+; GFX8-NEXT:    v_or_b32_e32 v0, v1, v0
 ; GFX8-NEXT:    s_setpc_b64 s[30:31]
   %add = add <2 x i16> %a, <i16 4, i16 -64>
   ret <2 x i16> %add
 }
 
 define amdgpu_ps i32 @s_add_v2i16_neg_inline_imm_splat(<2 x i16> inreg %a) {
 ; GFX9-LABEL: s_add_v2i16_neg_inline_imm_splat:
 ; GFX9:       ; %bb.0:

llvm/test/CodeGen/AMDGPU/GlobalISel/add_shl.ll

 ; GFX9-LABEL: add_shl_vgpr_c:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    s_add_i32 s2, s2, s3
 ; GFX9-NEXT:    v_lshlrev_b32_e64 v0, v0, s2
 ; GFX9-NEXT:    ; return to shader part epilog
 ;
 ; GFX10-LABEL: add_shl_vgpr_c:
 ; GFX10:       ; %bb.0:
-; GFX10-NEXT:    s_add_i32 s2, s2, s3
+; GFX10-NEXT:    v_add_lshl_u32 v0, s2, s3, v0
 ; GFX10-NEXT:    ; implicit-def: $vcc_hi
-; GFX10-NEXT:    v_lshlrev_b32_e64 v0, v0, s2
 ; GFX10-NEXT:    ; return to shader part epilog
   %x = add i32 %a, %b
   %result = shl i32 %x, %c
   %bc = bitcast i32 %result to float
   ret float %bc
 }
 
 define amdgpu_ps float @add_shl_vgpr_ac(i32 %a, i32 inreg %b, i32 %c) {

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-pattern-and-or.mir

bb.0:
     ; GFX9: [[V_OR_B32_e64_:%[0-9]+]]:vgpr_32 = V_OR_B32_e64 [[COPY3]], [[COPY2]], implicit $exec
     ; GFX9: S_ENDPGM 0, implicit [[V_OR_B32_e64_]]
     ; GFX10-LABEL: name: and_or_s32_sgpr_sgpr_vgpr
     ; GFX10: liveins: $sgpr0, $sgpr1, $vgpr0
     ; GFX10: $vcc_hi = IMPLICIT_DEF
     ; GFX10: [[COPY:%[0-9]+]]:sreg_32 = COPY $sgpr0
     ; GFX10: [[COPY1:%[0-9]+]]:sreg_32 = COPY $sgpr1
     ; GFX10: [[COPY2:%[0-9]+]]:vgpr_32 = COPY $vgpr0
-    ; GFX10: [[S_AND_B32_:%[0-9]+]]:sreg_32 = S_AND_B32 [[COPY]], [[COPY1]], implicit-def $scc
-    ; GFX10: [[COPY3:%[0-9]+]]:vgpr_32 = COPY [[S_AND_B32_]]
-    ; GFX10: [[V_OR_B32_e64_:%[0-9]+]]:vgpr_32 = V_OR_B32_e64 [[COPY3]], [[COPY2]], implicit $exec
-    ; GFX10: S_ENDPGM 0, implicit [[V_OR_B32_e64_]]
+    ; GFX10: [[V_AND_OR_B32_:%[0-9]+]]:vgpr_32 = V_AND_OR_B32 [[COPY]], [[COPY1]], [[COPY2]], implicit $exec
+    ; GFX10: S_ENDPGM 0, implicit [[V_AND_OR_B32_]]
     %0:sgpr(s32) = COPY $sgpr0
     %1:sgpr(s32) = COPY $sgpr1
     %2:vgpr(s32) = COPY $vgpr0
     %3:sgpr(s32) = G_AND %0, %1
     %4:vgpr(s32) = COPY %3
     %5:vgpr(s32) = G_OR %4, %2
     S_ENDPGM 0, implicit %5
 ...

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-pattern-xor3.mir

bb.0:
     ; GFX9: [[V_XOR_B32_e64_:%[0-9]+]]:vgpr_32 = V_XOR_B32_e64 [[COPY3]], [[COPY2]], implicit $exec
     ; GFX9: S_ENDPGM 0, implicit [[V_XOR_B32_e64_]]
     ; GFX10-LABEL: name: xor_s32_sgpr_sgpr_vgpr_copy
     ; GFX10: liveins: $sgpr0, $sgpr1, $vgpr0
     ; GFX10: $vcc_hi = IMPLICIT_DEF
     ; GFX10: [[COPY:%[0-9]+]]:sreg_32 = COPY $sgpr0
     ; GFX10: [[COPY1:%[0-9]+]]:sreg_32 = COPY $sgpr1
     ; GFX10: [[COPY2:%[0-9]+]]:vgpr_32 = COPY $vgpr0
-    ; GFX10: [[S_XOR_B32_:%[0-9]+]]:sreg_32 = S_XOR_B32 [[COPY]], [[COPY1]], implicit-def $scc
-    ; GFX10: [[COPY3:%[0-9]+]]:vgpr_32 = COPY [[S_XOR_B32_]]
-    ; GFX10: [[V_XOR_B32_e64_:%[0-9]+]]:vgpr_32 = V_XOR_B32_e64 [[COPY3]], [[COPY2]], implicit $exec
-    ; GFX10: S_ENDPGM 0, implicit [[V_XOR_B32_e64_]]
+    ; GFX10: [[V_XOR3_B32_:%[0-9]+]]:vgpr_32 = V_XOR3_B32 [[COPY]], [[COPY1]], [[COPY2]], implicit $exec
+    ; GFX10: S_ENDPGM 0, implicit [[V_XOR3_B32_]]
     %0:sgpr(s32) = COPY $sgpr0
     %1:sgpr(s32) = COPY $sgpr1
     %2:vgpr(s32) = COPY $vgpr0
     %3:sgpr(s32) = G_XOR %0, %1
     %4:vgpr(s32) = COPY %3
     %5:vgpr(s32) = G_XOR %4, %2
     S_ENDPGM 0, implicit %5
 ...
bb.0:
     ; GFX9: [[V_XOR_B32_e64_:%[0-9]+]]:vgpr_32 = V_XOR_B32_e64 [[COPY2]], [[COPY3]], implicit $exec
     ; GFX9: S_ENDPGM 0, implicit [[V_XOR_B32_e64_]]
     ; GFX10-LABEL: name: xor_s32_sgpr_sgpr_vgpr_copy_commute
     ; GFX10: liveins: $sgpr0, $sgpr1, $vgpr0
     ; GFX10: $vcc_hi = IMPLICIT_DEF
     ; GFX10: [[COPY:%[0-9]+]]:sreg_32 = COPY $sgpr0
     ; GFX10: [[COPY1:%[0-9]+]]:sreg_32 = COPY $sgpr1
     ; GFX10: [[COPY2:%[0-9]+]]:vgpr_32 = COPY $vgpr0
-    ; GFX10: [[S_XOR_B32_:%[0-9]+]]:sreg_32 = S_XOR_B32 [[COPY]], [[COPY1]], implicit-def $scc
-    ; GFX10: [[COPY3:%[0-9]+]]:vgpr_32 = COPY [[S_XOR_B32_]]
-    ; GFX10: [[V_XOR_B32_e64_:%[0-9]+]]:vgpr_32 = V_XOR_B32_e64 [[COPY2]], [[COPY3]], implicit $exec
-    ; GFX10: S_ENDPGM 0, implicit [[V_XOR_B32_e64_]]
+    ; GFX10: [[V_XOR3_B32_:%[0-9]+]]:vgpr_32 = V_XOR3_B32 [[COPY]], [[COPY1]], [[COPY2]], implicit $exec
+    ; GFX10: S_ENDPGM 0, implicit [[V_XOR3_B32_]]
     %0:sgpr(s32) = COPY $sgpr0
     %1:sgpr(s32) = COPY $sgpr1
     %2:vgpr(s32) = COPY $vgpr0
     %3:sgpr(s32) = G_XOR %0, %1
     %4:vgpr(s32) = COPY %3
     %5:vgpr(s32) = G_XOR %2, %4
     S_ENDPGM 0, implicit %5
 ...

llvm/test/CodeGen/Mips/GlobalISel/llvm-ir/select.ll

 entry:
   %cond = select i1 %test, i32* %a, i32* %b
   ret i32* %cond
 }
 
 define i32 @select_with_negation(i32 %a, i32 %b, i32 %x, i32 %y) {
 ; MIPS32-LABEL: select_with_negation:
 ; MIPS32:       # %bb.0: # %entry
-; MIPS32-NEXT:    ori $1, $zero, 1
-; MIPS32-NEXT:    slt $2, $4, $5
-; MIPS32-NEXT:    xor $1, $2, $1
+; MIPS32-NEXT:    slt $1, $4, $5
+; MIPS32-NEXT:    xori $1, $1, 1
 ; MIPS32-NEXT:    andi $1, $1, 1
 ; MIPS32-NEXT:    movn $7, $6, $1
 ; MIPS32-NEXT:    move $2, $7
 ; MIPS32-NEXT:    jr $ra
 ; MIPS32-NEXT:    nop
 entry:
   %cmp = icmp slt i32 %a, %b
   %lneg = xor i1 %cmp, true