[PATCH] D18049: AMDGPU/SI: Make i16 a legal type for VI subtargets

[Konstantin Zhuravlyov via llvm-commits]

kzhuravl added a comment.

LGTM overall with minor fixes:



================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:600
+  if (DestSize== 16 &&
+      Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
+    return SrcSize >= 32;
----------------
Subtarget->has16BitInsts()


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:611
+  if (SrcSize == 16 &&
+      Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
+    return DestSize >= 32;
----------------
Subtarget->has16BitInsts()


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:2369
 
+  //there are i16 integer mul/mad
+  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS &&
----------------
Space after //. Capitalize.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:2370
+  //there are i16 integer mul/mad
+  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS &&
+      VT.getScalarType().bitsLE(MVT::i16) )
----------------
Subtarget->has16BitInsts()


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:37
 #include "llvm/IR/Function.h"
+#include "llvm/ADT/SmallString.h"
 
----------------
Alphabetize


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:82
 
+  // TODO: Subtarget feature for i16
+  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
----------------
I do not think we need this comment


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:83
+  // TODO: Subtarget feature for i16
+  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
+    addRegisterClass(MVT::i16, &AMDGPU::SReg_32RegClass);
----------------
Subtarget->has16BitInsts()


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:229
 
+  if (Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS) {
+    setOperationAction(ISD::Constant, MVT::i16, Legal);
----------------
Subtarget->has16BitInsts()


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:273
+    setOperationAction(ISD::UINT_TO_FP, MVT::i16, Promote);
+	AddPromotedToType(ISD::UINT_TO_FP, MVT::i16, MVT::i32);
+    setOperationAction(ISD::SINT_TO_FP, MVT::i16, Promote);
----------------
Detabify


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:275
+    setOperationAction(ISD::SINT_TO_FP, MVT::i16, Promote);
+	AddPromotedToType(ISD::SINT_TO_FP, MVT::i16, MVT::i32);
+    setOperationAction(ISD::FP16_TO_FP, MVT::i16, Promote);
----------------
Detabify


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:277
+    setOperationAction(ISD::FP16_TO_FP, MVT::i16, Promote);
+	AddPromotedToType(ISD::FP16_TO_FP, MVT::i16, MVT::i32);
+    setOperationAction(ISD::FP_TO_FP16, MVT::i16, Promote);
----------------
Detabify


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:279
+    setOperationAction(ISD::FP_TO_FP16, MVT::i16, Promote);
+	AddPromotedToType(ISD::FP_TO_FP16, MVT::i16, MVT::i32);
+
----------------
Detabify


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:280
+	AddPromotedToType(ISD::FP_TO_FP16, MVT::i16, MVT::i32);
+
+  }
----------------
Remove extra new line


https://reviews.llvm.org/D18049