[PATCH] D102066: [AMDGPU] Fix codegen of image intrinsics for g16 and a16

[David Stuttard via Phabricator via llvm-commits]

dstuttard marked 4 inline comments as done.
dstuttard added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1505
+  // A16 implies 16 bit gradients if subtarget doesn't support G16
+  if (IsA16 && !AMDGPU::hasG16(STI) && !IsG16)
     return false;
----------------
foad wrote:
> "STI.hasG16()"
Meh - ok.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:4034
+               (I >= Intr->CoordStart && !IsA16)) {
+      // Handle any non-gradient / non-coordinate operands, plus any gradient
+      // or coordinate operands that should not be packed
----------------
foad wrote:
> Aren't "non-gradient / non-coordinate operands" handled in the "if" above? Also I don't understand why the case above has a bitcast but this case does not. But I see that that was the pre-existing behaviour.
I'm not 100% sure why it needs to do this either. I tried unifying it, but it failed verification, so resorted to this which more closely matches the original implementation.
I've adjusted the comment so it is less confusing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102066/new/

https://reviews.llvm.org/D102066