[PATCH] D11438: Fix x86_64 fp128 calling convention

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8589
@@ +8588,3 @@
+  // Do not optimize out type conversion of f128 type yet.
+  // Store and truncate operators for f128 are very limited.
+  EVT N1VT = N1->getValueType(0);
----------------
Can you explain this a little more?

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8593
@@ -8589,1 +8592,3 @@
+  if ((N1.getOpcode() == ISD::FP_EXTEND || N1.getOpcode() == ISD::FP_ROUND) &&
+      (N1VT == N1Op0VT || (N1VT != MVT::f128 && N1Op0VT != MVT::f128)))
     return DAG.getNode(ISD::FCOPYSIGN, SDLoc(N), VT,
----------------
Should it be  ... && (N1VT == N1OpVT || N1OpVT != MVT::f128)?  

================
Comment at: lib/CodeGen/SelectionDAG/InstrEmitter.cpp:169
@@ -164,1 +168,3 @@
+    // If TRI->getCommonSubClass(UseRC, RC) returns some register class,
+    // e.g. FR128, that does not contain VT, it should not be used for DstRC.
     DstRC = UseRC;
----------------
Why hasType(VT) returns false for FR128? Is it expected?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3535
@@ +3534,3 @@
+    ConstantSDNode *CP = cast<ConstantSDNode>(Node);
+    Results.push_back(ExpandConstant(CP));
+    break;
----------------
unconditionally expand? Do you need to add some assertion on the type expected?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:52
@@ -51,2 +51,3 @@
         dbgs() << "\n");
-  SDValue R = SDValue();
+  SDValue R(N, ResNo);
+
----------------
why this change?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:68
@@ +67,3 @@
+      case ISD::SELECT_CC:
+        return false;
+      // On x86_64 FABS, FNEG, and FCOPYSIGN can be simplified to
----------------
Why returning false for some cases and true for others?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:78
@@ +77,3 @@
+        // Mark this node as softened.
+        SetSoftenedFloat(R, R);
+        return true;
----------------
Explain this change?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:145
@@ +144,3 @@
+    // on the ScanOperands in LegalizeTypes.cpp to replace every operand.
+    // But SoftenFloatOperand is not smart enough to replace every operand.
+    // See CodeGen/X86/fp128-abs.ll, fp128-load.ll.
----------------
Should the problem be fixed in SoftenFloatOperand method?


http://reviews.llvm.org/D11438