[AMDGPU] Only unbundle memory accesses in SIMemoryLegalizer

[critson]

Summary

This restores previous behaviour and is a step toward removing
unbundling entirely.

[foad]

Specifically, this reverts part of D91048. Looks good to me, thanks!

[critson]

Thanks, I will leave this 12 hours before committing in case of further comments.

[rampitec]

LGTM

[t-tye]

What was the decision on the philosophy on who should be doing unbundling in general? How is it known that it is sfe for the memory legalized to do this unbundling? Is the policy documented in some AMD GPU design page?

[critson]

In D99061#2641967, @t-tye wrote:

What was the decision on the philosophy on who should be doing unbundling in general? How is it known that it is sfe for the memory legalized to do this unbundling? Is the policy documented in some AMD GPU design page?

I do not think we have reached a consensus yet.
However my understanding of current direction is:

• Stop all unbundling where possible
• Add annotations to bundles so that downstream passes know which ones are legal to modify

llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp

bool SIMemoryLegalizer::runOnMachineFunction(MachineFunction &MF) {
 
   SIMemOpAccess MOA(MF);
   CC = SICacheControl::create(MF.getSubtarget<GCNSubtarget>());
 
   for (auto &MBB : MF) {
     for (auto MI = MBB.begin(); MI != MBB.end(); ++MI) {
 
       // Unbundle instructions after the post-RA scheduler.
-      if (MI->isBundle()) {
+      if (MI->isBundle() && MI->mayLoadOrStore()) {
         MachineBasicBlock::instr_iterator II(MI->getIterator());
         for (MachineBasicBlock::instr_iterator I = ++II, E = MBB.instr_end();
              I != E && I->isBundledWithPred(); ++I) {
           I->unbundleFromPred();
           for (MachineOperand &MO : I->operands())
             if (MO.isReg())
               MO.setIsInternalRead(false);
         }

llvm/test/CodeGen/AMDGPU/GlobalISel/extractelement.i128.ll

 ; GFX9-LABEL: extractelement_vgpr_v4i128_sgpr_idx:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    global_load_dwordx4 v[2:5], v[0:1], off
 ; GFX9-NEXT:    global_load_dwordx4 v[6:9], v[0:1], off offset:16
 ; GFX9-NEXT:    global_load_dwordx4 v[10:13], v[0:1], off offset:32
 ; GFX9-NEXT:    global_load_dwordx4 v[14:17], v[0:1], off offset:48
 ; GFX9-NEXT:    s_lshl_b32 s0, s2, 1
 ; GFX9-NEXT:    s_lshl_b32 s2, s0, 1
-; GFX9-NEXT:    s_set_gpr_idx_on s2, gpr_idx(SRC0)
 ; GFX9-NEXT:    s_waitcnt vmcnt(0)
+; GFX9-NEXT:    s_set_gpr_idx_on s2, gpr_idx(SRC0)
 ; GFX9-NEXT:    v_mov_b32_e32 v0, v2
 ; GFX9-NEXT:    v_mov_b32_e32 v1, v3
 ; GFX9-NEXT:    v_mov_b32_e32 v18, v2
 ; GFX9-NEXT:    v_mov_b32_e32 v3, v3
 ; GFX9-NEXT:    s_set_gpr_idx_off
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    v_readfirstlane_b32 s1, v1
 ; GFX9-NEXT:    v_readfirstlane_b32 s2, v18