AMDGPU: Take care of "tied" operand when removeOperand

[cfang]

Summary

Flat scratch load of D16 type by default has tied vdst_in operand (with vdst). This should be taken
care of at the time of "removeOperand" in eliminateFrameIndex. Otherwise we will hit an assert saying
"Cannot move tied operands". This patch unties vdst_in before the move, and retie it with vdst afterwards.

[foad]

llvm/test/CodeGen/AMDGPU/tied-operand.ll

1

Can you add this to frame-index-elimination.ll instead of creating a new file? Or at least give the new file a more descriptive name, maybe frame-index-elimination-tied-operand.ll?

[cfang]

llvm/test/CodeGen/AMDGPU/tied-operand.ll

1

Rename to frame-index-elimination-tied-operand.ll. Using a new file to avoid unnecessary checks.

[cfang]

Rename the LIT test to frame-index-elimination-tied-operand.ll as suggested.

[arsenm]

I'd also like a mir test

llvm/test/CodeGen/AMDGPU/frame-index-elimination-tied-operand.ll

2 ↗

(On Diff #447762)

Can you merge this test with one of the existing frame elimination tests?

12–13 ↗

(On Diff #447762)

Avoid using undefs in this test

[cfang]

Add a mir test, and remove the use of undef in the test.

[cfang]

llvm/test/CodeGen/AMDGPU/frame-index-elimination-tied-operand.ll

2 ↗

(On Diff #447762)

There is a difficulty in the merge. This test is only for gfx1100, and we should not check for any other targets. And the existing tests of frame index elimination never check for gfx1100. It is beyond this work to do additional checks for gfx1100 for other works in frame index elimination.

[cfang]

Merge the new test into frame-index-elimination.ll

[arsenm]

llvm/test/CodeGen/AMDGPU/frame-index-elimination-tied-operand.mir

2 ↗

(On Diff #448214)

Should add gfx11 to the test name?

llvm/test/CodeGen/AMDGPU/frame-index-elimination.ll

4 ↗

(On Diff #448214)

Would be better to have full checks for all targets on all functions

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

               NewOpc = AMDGPU::getFlatScratchInstSVfromSVS(Opc);
             } else if (ST.hasFlatScratchSTMode()) {
               // On GFX10 we have ST mode to use no registers for an address.
               // Otherwise we need to materialize 0 into an SGPR.
               NewOpc = AMDGPU::getFlatScratchInstSTfromSS(Opc);
             }
 
             if (NewOpc != -1) {
+              // removeOperand doesn't fixup tied operand indexes at it goes, so
+              // it asserts. Untie vdst_in for now and retie them afterwards.
+              int VDstIn = AMDGPU::getNamedOperandIdx(Opc,
+                                                     AMDGPU::OpName::vdst_in);
+              bool TiedVDst = VDstIn != -1 &&
+                              MI->getOperand(VDstIn).isReg() &&
+                              MI->getOperand(VDstIn).isTied();
+              if (TiedVDst)
+                MI->untieRegOperand(VDstIn);
+
               MI->removeOperand(
                   AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::saddr));
+
+              if (TiedVDst) {
+                int NewVDst =
+                    AMDGPU::getNamedOperandIdx(NewOpc, AMDGPU::OpName::vdst);
+                int NewVDstIn =
+                    AMDGPU::getNamedOperandIdx(NewOpc, AMDGPU::OpName::vdst_in);
+                assert (NewVDst != -1 && NewVDstIn != -1 && "Must be tied!");
+                MI->tieOperands(NewVDst, NewVDstIn);
+              }
               MI->setDesc(TII->get(NewOpc));
               return;
             }
           }
         }
 
         if (!FrameReg) {
           FIOp.ChangeToImmediate(Offset);

llvm/test/CodeGen/AMDGPU/tied-operand.ll

+; RUN: llc -march=amdgcn -mcpu=gfx1100 < %s | FileCheck --check-prefix=GCN %s
+
+define amdgpu_kernel void @tied_operand_test() local_unnamed_addr {
+; GCN-LABEL: tied_operand_test:
+; GCN:       ; %bb.0: ; %entry
+; GCN-NEXT:    scratch_load_u16 [[LDRESULT:v[0-9]+]], off, off offset:4
+; GCN-NEXT:    s_waitcnt vmcnt(0)
+; GCN-NEXT:    ds_store_b16 v{{[0-9]+}}, [[LDRESULT]]
+; GCN-NEXT:    s_endpgm
+entry:
+  %scratch = alloca i16, align 4, addrspace(5)
+  %cond = select i1 undef, i16 addrspace(5)* %scratch, i16 addrspace(5)* undef
+  %spec.select = select i1 undef, i16 addrspace(5)* %cond, i16 addrspace(5)* %scratch
+  %dead.load = load i16, i16 addrspace(5)* %spec.select, align 2
+  %flat.scratch.load = load i16, i16 addrspace(5)* %scratch, align 4
+  store i16 %flat.scratch.load, i16 addrspace(3)* undef, align 2
+  ret void
+}