[all-commits] [llvm/llvm-project] 28b573: [TargetLowering] Fix another potential FPE in expa...

[topperc via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 28b573d2497e394d54d47778012887ab66d51f6f
      https://github.com/llvm/llvm-project/commit/28b573d2497e394d54d47778012887ab66d51f6f
  Author: Craig Topper <craig.topper at intel.com>
  Date:   2019-12-06 (Fri, 06 Dec 2019)

  Changed paths:
    M llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
    M llvm/lib/Target/X86/X86ISelLowering.cpp
    M llvm/test/CodeGen/SystemZ/fp-strict-conv-10.ll
    M llvm/test/CodeGen/SystemZ/fp-strict-conv-12.ll
    M llvm/test/CodeGen/X86/fp-cvt.ll
    M llvm/test/CodeGen/X86/fp-intrinsics.ll
    M llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
    M llvm/test/CodeGen/X86/vector-constrained-fp-intrinsics.ll

  Log Message:
  -----------
  [TargetLowering] Fix another potential FPE in expandFP_TO_UINT

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, 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.)

Original version of the patch by Ulrich Weigand. X86 changes added by Craig Topper

Differential Revision: https://reviews.llvm.org/D67105