AMDGPU/SI: Fix LowerParameter() for i16 arguments

[• tstellarAMD]

Summary

If we are loading an i16 value from a 32-bit memory location, then
we need to be able to truncate the loaded value to i16.

[arsenm]

lib/Target/AMDGPU/SIISelLowering.cpp

587–592

When is the trunc case necessary? Since the FP case only needs to handle extend this looks weird to me

[• tstellarAMD]

lib/Target/AMDGPU/SIISelLowering.cpp

587–592

Trunc is required when VT is MVT::i16 and MemVT is MVT::i32.

[• tstellarAMD]

lib/Target/AMDGPU/SIISelLowering.cpp

587–592

This happens for mesa, because 16-bit kernel arguments are stored as 32-bit values in the kernarg buffer.

[arsenm]

lib/Target/AMDGPU/SIISelLowering.cpp

587–592

So shouldn't it have the same problem with f16 and need to fptrunc it?

[• tstellarAMD]

lib/Target/AMDGPU/SIISelLowering.cpp

587–592

No, because f16 values are always 16-bits in memory. i16/i8 are just a special case when using the mesa ABI, since those areguments are sign/zero extended by the runtime.

[arsenm]

LGTM

lib/Target/AMDGPU/SIISelLowering.cpp

SDValue SITargetLowering::LowerParameterPtr(SelectionDAG &DAG,
   return DAG.getNode(ISD::ADD, SL, PtrVT, BasePtr,
                      DAG.getConstant(Offset, SL, PtrVT));
 }
 SDValue SITargetLowering::LowerParameter(SelectionDAG &DAG, EVT VT, EVT MemVT,
                                          const SDLoc &SL, SDValue Chain,
                                          unsigned Offset, bool Signed) const {
   const DataLayout &DL = DAG.getDataLayout();
   Type *Ty = VT.getTypeForEVT(*DAG.getContext());
-  MVT PtrVT = getPointerTy(DL, AMDGPUAS::CONSTANT_ADDRESS);
   PointerType *PtrTy = PointerType::get(Ty, AMDGPUAS::CONSTANT_ADDRESS);
-  SDValue PtrOffset = DAG.getUNDEF(PtrVT);
   MachinePointerInfo PtrInfo(UndefValue::get(PtrTy));
 
   unsigned Align = DL.getABITypeAlignment(Ty);
 
-  ISD::LoadExtType ExtTy = Signed ? ISD::SEXTLOAD : ISD::ZEXTLOAD;
-  if (MemVT.isFloatingPoint())
-    ExtTy = ISD::EXTLOAD;
-
   SDValue Ptr = LowerParameterPtr(DAG, SL, Chain, Offset);
-  return DAG.getLoad(ISD::UNINDEXED, ExtTy, VT, SL, Chain, Ptr, PtrOffset,
-                     PtrInfo, MemVT, Align,
+  SDValue Load = DAG.getLoad(MemVT, SL, Chain, Ptr, PtrInfo, Align,
                              MachineMemOperand::MONonTemporal |
                              MachineMemOperand::MODereferenceable |
                              MachineMemOperand::MOInvariant);
+
+  SDValue Val;
+  if (MemVT.isFloatingPoint())
+    Val = DAG.getNode(ISD::FP_EXTEND, SL, VT, Load);
+  else if (Signed)
+    Val = DAG.getSExtOrTrunc(Load, SL, VT);
+  else
+    Val = DAG.getZExtOrTrunc(Load, SL, VT);
+
+  SDValue Ops[] = {
+    Val,
+    Load.getValue(1)
+  };
+
+  return DAG.getMergeValues(Ops, SL);
 }
 
 SDValue SITargetLowering::LowerFormalArguments(
     SDValue Chain, CallingConv::ID CallConv, bool isVarArg,
     const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &DL,
     SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const {
   const SIRegisterInfo *TRI = getSubtarget()->getRegisterInfo();
 

test/CodeGen/AMDGPU/merge-stores.ll

define void @merge_global_store_4_constants_f32(float addrspace(1)* %out) #0 {
 
   store float 1.0, float addrspace(1)* %out.gep.1
   store float 2.0, float addrspace(1)* %out.gep.2
   store float 4.0, float addrspace(1)* %out.gep.3
   store float 8.0, float addrspace(1)* %out
   ret void
 }
 
-; FIXME: Should be able to merge this
 ; GCN-LABEL: {{^}}merge_global_store_4_constants_mixed_i32_f32:
-; GCN-NOAA: buffer_store_dword v
-; GCN-NOAA: buffer_store_dword v
-; GCN-NOAA: buffer_store_dword v
-; GCN-NOAA: buffer_store_dword v
+; GCN-NOAA: buffer_store_dwordx4 v
 
 ; GCN-AA: buffer_store_dwordx2
 ; GCN-AA: buffer_store_dword v
 ; GCN-AA: buffer_store_dword v
 
 ; GCN: s_endpgm
 define void @merge_global_store_4_constants_mixed_i32_f32(float addrspace(1)* %out) #0 {
   %out.gep.1 = getelementptr float, float addrspace(1)* %out, i32 1