[PATCH] D39912: AMDGPU/SI: Implement d16 support for image intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/MIMGInstructions.td:181
   let DisableDecoder = DisableSIDecoder;
+  let d16 = 0;
 }
----------------
This should be the default anyway, so you shouldn't need to 0 it again


================
Comment at: lib/Target/AMDGPU/MIMGInstructions.td:322-331
+    def _V1 : MIMG_Gather_Helper <op, asm, dst_rc, VGPR_32, wqm>,
+              MIMG_Mask<asm#"_V1", channels>;
+    def _V2 : MIMG_Gather_Helper <op, asm, dst_rc, VReg_64, wqm>,
+              MIMG_Mask<asm#"_V2", channels>;
+    def _V4 : MIMG_Gather_Helper <op, asm, dst_rc, VReg_128, wqm>,
+              MIMG_Mask<asm#"_V4", channels>;
+    def _V8 : MIMG_Gather_Helper <op, asm, dst_rc, VReg_256, wqm>,
----------------
These are all the same within each block, so you can introduce another multiclass rather than repeating the same combintions. Same for the other MIMG types


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3507
 
+static unsigned getImageOpcode (unsigned IID) {
+  switch (IID) {
----------------
Extra space before (


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3508-3509
+static unsigned getImageOpcode (unsigned IID) {
+  switch (IID) {
+    case Intrinsic::amdgcn_image_load :
+      return AMDGPUISD::IMAGE_LOAD;
----------------
Indentation off


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3658
+
+    default: break;
+  }
----------------
newline


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3742
+    };
+    unsigned Opc = getImageOpcode (IID);
+    Res = DAG.getMemIntrinsicNode(Opc, DL, VTList, Ops, M->getMemoryVT(),
----------------
Extra space before (


https://reviews.llvm.org/D39912