[AMDGPU] Don't require swz operand for non-return Atomics.

[MJDSys]

Summary

When legalizing operands for instructions that are not atmoics with
a return, do not assume the swz operand is present.

This avoids an assert in the openmm test suite.

[rampitec]

Needs test.

[MJDSys]

In D91487#2397822, @rampitec wrote:

Needs test.

Hi @rampitec,

I've tried writing a test for this change, but I've been having trouble (I'm new to LLVM so I might be missing the obvious answer). From what I can see, I thought the correct option would be to create a MIR test, using the same instruction that I saw when investigating the original problem. I started by copying and modifying an existing test in fix-sgpr-copies.mir:

# Test to ensure non-return buffer atomics work
# GCN-LABEL: name: non-return-buffer-atomic
# GCN: S_CSELECT_B64 -1, 0, implicit undef $scc
# GCN: V_CNDMASK
---
name: non-return-buffer-atomic
tracksRegLiveness: true

body:               |
  bb.0:
  %1:vreg_64 = IMPLICIT_DEF
  %2:vreg_128 = IMPLICIT_DEF
  BUFFER_ATOMIC_OR_X2_OFFSET %1, %2, 0, 0, 0, implicit $exec
---

However, when I try running the test I get an assert happening, with LLVM complaining the machine code is incorrect due to an illegal virtual register. I got stuck here as I'm trying to create an instruction with an incorrect register to have the code fix it (same as what is done in the failing pass). I'm not sure how to proceed, do you have any suggestion/pointers? Or an alternate test I can use as a template? Note: I'm aware the expected output of the test was incorrect, I was first wanting to verify the test output manually before making it work during a normal test run.

Thanks,

Matthew

[rampitec]

In D91487#2421897, @MJDSys wrote:

In D91487#2397822, @rampitec wrote:

Needs test.

Hi @rampitec,

I've tried writing a test for this change, but I've been having trouble (I'm new to LLVM so I might be missing the obvious answer). From what I can see, I thought the correct option would be to create a MIR test, using the same instruction that I saw when investigating the original problem. I started by copying and modifying an existing test in fix-sgpr-copies.mir:

legalizeOperands() works as a part of the instruction selection so you cannot use mir test anyway. You need to write an .ll test (presumably calling an intrinsic to emit this atomic). You can start with code from llvm.amdgcn.buffer.atomic.ll.

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (VAddr && AMDGPU::getIfAddr64Inst(MI.getOpcode()) != -1) {
 
         MIB.addImm(getNamedImmOperand(MI, AMDGPU::OpName::slc));
 
         if (const MachineOperand *TFE =
                 getNamedOperand(MI, AMDGPU::OpName::tfe)) {
           MIB.addImm(TFE->getImm());
         }
 
-        MIB.addImm(getNamedImmOperand(MI, AMDGPU::OpName::swz));
+        if (const MachineOperand *SWZ =
+                getNamedOperand(MI, AMDGPU::OpName::swz)) {
+          MIB.addImm(SWZ->getImm());
+        }
 
         MIB.cloneMemRefs(MI);
         Addr64 = MIB;
       } else {
         // Atomics with return.
         Addr64 = BuildMI(MBB, MI, MI.getDebugLoc(), get(Addr64Opcode))
                      .add(*VData)
                      .add(*VDataIn)
MachineBasicBlock *SIInstrInfo::moveToVALU(MachineInstr &TopInst,
                                            MachineDominatorTree *MDT) const {
   SetVectorType Worklist;
   Worklist.insert(&TopInst);
   MachineBasicBlock *CreatedBB = nullptr;
   MachineBasicBlock *CreatedBBTmp = nullptr;
 
   while (!Worklist.empty()) {
     MachineInstr &Inst = *Worklist.pop_back_val();
+    LLVM_DEBUG(dbgs() << "Moving to VALU " << Inst);
     MachineBasicBlock *MBB = Inst.getParent();
     MachineRegisterInfo &MRI = MBB->getParent()->getRegInfo();
 
     unsigned Opcode = Inst.getOpcode();
     unsigned NewOpcode = getVALUOp(Inst);
 
     // Handle some special cases
     switch (Opcode) {