[PATCH] D15134: Part 1 to fix x86_64 fp128 calling convention.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:52
@@ -51,4 +51,3 @@
         dbgs() << "\n");
-  SDValue R = SDValue();
-
+  SDValue R(N, ResNo);
   switch (N->getOpcode()) {
----------------
Push this change down to cases where No softening is needed.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:73
@@ -68,3 +72,3 @@
     case ISD::EXTRACT_VECTOR_ELT:
-      R = SoftenFloatRes_EXTRACT_VECTOR_ELT(N); break;
-    case ISD::FABS:        R = SoftenFloatRes_FABS(N); break;
+      R = SoftenFloatRes_EXTRACT_VECTOR_ELT(N);
+      break;
----------------
Unrelated format change.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:117
@@ -111,4 +116,3 @@
     SetSoftenedFloat(SDValue(N, ResNo), R);
-}
-
-SDValue DAGTypeLegalizer::SoftenFloatRes_BITCAST(SDNode *N) {
+    // Some soften sub-methods, e.g. those calling TLI.makeLibCall,
+    // do not call ReplaceValueWith and depend on the ScanOperands
----------------
Put those comments and replace code into a small wrapper -- 'replaceValueIfNeeded'.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:734
@@ +733,3 @@
+
+  // When an operand is float but can be kept in register,
+  // many of the SoftenFloatOp_* functions will call GetSoftenedFloat
----------------
This comment seems out of place. Either remove it or put it in better place.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:741
@@ +740,3 @@
+    // When operand type can be kept in registers, op code like CopyToReg,
+    // CopyFromReg, Register, etc. do not need to be softened again.
+    if (LegalInHWReg) {
----------------
Move this comment also into the wrapper function. Explain better why it does not need to be softened again -- e.g, ReplaceValueWith is called eagerly ...

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:742
@@ +741,3 @@
+    // CopyFromReg, Register, etc. do not need to be softened again.
+    if (LegalInHWReg) {
+      switch (N->getOperand(OpNo).getOpcode()) {
----------------
Move this into a helper function with a proper name? Also add comment why only these subset of opcodes can be skipped.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:785
@@ +784,3 @@
+    // Do not try to analyze or soften this node again if the value is
+    // or can be hold in a register. In that case, Res.getNode() should
+    // be euqal to N.
----------------
can be held.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:787
@@ +786,3 @@
+    // be euqal to N.
+    if (Res.getNode() == N && LegalInHWReg)
+      return false;
----------------
Is the first condition needed?  If Res.getNode() != N, ReplaceValueWith should also be called (see below), so this condition does not look correct.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:797
@@ -713,3 +796,3 @@
   // If the result is N, the sub-method updated N in place.  Tell the legalizer
-  // core about this.
+  // core about this to reanalize.
   if (Res.getNode() == N)
----------------
typo re-analyze 

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.cpp:241
@@ -240,4 +240,3 @@
       case TargetLowering::TypeSoftenFloat:
-        SoftenFloatResult(N, i);
-        Changed = true;
-        goto NodeDone;
+        // If a soften float result is converted to integer,
+        // assume the operands are also converted when necessary.
----------------
Move this comment to SoftFloatResult where short cut was taken: 

 ' Return false to tell caller to scan operands ...'

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:391
@@ -380,1 +390,3 @@
+  /// stay in a register, the Op is not converted to an integer.
+  /// In that cast, the given op is returned.
   SDValue GetSoftenedFloat(SDValue Op) {
----------------
In that case,

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:394
@@ -381,1 +393,3 @@
     SDValue &SoftenedOp = SoftenedFloats[Op];
+    if (!SoftenedOp.getNode() &&
+        Op.getValueType().isSimple() &&
----------------
Put it in a helper and add some DEBUG check what kind of Opcode are expected to not 'softened' at this point.


http://reviews.llvm.org/D15134