[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/SIISelLowering.cpp:3491-3569
+  // Basic sample.
+  case Intrinsic::amdgcn_image_sample:
+  case Intrinsic::amdgcn_image_sample_cl:
+  case Intrinsic::amdgcn_image_sample_d:
+  case Intrinsic::amdgcn_image_sample_d_cl:
+  case Intrinsic::amdgcn_image_sample_l:
+  case Intrinsic::amdgcn_image_sample_b:
----------------
nhaehnle wrote:
> This is pretty terrible :/
> 
> I'm sorry I don't have anything more constructive to say about it yet (it's a pity that TableGen is so difficult to work with), but it needs to be said.
If we didn't have to deal with the unpacked register layout mistake, we wouldn't need to do this. There's no way to create an InstrMapping type thing for other types.

Although I'm confused why I don't see the type conversion here?  It's there for the stores, but the load should also need to re-pack the results


https://reviews.llvm.org/D39912