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

[Chih-Hung Hsieh via llvm-commits]

chh added inline comments.

================
Comment at: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8728-8740
@@ -8725,1 +8727,15 @@
+  // copysign(x, fp_round(y)) -> copysign(x, y)
+  // Do not optimize out type conversion of f128 type yet.
+  // For some target like x86_64, configuration is changed
+  // to keep one f128 value in one SSE register, but
+  // instruction selection cannot handle FCOPYSIGN on
+  // SSE registers yet.
+  SDValue N1 = N->getOperand(1);
+  EVT N1VT = N1->getValueType(0);
+  EVT N1Op0VT = N1->getOperand(0)->getValueType(0);
+  return (N1.getOpcode() == ISD::FP_EXTEND ||
+          N1.getOpcode() == ISD::FP_ROUND) &&
+         (N1VT == N1Op0VT || N1Op0VT != MVT::f128);
+}
+
 SDValue DAGCombiner::visitFCOPYSIGN(SDNode *N) {
----------------
spatel wrote:
> Why is an x86-specific limitation being handled in DAGCombiner? Maybe no target supports f128, so it doesn't matter, but this seems wrong.
> 
I thought this optimization won't work for f128 given that current
llvm targets I know do not keep f128 in one register and have not defined
required instruction selection patterns. Instead of keeping this
optimization here for all targets and disable it in a target specific
way, it seems simpler to disable it here for f128 and enable such optimization
for any target later.

That being said, I didn't check all ad-hoc llvm floating point optimizations.
If my assumption is wrong or if there is a simpler way to handle this,
please feel free to suggest any different implementation.
Thanks.



Repository:
  rL LLVM

http://reviews.llvm.org/D15134