AMDGPU: Adjust the chain for loads writing to the HI part of a register.

[cfang]

Summary

For these loads that write to the HI part of a register, we should chain them to the op that writes to the LO part
of the register to maintain the appropriate order.

[rampitec]

Can you add a test where low half does not produce a chain? An arithmetic operation and an undef.

lib/Target/AMDGPU/SIISelLowering.cpp

9027

4 is not enough, it will not be a small vector. I think 16 is ok.

[arsenm]

This problem isn't limited to private address space. This should have tests for every address space, and with cases using unrelated bases

[cfang]

Update based on the comments from the reviewers:

1. add test that the lower half op does not have a chain, so we don't adjust the chain;
2. extend to every address space;
3. add tests for unrelated bases;
4. set the size of the small vector for Ops to 16.

[cfang]

In D56454#1350293, @rampitec wrote:

Can you add a test where low half does not produce a chain? An arithmetic operation and an undef.

Tests added. Thanks.

[cfang]

In D56454#1350348, @arsenm wrote:

This problem isn't limited to private address space. This should have tests for every address space, and with cases using unrelated bases

Done for other other address spaces. Thanks.

[rampitec]

test/CodeGen/AMDGPU/chain-hi-to-lo.ll

2

Could you use -check-prefix=GCN ?

133

addrspace(0) is not needed.

[cfang]

test/CodeGen/AMDGPU/chain-hi-to-lo.ll

2

sure, thanks.

133

will remove it. Thanks.

[cfang]

Update based on reviewer's suggestions:

1. -check-prefix=GCN
2. addrspace(0) is not needed.

[rampitec]

LGTM

lib/Target/AMDGPU/SIISelLowering.cpp

 
     SmallVector<SDValue, 4> Ops = { Src0, Src1, Src2 };
     for (unsigned I = 3, N = Node->getNumOperands(); I != N; ++I)
       Ops.push_back(Node->getOperand(I));
 
     Ops.push_back(ImpDef.getValue(1));
     return DAG.getMachineNode(Opcode, SDLoc(Node), Node->getVTList(), Ops);
   }
+  case AMDGPU::BUFFER_LOAD_SHORT_D16_HI_OFFSET:
+  case AMDGPU::BUFFER_LOAD_UBYTE_D16_HI_OFFSET:
+  case AMDGPU::BUFFER_LOAD_SBYTE_D16_HI_OFFSET:
+  case AMDGPU::BUFFER_LOAD_SHORT_D16_HI_OFFEN:
+  case AMDGPU::BUFFER_LOAD_UBYTE_D16_HI_OFFEN:
+  case AMDGPU::BUFFER_LOAD_SBYTE_D16_HI_OFFEN: {
+    // For these loads that write to the HI part of a register,
+    // we should chain them to the op that writes to the LO part
+    // of the register to maintain the order.
+    unsigned NumOps = Node->getNumOperands();
+    SDValue OldChain = Node->getOperand(NumOps-1);
+
+    if (OldChain.getValueType() != MVT::Other)
+      break;
+
+    // Look for the chain to replace to.
+    SDValue Lo = Node->getOperand(NumOps-2);
+    SDNode *LoNode = Lo.getNode();
+    if (LoNode->getNumValues() == 1 ||
+        LoNode->getValueType(LoNode->getNumValues() - 1) != MVT::Other)
+      break;
+
+    SDValue NewChain = Lo.getValue(LoNode->getNumValues() - 1);
+    if (NewChain == OldChain) // Already replaced.
+      break;
+
+    SmallVector<SDValue, 4> Ops;
+    for (unsigned I = 0; I < NumOps-1; ++I)
+      Ops.push_back(Node->getOperand(I));
+    // Repalce the Chain.
+    Ops.push_back(NewChain);
+    MachineSDNode *NewNode = DAG.getMachineNode(Opcode, SDLoc(Node),
+                                                Node->getVTList(), Ops);
+    DAG.setNodeMemRefs(NewNode, Node->memoperands());
+    return NewNode;
+  }
   default:
     break;
   }
 
   return Node;
 }
 
 /// Assign the register class depending on the number of

test/CodeGen/AMDGPU/chain-hi-to-lo.ll

+; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN %s
+
+; GCN-LABEL: {{^}}chain_hi_to_lo:
+; GCN: buffer_load_ushort [[DST:v[0-9]+]], off, [[RSRC:s\[[0-9]+:[0-9]+\]]], [[SOFF:s[0-9]+]] offset:2
+; GCN-NEXT: s_waitcnt vmcnt(0)
+; GCN-NEXT: buffer_load_short_d16_hi [[DST]], off, [[RSRC]], [[SOFF]]
+
+define amdgpu_kernel void @chain_hi_to_lo() {
+bb:
+  %loads = load <2 x half>, <2 x half> addrspace(5)* null, align 2
+  %shuffled = shufflevector <2 x half> %loads, <2 x half> undef, <2 x i32> <i32 1, i32 0>
+  br label %bb1
+
+bb1:
+  call void asm sideeffect "; use $0", "v"(<2 x half> %shuffled)
+  br label %bb1
+}
+