[PATCH] D11438: Fix x86_64 fp128 calling convention

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:60
@@ +59,3 @@
+  EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), VT);
+  bool KeepFloat = VT == NVT && VT.isSimple() && TLI.isTypeLegal(VT);
+
----------------
KeepFloat does not convey the real meaning. Is is better to call it LegalInHWReg?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:68
@@ +67,3 @@
+      case ISD::BITCAST:
+      case ISD::SELECT:
+      case ISD::SELECT_CC:
----------------
With the handling of CopyFromReg CopyToReg, there does not seem to be a need to special case SELECT, SELECT_CC by briefly looking at the code.

In fact, I think we should push the logic all down to the leaf and get rid of return true/false change.  It is the change of 'flow' and special handling of operand scanning for f128 that makes the current patch look intrusive.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:145
@@ -111,1 +144,3 @@
     SetSoftenedFloat(SDValue(N, ResNo), R);
+    // Some soften sub-methods do not call ReplaceValueWith and depend
+    // on the ScanOperands in LegalizeTypes.cpp to replace every operand.
----------------
Which sub-methods need special handling here?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:159
@@ +158,3 @@
+  assert(KeepFloat && "Unsupported SoftenFloatRes of ISD::Register!");
+  return KeepFloat ? SDValue(N, ResNo) : SDValue();
+}
----------------
Just return SDValue(N, ResNo)?


http://reviews.llvm.org/D11438