[PATCH] D70214: [X86] Add custom type legalization and lowering for scalar STRICT_FP_TO_SINT/UINT

[Craig Topper via Phabricator via llvm-commits]

craig.topper marked 4 inline comments as done.
craig.topper added inline comments.


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:714
 ; SSE:       # %bb.0: # %entry
+; SSE-NEXT:    cvttsd2si %xmm0, %rax
+; SSE-NEXT:    # kill: def $eax killed $eax killed $rax
----------------
andrew.w.kaylor wrote:
> This is wrong for the strict case. If the converted value doesn't fit in the destination range, the invalid flag is supposed to be raised. Likewise with the AVX case below. Possibly also the x87 case above.
Agreed, but the only way I see to fix this is to add an IR pass to insert the checks before SelectionDAG.


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:716
+; SSE-NEXT:    # kill: def $eax killed $eax killed $rax
+; SSE-NEXT:    retq
+;
----------------
andrew.w.kaylor wrote:
> Why is this retq?
retq just indicates the size of the return address to pop. In 64-bit mode its always retq.


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:740
+; X87:       # %bb.0: # %entry
+; X87-NEXT:    subl $20, %esp
+; X87-NEXT:    .cfi_def_cfa_offset 24
----------------
andrew.w.kaylor wrote:
> I don't think this sequence or the SSE and AVX sequences are correct for strictfp.
Agreed, but I think fixing that is out of scope for this patch. I don't have any good ideas for how to fix it other than to introduce an IR pass to insert control flow before SelectionDAG.


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

https://reviews.llvm.org/D70214