[AMDGPU] Combine DPP mov even if old reg def is in different BB

[foad]

Summary

Given a DPP mov like this:

%2:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
...
%3:vgpr_32 = V_MOV_B32_dpp %2, %1, 1, 1, 1, 0, implicit $exec

this patch just removes a check that %2 (the "old reg") was defined in
the same BB as the DPP mov instruction. GCNDPPCombine requires that the
MIR is in SSA form so I don't understand why the BB matters.

This lets the optimization work in more real world cases when the
definition of %2 gets hoisted out of a loop.

[foad]

This has been tested by running the Vulkan CTS subgroups tests on gfx1030 (./deqp-vk --deqp-case="dEQP-VK.subgroups*") - thanks @piotr!

[arsenm]

The worry is changes in exec between the def and the use

[rampitec]

In D124182#3465094, @arsenm wrote:

The worry is changes in exec between the def and the use

There is llvm::execMayBeModifiedBeforeUse() in the SIInstrInfo.cpp. Maybe it shall be used here?

[foad]

In D124182#3465094, @arsenm wrote:

The worry is changes in exec between the def and the use

I don't get it. The full sequence is something like this:

%2:vgpr_32 = V_MOV_B32_e32 0, implicit $exec // A
...
%3:vgpr_32 = V_MOV_B32_dpp %2, %1, 1, 1, 1, 0, implicit $exec // B
...
%4:vgpr_32 = V_ADD_U32_e32 %3, %0, implicit $exec // C

A sets up the value to be used when reading inactive lanes, which will either be don't-care or must be an identity for the operation done in C.

I can kind of understand that exec changes between B and C might be a problem. That is what @cwabbott called out in D55314#1321303, and @vpykhtin added a check for in D55444. But they also added a check for A and B being in the same BB.

If we're in SSA form then the only exec change that makes sense between A and B is narrowing the exec mask, as if B was inside the body of an "if", and that does not change the fact that B uses the constant value defined by A. I don't see how this can go wrong.

[vpykhtin]

Yep, in SSA use of A means it dominates B so it seems correct to expect only narrowing of the exec mask. Let's try and see what happens.

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

bool GCNDPPCombine::combineDPPMov(MachineInstr &MovMI) const {
   if (MaskAllLanes && BoundCtrlZero) { // [1]
     CombBCZ = true;
   } else {
     if (!OldOpndValue || !OldOpndValue->isImm()) {
       LLVM_DEBUG(dbgs() << "  failed: the DPP mov isn't combinable\n");
       return false;
     }
 
-    if (OldOpndValue->getParent()->getParent() != MovMI.getParent()) {
-      LLVM_DEBUG(dbgs() <<
-        "  failed: old reg def and mov should be in the same BB\n");
-      return false;
-    }
-
     if (OldOpndValue->getImm() == 0) {
       if (MaskAllLanes) {
         assert(!BoundCtrlZero); // by check [1]
         CombBCZ = true;
       }
     } else if (BoundCtrlZero) {
       assert(!MaskAllLanes); // by check [1]
       LLVM_DEBUG(dbgs() <<

llvm/test/CodeGen/AMDGPU/dpp_combine.mir

body: |
 
   bb.1:
     successors: %bb.2
 
   bb.2:
     SI_END_CF %8, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
 ...
 
-# old reg def is in diff BB - cannot combine
 # GCN-LABEL: name: old_in_diff_bb
-# GCN: %3:vgpr_32 = V_MOV_B32_dpp %2, %1, 1, 1, 1, 0, implicit $exec
+# GCN: %4:vgpr_32 = V_ADD_U32_dpp %0, %1, %0, 1, 1, 1, 0, implicit $exec
 
 name: old_in_diff_bb
 tracksRegLiveness: true
 body: |
   bb.0:
     successors: %bb.1
     liveins: $vgpr0, $vgpr1