[AMDGPU] Remove cndmask from readsExecAsData

[ruiling]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

133

The function name is confusing, all these instructions access EXEC just as normal vector instructions. One cause of the failure @sebastian-ne showed in D116053 is we are dropping convergent attribute when lowering amdgcn.icmp into V_CMP. I am not sure if there is any other reason. I think may be a right fix for the issue is define convergent version of V_CMP, I am not sure whether it sounds reasonable? Please also add some comments this is just workaround for the issue that we are lowering convergent icmp into normal V_CMP. We should fix it some other way.

140

Did you see a failure caused by v_readfirstlane? if not, please also remove it.

[foad]

LGTM. I agree with all of @ruiling's comments but I don't think they should block this patch, which is clearly a step in the right direction.

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

133

I think may be a right fix for the issue is define convergent version of V_CMP, I am not sure whether it sounds reasonable?

Yes, sounds good to me. There is also a related problem in that the "convergent" attribute in IR prevents an instruction from being sunk into an "if" but it does not prevent it from being hoisted out of an "if". For ballot we need both. I'm not sure if the MachineIR convergent flag does both.

[ruiling]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

133

Yes, sounds good to me. There is also a related problem in that the "convergent" attribute in IR prevents an instruction from being sunk into an "if" but it does not prevent it from being hoisted out of an "if". For ballot we need both. I'm not sure if the MachineIR convergent flag does both.

MachineIR have prohibited both sinking and hoisting.

[rampitec]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

133

The function name and the whole idea reflects that unlike other VALU where exec just predicates the execution these instructions use exec much like a normal operand which is reflected in the actual numerical answer. I.e., for cmp that is:

dst = cmp(src0, src1) & exec

For readfirstlane that is:

dst = vgpr[cttz(exec)]

Marking cmp as convergent might be a better fix because it will still allow to move compares within a block, although it sounds a little orthogonal to the exec use modeling to me.

140

No, because it is convergent. But technically is has the same effect of using exec.

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (isVOP1(MI) || isVOP2(MI) || isVOP3(MI) || isSDWA(MI) || isSALU(MI)) {
     return !MI.hasImplicitDef() &&
            MI.getNumImplicitOperands() == MI.getDesc().getNumImplicitUses() &&
            !MI.mayRaiseFPException();
   }
 
   return false;
 }
 
 static bool readsExecAsData(const MachineInstr &MI) {
   if (MI.isCompare())
     return true;
 
   switch (MI.getOpcode()) {
   default:
     break;
   case AMDGPU::V_READFIRSTLANE_B32:
-  case AMDGPU::V_CNDMASK_B64_PSEUDO:
-  case AMDGPU::V_CNDMASK_B32_dpp:
-  case AMDGPU::V_CNDMASK_B32_e32:
-  case AMDGPU::V_CNDMASK_B32_e64:
-  case AMDGPU::V_CNDMASK_B32_sdwa:
     return true;
   }
 
   return false;
 }
 
 bool SIInstrInfo::isIgnorableUse(const MachineOperand &MO) const {
   // Any implicit use of exec by VALU is not a real register read.

llvm/test/CodeGen/AMDGPU/licm-valu.mir

bb.1:
     %1:sgpr_32 = V_READFIRSTLANE_B32 %0:vgpr_32, implicit $exec
     $exec = S_OR_B64 $exec, 1, implicit-def $scc
     S_CBRANCH_EXECNZ %bb.1, implicit $exec
     S_BRANCH %bb.2
 
   bb.2:
     S_ENDPGM 0
 ...
----
-name: no_hoist_cndmask_e64
-tracksRegLiveness: true
-body:             |
-  ; GCN-LABEL: name: no_hoist_cndmask_e64
-  ; GCN: bb.0:
-  ; GCN-NEXT:   successors: %bb.1(0x80000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
-  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
-  ; GCN-NEXT:   S_BRANCH %bb.1
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.1:
-  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[V_CNDMASK_B32_e64_:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_e64 0, [[DEF]], 0, [[DEF]], [[DEF1]], implicit $exec
-  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
-  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
-  ; GCN-NEXT:   S_BRANCH %bb.2
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.2:
-  ; GCN-NEXT:   S_ENDPGM 0
-  bb.0:
-    %0:vgpr_32 = IMPLICIT_DEF
-    %1:sreg_64_xexec = IMPLICIT_DEF
-    S_BRANCH %bb.1
-
-  bb.1:
-    %2:vgpr_32 = V_CNDMASK_B32_e64 0, %0, 0, %0, %1, implicit $exec
-    $exec = S_OR_B64 $exec, 1, implicit-def $scc
-    S_CBRANCH_EXECNZ %bb.1, implicit $exec
-    S_BRANCH %bb.2
-
-  bb.2:
-    S_ENDPGM 0
-...
----
-name: no_hoist_cndmask_e32
-tracksRegLiveness: true
-body:             |
-  ; GCN-LABEL: name: no_hoist_cndmask_e32
-  ; GCN: bb.0:
-  ; GCN-NEXT:   successors: %bb.1(0x80000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
-  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
-  ; GCN-NEXT:   S_BRANCH %bb.1
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.1:
-  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[V_CNDMASK_B32_e32_:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_e32 [[DEF]], [[DEF]], implicit undef $vcc, implicit $exec
-  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
-  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
-  ; GCN-NEXT:   S_BRANCH %bb.2
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.2:
-  ; GCN-NEXT:   S_ENDPGM 0
-  bb.0:
-    %0:vgpr_32 = IMPLICIT_DEF
-    %1:sreg_64_xexec = IMPLICIT_DEF
-    S_BRANCH %bb.1
-
-  bb.1:
-    %2:vgpr_32 = V_CNDMASK_B32_e32 %0, %0, implicit undef $vcc, implicit $exec
-    $exec = S_OR_B64 $exec, 1, implicit-def $scc
-    S_CBRANCH_EXECNZ %bb.1, implicit $exec
-    S_BRANCH %bb.2
-
-  bb.2:
-    S_ENDPGM 0
-...
----
-name: no_hoist_cndmask_dpp
-tracksRegLiveness: true
-body:             |
-  ; GCN-LABEL: name: no_hoist_cndmask_dpp
-  ; GCN: bb.0:
-  ; GCN-NEXT:   successors: %bb.1(0x80000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
-  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
-  ; GCN-NEXT:   S_BRANCH %bb.1
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.1:
-  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[V_CNDMASK_B32_dpp:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_dpp [[DEF]], 0, [[DEF]], 0, [[DEF]], 1, 15, 15, 10, implicit $exec, implicit undef $vcc
-  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
-  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
-  ; GCN-NEXT:   S_BRANCH %bb.2
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.2:
-  ; GCN-NEXT:   S_ENDPGM 0
-  bb.0:
-    %0:vgpr_32 = IMPLICIT_DEF
-    %1:sreg_64_xexec = IMPLICIT_DEF
-    S_BRANCH %bb.1
-
-  bb.1:
-    %2:vgpr_32 = V_CNDMASK_B32_dpp %0:vgpr_32, 0, %0:vgpr_32, 0, %0:vgpr_32, 1, 15, 15, 10, implicit $exec, implicit undef $vcc
-    $exec = S_OR_B64 $exec, 1, implicit-def $scc
-    S_CBRANCH_EXECNZ %bb.1, implicit $exec
-    S_BRANCH %bb.2
-
-  bb.2:
-    S_ENDPGM 0
-...
----
-name: no_hoist_cndmask_sdwa
-tracksRegLiveness: true
-body:             |
-  ; GCN-LABEL: name: no_hoist_cndmask_sdwa
-  ; GCN: bb.0:
-  ; GCN-NEXT:   successors: %bb.1(0x80000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
-  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
-  ; GCN-NEXT:   S_BRANCH %bb.1
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.1:
-  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT:   [[V_CNDMASK_B32_sdwa:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_sdwa 0, [[DEF]], 0, [[DEF]], 0, 0, 0, 0, 0, implicit $exec, implicit undef $vcc
-  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
-  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
-  ; GCN-NEXT:   S_BRANCH %bb.2
-  ; GCN-NEXT: {{  $}}
-  ; GCN-NEXT: bb.2:
-  ; GCN-NEXT:   S_ENDPGM 0
-  bb.0:
-    %0:vgpr_32 = IMPLICIT_DEF
-    %1:sreg_64_xexec = IMPLICIT_DEF
-    S_BRANCH %bb.1
-
-  bb.1:
-    %2:vgpr_32 = V_CNDMASK_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 0, 0, 0, 0, implicit $exec, implicit undef $vcc
-    $exec = S_OR_B64 $exec, 1, implicit-def $scc
-    S_CBRANCH_EXECNZ %bb.1, implicit $exec
-    S_BRANCH %bb.2
-
-  bb.2:
-    S_ENDPGM 0
-...