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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Can you add some assembler tests for this, particularly the 3x components ones



================
Comment at: lib/Target/AMDGPU/BUFInstructions.td:925
+  defm TBUFFER_STORE_FORMAT_D16_XY_gfx80   : MTBUF_Pseudo_Stores <"tbuffer_store_format_d16_xy",   VReg_64>;
+  defm TBUFFER_STORE_FORMAT_D16_XYZ_gfx80  : MTBUF_Pseudo_Stores <"tbuffer_store_format_d16_xyz",  VReg_128>;
+  defm TBUFFER_STORE_FORMAT_D16_XYZW_gfx80 : MTBUF_Pseudo_Stores <"tbuffer_store_format_d16_xyzw", VReg_128>;
----------------
You an use VReg_96


================
Comment at: lib/Target/AMDGPU/BUFInstructions.td:1873
+defm TBUFFER_LOAD_FORMAT_XY    : MTBUF_Real_AllAddr_vi <0x01>;
+//defm TBUFFER_LOAD_FORMAT_XYZ   : MTBUF_Real_AllAddr_vi <0x02>;
+defm TBUFFER_LOAD_FORMAT_XYZW  : MTBUF_Real_AllAddr_vi <0x03>;
----------------
You should be able to define the 3x encoded ones


================
Comment at: lib/Target/AMDGPU/BUFInstructions.td:1881
+defm TBUFFER_LOAD_FORMAT_D16_XY    : MTBUF_Real_AllAddr_vi <0x09>;
+//defm TBUFFER_LOAD_FORMAT_D16_XYZ   : MTBUF_Real_AllAddr_vi <0x0a>;
+defm TBUFFER_LOAD_FORMAT_D16_XYZW  : MTBUF_Real_AllAddr_vi <0x0b>;
----------------
You should be able to define the 3x encoded ones


================
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;
----------------
This part should be superseded by D41470


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3398
+  case ISD::INTRINSIC_W_CHAIN: {
+    SDLoc SL(N);
+    SDValue Op = SDValue(N, 0);
----------------
This should be in a separate function


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4747
+      if (StoreVT.isVector()) {
+        //assert ((StoreVT != MVT::v3f16) && "Handle v3f16");
+        if (!Subtarget->hasUnpackedD16VMem()) {
----------------
Dead code. You need to check the number of elements since there is no v3f16 yet


https://reviews.llvm.org/D38906