[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/DAGCombiner.cpp:8776
@@ -8769,1 +8775,3 @@
+  if ((N1.getOpcode() == ISD::FP_EXTEND || N1.getOpcode() == ISD::FP_ROUND) &&
+      (N1VT == N1Op0VT || N1Op0VT != MVT::f128))
     return DAG.getNode(ISD::FCOPYSIGN, SDLoc(N), VT,
----------------
Move the comment and additional check into a small helper for readability:

if ( (.... || ...) && legalToSimplifyCopysign(N1))

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:122
@@ +121,3 @@
+  // assuming that the operands are also converted when necessary.
+  // Otherwise, return false to tell caller to scan operands.
+  return R.getNode() && R.getNode() != N;
----------------
the caller.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:775
@@ +774,3 @@
+  if (!isLegalInHWReg(N->getOperand(OpNo).getValueType()))
+    return false;
+  // When the operand type can be kept in registers, SoftenFloatResult
----------------
llvm_unreachable? When this function is called, isLegalInHWReg must be true.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:400
@@ +399,3 @@
+    if (!SoftenedOp.getNode() &&
+        isSimpleLegalType(Op.getValueType()))
+      return Op;
----------------
if (!SoftenedOp.getNode()) {
   assert(isSimpleLegalType(...));  // or should it be assert(isLegalInHWReg(...)) ?
   return Op;
}

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:414
@@ +413,3 @@
+    // by calling ReplaceValueWith here to update all users.
+    if (NewRes.getNode() != N && isLegalInHWReg(N->getValueType(ResNo)))
+      ReplaceValueWith(SDValue(N, ResNo), NewRes);
----------------
It is clearer to do:

  if (!isLegal...)
      return;

  if (...) 

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:463
@@ +462,3 @@
+  // Return true if we can skip softening the given operand or SDNode because
+  // it was soften before by SoftenFloatResult and references to the operand
+  // were replaced by ReplaceValueWith.
----------------
soften -> softened

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeTypes.h:464
@@ +463,3 @@
+  // it was soften before by SoftenFloatResult and references to the operand
+  // were replaced by ReplaceValueWith.
+  bool SkipSoftenFloatOperand(SDNode *N, unsigned OpNo);
----------------
because the operand has already been softened in SoftenFloatResult with calls to ReplaceValueWith.


http://reviews.llvm.org/D15134