[PATCH] D67105: [TargetLowering] Fix another potential FPE in expandFP_TO_UINT

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand created this revision.
uweigand added reviewers: RKSimon, efriedma, atanasyan, craig.topper, spatel, kpn.
Herald added subscribers: llvm-commits, dexonsmith, mehdi_amini.
Herald added a project: LLVM.

D53794 <https://reviews.llvm.org/D53794> introduced code to perform the FP_TO_UINT expansion via FP_TO_SINT in a way that would never expose floating-point exceptions in the intermediate steps.  Unfortunately, I just noticed there is still a way this can happen.  As discussed in D53794 <https://reviews.llvm.org/D53794>, the compiler now generates this sequence:

  // Sel = Src < 0x8000000000000000 
  // Val = select Sel, Src, Src - 0x8000000000000000 
  // Ofs = select Sel, 0, 0x8000000000000000 
  // Result = fp_to_sint(Val) ^ Ofs

The problem is with the Src - 0x8000000000000000 expression.  As I mentioned in the original review, that expression can never overflow or underflow if the original value is in range for FP_TO_UINT.  But I missed that we **can** get an Inexact exception in the case where Src is a very small positive value.  (In this case the result of the sub is ignored, but that doesn't help.)

Instead, I'd suggest to use the following sequence:

  // Sel = Src < 0x8000000000000000 
  // FltOfs = select Sel, 0, 0x8000000000000000
  // IntOfs = select Sel, 0, 0x8000000000000000
  // Result = fp_to_sint(Val - FltOfs) ^ IntOfs

In the case where the value is already in range of FP_TO_SINT, we now simply compute Val - 0, which now definitely cannot trap (unless Val is a NaN in which case we'd want to trap anyway).

In the case where the value is **not** in range of FP_TO_SINT, but still in range of FP_TO_UINT, the sub can never be inexact, as Val is between 2^(n-1) and (2^n)-1, i.e. always has the 2^(n-1) bit set, and the sub is always simply clearing that bit.

There is a slight complication in the case where Val is a constant, so we know at compile time whether Sel is true or false.  In that scenario, the old code would automatically optimize the sub away, while this no longer happens with the new code.  Instead, I've added extra code to check for this case and then just fall back to FP_TO_SINT directly.  (This seems to catch even slightly more cases.)


Repository:
  rL LLVM

https://reviews.llvm.org/D67105

Files:
  lib/CodeGen/SelectionDAG/TargetLowering.cpp
  test/CodeGen/SystemZ/fp-strict-conv-10.ll
  test/CodeGen/SystemZ/fp-strict-conv-12.ll
  test/CodeGen/X86/fp-intrinsics.ll
  test/CodeGen/X86/vector-constrained-fp-intrinsics.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67105.218464.patch
Type: text/x-patch
Size: 12926 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190903/ae5e340a/attachment.bin>