[AMDGPU] Allow hoisting of some VALU compare instructions

[critson]

Summary

Conversatively allow hoisting/sinking of VALU comparisons.
If the result of a comparison is masked with exec, narrowing the
set of active lanes, then it is safe to hoist it as the masking
instruction will never by hoisted.

Heuristically this is also true for sinking, as we do not expect
the result of a sunk comparison that is masked with exec to be
used outside of the loop.

[rampitec]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

132–133

The function probably needs a different name now.

133–157

This needs a comment.

135

Check it is virtual.

[critson]

• Address reviewer comments.

[nhaehnle]

I suspect that V_CNDMASK uses are fairly common, and those implicitly mask with EXEC as well.

This also doesn't address more complex cases like

%0 = V_CMP ...
%1 = V_CMP ...
%2 = S_OR %0, %1
V_CNDMASK lhs, rhs, %2

though perhaps the change is good as-is for now.

[rampitec]

LGTM with a nit.

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

142

use_nodbg_instructions, otherwise result may be different with and without debug.

[critson]

In D118975#3302072, @nhaehnle wrote:

I suspect that V_CNDMASK uses are fairly common, and those implicitly mask with EXEC as well.

This also doesn't address more complex cases like

%0 = V_CMP ...
%1 = V_CMP ...
%2 = S_OR %0, %1
V_CNDMASK lhs, rhs, %2

though perhaps the change is good as-is for now.

Thanks for pointing this out.

I think it makes sense to look at additional patterns in a follow up patch.
IMO it's safer to expand the set of allowable hoists iteratively (testing as we go).

[critson]

• Address reviewer comments.

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (isVOP1(MI) || isVOP2(MI) || isVOP3(MI) || isSDWA(MI) || isSALU(MI)) {
     // therefore this method includes SOP instructions as well.
     return !MI.hasImplicitDef() &&
            MI.getNumImplicitOperands() == MI.getDesc().getNumImplicitUses() &&
            !MI.mayRaiseFPException();
   }
 
   return false;
 }
 
-static bool readsExecAsData(const MachineInstr &MI) {
-  if (MI.isCompare())
+// Returns true if the scalar result of a VALU instruction depends on exec.
+static bool resultDependsOnExec(const MachineInstr &MI) {
+  // Ignore comparisons which are only used masked with exec.
+  // This allows some hoisting/sinking of VALU comparisons.
+  if (MI.isCompare()) {
+    const MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
+    Register DstReg = MI.getOperand(0).getReg();
+    if (!DstReg.isVirtual())
+      return true;
+    for (MachineInstr &Use : MRI.use_nodbg_instructions(DstReg)) {
+      switch (Use.getOpcode()) {
+      case AMDGPU::S_AND_SAVEEXEC_B32:
+      case AMDGPU::S_AND_SAVEEXEC_B64:
+        break;
+      case AMDGPU::S_AND_B32:
+      case AMDGPU::S_AND_B64:
+        if (!Use.readsRegister(AMDGPU::EXEC))
+          return true;
+        break;
+      default:
         return true;
+      }
+    }
+    return false;
+  }
 
   switch (MI.getOpcode()) {
   default:
     break;
   case AMDGPU::V_READFIRSTLANE_B32:
     return true;
   }
 
   return false;
 }
 
 bool SIInstrInfo::isIgnorableUse(const MachineOperand &MO) const {
   // Any implicit use of exec by VALU is not a real register read.
   return MO.getReg() == AMDGPU::EXEC && MO.isImplicit() &&
-         isVALU(*MO.getParent()) && !readsExecAsData(*MO.getParent());
+         isVALU(*MO.getParent()) && !resultDependsOnExec(*MO.getParent());
 }
 
 bool SIInstrInfo::areLoadsFromSameBasePtr(SDNode *Load0, SDNode *Load1,
                                           int64_t &Offset0,
                                           int64_t &Offset1) const {
   if (!Load0->isMachineOpcode() || !Load1->isMachineOpcode())
     return false;
 

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

body:             |
   ; GCN-NEXT:   successors: %bb.1(0x80000000)
   ; GCN-NEXT: {{  $}}
   ; 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_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 1, 2, implicit $exec
-  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, [[V_CMP_EQ_U32_e64_]], 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:
     S_BRANCH %bb.1
 
   bb.1:
     %0:sreg_64 = V_CMP_EQ_U32_e64 1, 2, implicit $exec
-    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    $exec = S_OR_B64 $exec, %0:sreg_64, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: allowable_hoist_cmp
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: allowable_hoist_cmp
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 1, 2, implicit $exec
+  ; 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:   $exec = S_AND_B64 $exec, [[V_CMP_EQ_U32_e64_]], 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:
+    S_BRANCH %bb.1
+
+  bb.1:
+    %0:sreg_64 = V_CMP_EQ_U32_e64 1, 2, implicit $exec
+    $exec = S_AND_B64 $exec, %0:sreg_64, implicit-def $scc
     S_CBRANCH_EXECNZ %bb.1, implicit $exec
     S_BRANCH %bb.2
 
   bb.2:
     S_ENDPGM 0
 ...
 ---
 name: no_hoist_readfirstlane