[PATCH] D11081: SelectionDAG: Support Expand of f16 extloads

[Ahmed Bougacha via llvm-commits]

ab added a subscriber: ab.
ab accepted this revision.
ab added a reviewer: ab.
ab added a comment.
This revision is now accepted and ready to land.

I'll admit I'm not sure I understand what the test does (%arg is loaded?), but the code seems reasonable.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:882-887
@@ -881,2 +881,8 @@
 
+static EVT toIntegerVT(EVT VT) {
+  if (VT.isVector())
+    return VT.changeVectorElementTypeToInteger();
+  return MVT::getIntegerVT(VT.getSizeInBits());
+}
+
 void SelectionDAGLegalize::LegalizeLoadOps(SDNode *Node) {
----------------
Should this be part of EVT, with a more robust extended handling? Say, "changeTypeToInteger".

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1139
@@ +1138,3 @@
+        // Handle the special case of fp16 extloads. EXTLOAD doesn't have the
+        // normal undefined upper bits behavior to use allow an in-reg extend
+        // with the illegal FP type, so load as an integer and do the
----------------
"use allow" -> "allow using" ?

================
Comment at: test/CodeGen/AMDGPU/half.ll:125-130
@@ -120,7 +124,8 @@
+
 ; GCN-LABEL: {{^}}extload_v2f16_to_v2f64_arg:
 define void @extload_v2f16_to_v2f64_arg(<2 x double> addrspace(1)* %out, <2 x half> %arg) #0 {
   %ext = fpext <2 x half> %arg to <2 x double>
   store <2 x double> %ext, <2 x double> addrspace(1)* %out
   ret void
 }
 
----------------
I assume these functions might go through the same codepath, can you add checks for them?


http://reviews.llvm.org/D11081