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

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added inline comments.


================
Comment at: llvm/docs/LangRef.rst:15589
+
+The result produced is a floating point value.
+
----------------
This sounds incomplete. I think some of the information from the arguments section ought to be here instead.


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:481
+; NO-FMA-LABEL: uifdi:
+; NO-FMA:        cvtsi2sd
+;
----------------
I feel like this should also be checking for the mov instruction, which implicitly zero-extends the 32-bit input. Maybe even a comment explaining what happened, because just looking at the test and seeing that **u**itofp gets lowered to cvt**s**i2sd looks wrong.


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:499
+; NO-FMA-NEXT:    unpckhpd
+; NO-FMA-NEXT:    addpd
+;
----------------
This sequence is quite opaque. I'm not sure it's the best way of handling this case, though I realize that's unrelated to your patch. What is related to your patch is the question of whether the subpd here might be introducing a spurious exception. I'd need to figure out what this is doing to answer that question.


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:530
+; NO-FMA:         cvtsi2ss
+; NO-FMA:         cvtsi2ss
+;
----------------
Some more context would be useful here. I believe there is a test a jump and an addss that are also relevant in the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69275