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

[Changpeng Fang via Phabricator via llvm-commits]

cfang added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:549
+
+static EVT getExtVT (EVT VT) {
+  if (VT == MVT::v2f16)
----------------
arsenm wrote:
> Since you really want to get the i16 equivalent vector, you can use changeTypeToInteger on the f16 type.
For v2f16, I want something that uses 2 registers, so v2i16 doesn't work for me. 
What should be the type that specifies the two registers that hold the two half values? Here I am using v2f32.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3345
+    if (SDValue Res = ChangeResultType(Op, ToVT, DAG)) {
+      if (!HasPacked) { // From v2f32/v4f32 back to v2f16/v4f16.
+        SDValue RFlag = DAG.getTargetConstant(0, SL, MVT::i32);
----------------
arsenm wrote:
> I'm pretty sure there should be no f32 or FP_ROUND here. This is returning the half format result just expanded into multiple 32-bit registers. They are still half, you need to truncate the int type and bitcast to f16.
Do you mean the returned type is not v2f32? so what is the type that returned? v2f16? I just could not understand how to represent the two half values in two registers!  

I hope you can be more specific on what to do here.


https://reviews.llvm.org/D38906