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

[Changpeng Fang via Phabricator via llvm-commits]

cfang marked 6 inline comments as done.
cfang added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4906-4908
+    SDValue VData = Op.getOperand(2);
+    EVT StoreVT = VData.getValueType();
+    if (isHalfVT(StoreVT)) {
----------------
arsenm wrote:
> Can't you reuse the type convert function from the buffer patch?
Yes. I have used a new function in the buffer patch to do the conversion, and will call that function. Thanks.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:7125
+    //int D16Idx = AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::d16);
+    //if (D16Idx > 0 && MI.getOperand(D16Idx).getImm() != 0)
+    if (TII->isD16(MI))
----------------
arsenm wrote:
> Dead code. Comment isn't helpful, this is a TODO to implement this optimization for d16. Can you open a bug for that?.
> 
> For the unpacked case I would expect it to be trivial to extend for it
Sure I will open a ticket to add similar optimization for d16. You are right for unpacked case. Thanks.


https://reviews.llvm.org/D39912