[PATCH] D38906: AMDGPU/SI: Implement d16 support for buffer intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3333
 
+static SDValue lowerIntrinsicWChain(SDValue Op, EVT EquivResultT,
+                                    SelectionDAG &DAG) {
----------------
cfang wrote:
> arsenm wrote:
> > I'm confused because all of the intrinsic logic isn't contained here, and this also just always uses the d16 version which is wrong
> >SDValue Res = lowerIntrinsicWChain(Op, EquivLoadVT, DAG)
> 
> This is just a help function to replace the result type of "Op" with " EquivLoadVT".  maybe we should use a different name to avoid confusion.
> 
> What about: ReplaceResultType ? Thanks. 
The point of this is to move all of the intrinsic lowering logic to one function. The type change wrapper may be a useful function used by the function called from the switch. Part of the point is to reduce the amount of code nested in switches


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:581-601
+  case Intrinsic::amdgcn_buffer_load:
+  case Intrinsic::amdgcn_buffer_load_format:
+  case Intrinsic::amdgcn_tbuffer_load: {
+    Info.opc = ISD::INTRINSIC_W_CHAIN;
+    Info.memVT = MVT::getVT(CI.getType());
+    Info.ptrVal = nullptr;
+    Info.align = 0;
----------------
arsenm wrote:
> This part should be superseded by D41470
Also it's unrelated because this could be done separately for the existing intrinsics, as is done in D41470


https://reviews.llvm.org/D38906