[PATCH] D18049: AMDGPU i16 implementation

[Tom Stellard via llvm-commits]

tstellarAMD added inline comments.

================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:264-268
@@ +263,7 @@
+
+    setOperationAction(ISD::ADD, MVT::i16, Legal);
+    setOperationAction(ISD::SUB, MVT::i16, Legal);
+    setOperationAction(ISD::SHL, MVT::i16, Legal);
+    setOperationAction(ISD::SRL, MVT::i16, Legal);
+    setOperationAction(ISD::SRA, MVT::i16, Legal);
+
----------------
Are these really necessary?  I thought making a type legal marked these operations legal by default.

================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:270-276
@@ +269,9 @@
+
+    setOperationAction(ISD::SMIN, MVT::i16, Legal);
+    setOperationAction(ISD::SMAX, MVT::i16, Legal);
+    setOperationAction(ISD::UMIN, MVT::i16, Legal);
+    setOperationAction(ISD::UMAX, MVT::i16, Legal);
+
+    setOperationAction(ISD::SETCC, MVT::i16, Legal);
+    setOperationAction(ISD::TRUNCATE, MVT::i16, Legal);
+
----------------
Same with these too.  Are they really necessary?

================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:1743-1756
@@ -1680,1 +1742,16 @@
 
+  if (MemVT == MVT::i16) {
+    assert(Load->getValueType(0) == MVT::i16);
+
+    SDValue ExtLoad = DAG.getExtLoad(ISD::EXTLOAD, DL, MVT::i32, Load->getChain(),
+                                     Load->getBasePtr(), MVT::i16,
+                                     Load->getMemOperand());
+
+    SDValue Ops[] = {
+      DAG.getNode(ISD::TRUNCATE, DL, MVT::i16, ExtLoad),
+      ExtLoad.getValue(1)
+    };
+
+    return DAG.getMergeValues(Ops, DL);
+  }
+
----------------
Why does i16 needs special handling here.  These seems to be nearly identical to the block of code directly below.

================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:2022-2031
@@ -1944,2 +2021,12 @@
 
+  if (VT == MVT::i16) {
+    SDValue Ext = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i32, Store->getValue());
+
+    return DAG.getTruncStore(Store->getChain(), DL,
+                             Ext,
+                             Store->getBasePtr(),
+                             MVT::i16,
+                             Store->getMemOperand());
+  }
+
   if (VT == MVT::i1) {
----------------
We do we need to custom lower i16 stores?  Can't we just mark then as promote?


Repository:
  rL LLVM

http://reviews.llvm.org/D18049