[PATCH] D69275: Add constrained int->FP intrinsics

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 2 09:12:54 PST 2019


craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7349
+    if (N1.getValueType() == VT)
+      return N1; // noop conversion.
+    break;
----------------
kpn wrote:
> craig.topper wrote:
> > This only works if N2 is result 1 of N1.
> I don't understand. N2 is a constant that is either 0 or 1. What will happen if it is discarded here?
> 
> This code was lifted straight out of getNode() somewhere around line 5194. Without it the X86 target dies trying to lower a rounding of f64 to f64. This happens because getStrictFPExtendOrRound() returns a round when input and output are the same size. This mirrors the non-strict getFPExtendOrRound().
Oops you’re right about N2. But there is an issue with the chain. Which I think is what I was thinking N2 was when I wrote that. If the input chain didn’t come from N1 this is broken.

I don’t see any precedent for returning a MERGE_VALUES from getNode. I think we need to fix the caller of getStrictFPExtendOrRound to only call when necessary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69275/new/

https://reviews.llvm.org/D69275





More information about the llvm-commits mailing list