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

[Changpeng Fang via Phabricator via llvm-commits]

cfang 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:
----------------
arsenm wrote:
> 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
The type manipulation is done in the buffer d16 support patch (common with buffer_load_format and tbuffer_load_format). 

The idea is, (v4f16 as example), "load" to v4i32 --> truncate to v4i16, then bitcast back to v4f16.


https://reviews.llvm.org/D39912