[AMDGPU][GlobalISel] Insert an and with exec before s_cbranch_vccnz if necessary

[mbrkusanin]

Summary

While v_cmp will AND inactive lanes with 0, that is not the case for logical
operations.

This fixes a Vulkan CTS test that would hang otherwise.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2517

This isn't the precise condition. It should also cover class intrinsics and maybe a few other cases. I thought we had an is known vector bool helper somewhere?

[foad]

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2517

For SelectionDAG we have isBoolSGPR. I don't know of anything similar for GlobalISel.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2517

We should have the equivalent

[mbrkusanin]

Alternatively we could always insert and with exec and try to remove it in SIOptimizeExecMaskingPreRA (something similar to optimizeVcndVcmpPair).

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2517

I could not find a global-isel equivalent.

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-brcond.mir

231–233

Is this correct? As far as I can tell this is the only way to copy sgpr to vcc.

[foad]

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2485–2486

I don't think you should handle amdgcn_icmp and amdgcn_fcmp here. They are strange beasts that return the full 32/64-bit results of executing a divergent comparison in all lanes, and they should be deprecated in favour of amdgcn_ballot. Remove the corresponding test case as well.

[mbrkusanin]

• Removed amdgcn_icmp and amdgcn_fcmp

[mbrkusanin]

• Covered case with amdgcn_ballot.

[foad]

Looks OK to me, just one suggestion inline.

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

2518–2519

Use TRI.getBoolRC?

[foad]

In D105709#2910506, @mbrkusanin wrote:

• Covered case with amdgcn_ballot.

No, ballot is a weird thing that returns the full 32/64-bit result of evaluating an expression in all lanes. You should not handle it here. I meant to approve the previous version of the patch.

[mbrkusanin]

• Remove amdgcn_ballot case
• Use TRI.getBoolRC()

llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp

bool AMDGPUInstructionSelector::selectG_AMDGPU_ATOMIC_CMPXCHG(
 
   MI.eraseFromParent();
 
   MRI->setRegClass(
     DstReg, Is64 ? &AMDGPU::VReg_64RegClass : &AMDGPU::VGPR_32RegClass);
   return constrainSelectedInstRegOperands(*MIB, TII, TRI, RBI);
 }
 
+static bool isVCmpResult(Register Reg, MachineRegisterInfo &MRI) {
+  if (Reg.isPhysical())
+    return false;
+
+  MachineInstr &MI = *MRI.getUniqueVRegDef(Reg);
+  const unsigned Opcode = MI.getOpcode();
+
+  if (Opcode == AMDGPU::COPY)
+    return isVCmpResult(MI.getOperand(1).getReg(), MRI);
+
+  if (Opcode == AMDGPU::G_AND || Opcode == AMDGPU::G_OR ||
+      Opcode == AMDGPU::G_XOR)
+    return isVCmpResult(MI.getOperand(1).getReg(), MRI) &&
+           isVCmpResult(MI.getOperand(2).getReg(), MRI);
+
+  if (Opcode == TargetOpcode::G_INTRINSIC)
+    return MI.getIntrinsicID() == Intrinsic::amdgcn_class;
+
+  return Opcode == AMDGPU::G_ICMP || Opcode == AMDGPU::G_FCMP;
+}
+
 bool AMDGPUInstructionSelector::selectG_BRCOND(MachineInstr &I) const {
   MachineBasicBlock *BB = I.getParent();
   MachineOperand &CondOp = I.getOperand(0);
   Register CondReg = CondOp.getReg();
   const DebugLoc &DL = I.getDebugLoc();
 
   unsigned BrOpcode;
   Register CondPhysReg;
   const TargetRegisterClass *ConstrainRC;
 
   // In SelectionDAG, we inspect the IR block for uniformity metadata to decide
   // whether the branch is uniform when selecting the instruction. In
   // GlobalISel, we should push that decision into RegBankSelect. Assume for now
   // RegBankSelect knows what it's doing if the branch condition is scc, even
   // though it currently does not.
   if (!isVCC(CondReg, *MRI)) {
     if (MRI->getType(CondReg) != LLT::scalar(32))
       return false;
 
     CondPhysReg = AMDGPU::SCC;
     BrOpcode = AMDGPU::S_CBRANCH_SCC1;
     ConstrainRC = &AMDGPU::SReg_32RegClass;
   } else {
-    // FIXME: Do we have to insert an and with exec here, like in SelectionDAG?
-    // We sort of know that a VCC producer based on the register bank, that ands
-    // inactive lanes with 0. What if there was a logical operation with vcc
-    // producers in different blocks/with different exec masks?
     // FIXME: Should scc->vcc copies and with exec?
+
+    // Unless the value of CondReg is a result of a V_CMP* instruction then we
+    // need to insert an and with exec.
+    if (!isVCmpResult(CondReg, *MRI)) {
+      const bool Is64 = STI.isWave64();
+      const unsigned Opcode = Is64 ? AMDGPU::S_AND_B64 : AMDGPU::S_AND_B32;
+      const Register Exec = Is64 ? AMDGPU::EXEC : AMDGPU::EXEC_LO;
+
+      Register TmpReg = MRI->createVirtualRegister(TRI.getBoolRC());
+      BuildMI(*BB, &I, DL, TII.get(Opcode), TmpReg)
+          .addReg(CondReg)
+          .addReg(Exec);
+      CondReg = TmpReg;
+    }
+
     CondPhysReg = TRI.getVCC();
     BrOpcode = AMDGPU::S_CBRANCH_VCCNZ;
     ConstrainRC = TRI.getBoolRC();
   }
 
   if (!MRI->getRegClassOrNull(CondReg))
     MRI->setRegClass(CondReg, ConstrainRC);
 

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-brcond.mir

bb.0:
 
     %0:vgpr(s32) = COPY $vgpr0
     %1:vgpr(s1) = G_TRUNC %0
     G_BRCOND %1, %bb.1
 
   bb.1:
 
 ...
+
+---
+
+name:            brcond_class_intrinsic
+legalized:       true
+regBankSelected: true
+
+body:             |
+  ; GCN-LABEL: name: brcond_class_intrinsic
+  ; GCN: bb.0:
+  ; GCN:   successors: %bb.1(0x80000000)
+  ; GCN:   [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+  ; GCN:   [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr1
+  ; GCN:   [[V_CMP_CLASS_F32_e64_:%[0-9]+]]:sreg_64 = V_CMP_CLASS_F32_e64 0, [[COPY]], [[COPY1]], implicit $exec
+  ; GCN:   $vcc = COPY [[V_CMP_CLASS_F32_e64_]]
+  ; GCN:   S_CBRANCH_VCCNZ %bb.1, implicit $vcc
+  ; GCN: bb.1:
+  bb.0:
+    liveins: $vgpr0, $vgpr1
+
+    %0:vgpr(s32) = COPY $vgpr0
+    %1:vgpr(s32) = COPY $vgpr1
+    %2:vcc(s1) = G_INTRINSIC intrinsic(@llvm.amdgcn.class), %0:vgpr(s32), %1:vgpr(s32)
+    G_BRCOND %2(s1), %bb.1
+
+  bb.1:
+
+...
+
+---
+
+name:            brcond_cmp_logic
+legalized:       true
+regBankSelected: true
+
+body: |
+  ; GCN-LABEL: name: brcond_cmp_logic
+  ; GCN: bb.0:
+  ; GCN:   successors: %bb.1(0x80000000)
+  ; GCN:   [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+  ; GCN:   [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr1
+  ; GCN:   [[COPY2:%[0-9]+]]:vgpr_32 = COPY $vgpr2
+  ; GCN:   [[COPY3:%[0-9]+]]:vgpr_32 = COPY $vgpr3
+  ; GCN:   [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 [[COPY]], [[COPY1]], implicit $exec
+  ; GCN:   %5:sreg_64 = nofpexcept V_CMP_EQ_F32_e64 0, [[COPY2]], 0, [[COPY3]], 0, implicit $mode, implicit $exec
+  ; GCN:   [[S_AND_B64_:%[0-9]+]]:sreg_64 = S_AND_B64 [[V_CMP_EQ_U32_e64_]], %5, implicit-def dead $scc
+  ; GCN:   $vcc = COPY [[S_AND_B64_]]
+  ; GCN:   S_CBRANCH_VCCNZ %bb.1, implicit $vcc
+  ; GCN: bb.1:
+  bb.0:
+    liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3
+
+    %0:vgpr(s32) = COPY $vgpr0
+    %1:vgpr(s32) = COPY $vgpr1
+    %2:vgpr(s32) = COPY $vgpr2
+    %3:vgpr(s32) = COPY $vgpr3
+    %4:vcc(s1) = G_ICMP intpred(eq), %0, %1
+    %5:vcc(s1) = G_FCMP floatpred(oeq), %2, %3
+    %6:vcc(s1) = G_AND %4, %5
+    G_BRCOND %6(s1), %bb.1
+
+  bb.1:
+
+...
+
+---
+
+name:            brcond_logic
+legalized:       true
+regBankSelected: true
+
+body:             |
+  ; GCN-LABEL: name: brcond_logic
+  ; GCN: bb.0:
+  ; GCN:   successors: %bb.1(0x80000000)
+  ; GCN:   [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+  ; GCN:   [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr1
+  ; GCN:   [[COPY2:%[0-9]+]]:sreg_32 = COPY $sgpr0
+  ; GCN:   [[S_AND_B32_:%[0-9]+]]:sreg_32 = S_AND_B32 1, [[COPY2]], implicit-def $scc
+  ; GCN:   [[V_CMP_NE_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_NE_U32_e64 0, [[S_AND_B32_]], implicit $exec
+  ; GCN:   [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 [[COPY]], [[COPY1]], implicit $exec
+  ; GCN:   [[S_AND_B64_:%[0-9]+]]:sreg_64 = S_AND_B64 [[V_CMP_EQ_U32_e64_]], [[V_CMP_NE_U32_e64_]], implicit-def dead $scc
+  ; GCN:   [[S_AND_B64_1:%[0-9]+]]:sreg_64 = S_AND_B64 [[S_AND_B64_]], $exec, implicit-def $scc
+  ; GCN:   $vcc = COPY [[S_AND_B64_1]]
+  ; GCN:   S_CBRANCH_VCCNZ %bb.1, implicit $vcc
+  ; GCN: bb.1:
+  bb.0:
+    liveins: $sgpr0, $vgpr0, $vgpr1
+
+    %0:vgpr(s32) = COPY $vgpr0
+    %1:vgpr(s32) = COPY $vgpr1
+    %2:sgpr(s32) = COPY $sgpr0
+    %3:sgpr(s1) = G_TRUNC %2(s32)
+    %4:vcc(s1) = COPY %3(s1)
+    %5:vcc(s1) = G_ICMP intpred(eq), %0, %1
+    %6:vcc(s1) = G_AND %5, %4
+    G_BRCOND %6(s1), %bb.1
+
+  bb.1:
+
+...
+
+---
+
+name:            brcond_logic_const
+legalized:       true
+regBankSelected: true
+
+body:             |
+  ; GCN-LABEL: name: brcond_logic_const
+  ; GCN: bb.0:
+  ; GCN:   successors: %bb.1(0x80000000)
+  ; GCN:   [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+  ; GCN:   [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr1
+  ; GCN:   [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 [[COPY]], [[COPY1]], implicit $exec
+  ; GCN:   [[S_MOV_B64_:%[0-9]+]]:sreg_64 = S_MOV_B64 -1
+  ; GCN:   [[S_XOR_B64_:%[0-9]+]]:sreg_64 = S_XOR_B64 [[V_CMP_EQ_U32_e64_]], [[S_MOV_B64_]], implicit-def dead $scc
+  ; GCN:   [[S_AND_B64_:%[0-9]+]]:sreg_64 = S_AND_B64 [[S_XOR_B64_]], $exec, implicit-def $scc
+  ; GCN:   $vcc = COPY [[S_AND_B64_]]
+  ; GCN:   S_CBRANCH_VCCNZ %bb.1, implicit $vcc
+  ; GCN: bb.1:
+  bb.0:
+    liveins: $vgpr0, $vgpr1
+
+    %0:vgpr(s32) = COPY $vgpr0
+    %1:vgpr(s32) = COPY $vgpr1
+    %2:vcc(s1) = G_ICMP intpred(eq), %0, %1
+    %3:sgpr(s1) = G_CONSTANT i1 true
+    %4:vcc(s1) = COPY %3(s1)
+    %5:vcc(s1) = G_XOR %2, %4
+    G_BRCOND %5(s1), %bb.1
+
+  bb.1:
+
+...