[PATCH] D14079: [FPEnv Core 14/14] Introduce F*_W_CHAIN instrs to prevent reordering

[Sergey Dmitrouk via llvm-commits]

sdmitrouk added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4344
@@ +4343,3 @@
+    Results.push_back(DAG.getNode(ISD::FP_ROUND, dl, OVT,
+                                  Tmp3, DAG.getIntPtrConstant(0, dl)));
+    Results.push_back(Tmp3.getValue(1));
----------------
arsenm wrote:
> Using the integer size for address space 0 seems like a poor choice. Do targets actually care about this type? Why isn't it an i1 target constant?
This is a rewrite of code in the case directly above, I just made it aware of the chain.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:208-209
@@ +207,4 @@
+        Op.getValueType() == MVT::Other)
+      // Do not attempt to legalize chain.
+      Ops.push_back(Op.getOperand(0));
+    else
----------------
arsenm wrote:
> Why is this Op.getOperand(0) instead of just Op? I would also move the chain handling out of the loop and handle separately or have a separate w/chain and wo/chain loop
Thanks, it should be `Op`.

> I would also move the chain handling out of the loop and handle separately or have a separate w/chain and wo/chain loop

It's not the chain of the node we're processing, it's reference to the chain of an operand. Node shouldn't be legalized by reference to its chain, which will be replaced with new value when such node gets legalized through reverence to its value.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2182-2186
@@ +2181,7 @@
+  if (NumElts == 1) {
+    SDValue Chain;
+    SDValue Ret = DAG.UnrollVectorOp(N, WidenVT.getVectorNumElements(), &Chain);
+    if (Chain)
+      AnalyzeNewNode(Chain.getNode());
+    return Ret;
+  }
----------------
arsenm wrote:
> A Chain out arguments looks weird here. Why can't UnrollVectorOp's result have the chain result?
> Why can't UnrollVectorOp's result have the chain result?

Because its result is processed is assumed to have type of vector element and it is processed that way.


Repository:
  rL LLVM

http://reviews.llvm.org/D14079