[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

Thu Nov 21 12:22:21 PST 2019

bkramer added a comment.

In D70488#1753897 <https://reviews.llvm.org/D70488#1753897>, @mcberg2017 wrote:

> For us this would be an impediment as we have math models that want ieee behavior while relaxing precision.  Adding nnan or ninf would obstruct those choices.

Mind elaborating why nnan/ninf are problematic for you? They're supposed to be a hint to the optimizer and can be dropped any time.

In D70488#1753832 <https://reviews.llvm.org/D70488#1753832>, @spatel wrote:

> I like the idea, but I'd be more comfortable reviewing the diffs in stages, so we know that the test coverage for the value tracking calls is good. So I'd prefer if we split this somehow - either by the opcode callers (fadd, fsub, fmul...) or the the FMF analysis (nnan, nsz, ninf). That raises a few questions:
>
> 1. Why aren't fdiv and frem included?

We currently cannot infer anything for fdiv/frem in isKnownNeverNaN/Inf so there's no way to test it.

> 2. Can we infer FMF for FP intrinsics/libcalls/select/phi? (follow-on patches)

Yeah, that's a logical followup

> 3. We're moving away from FMF on fcmp (recent step: rGebf9bf2cbc8f <https://reviews.llvm.org/rGebf9bf2cbc8fa68d536e481e370c4ba40ce61a8a>), so is it worth including starting from fcmp, or can we wait for that part to settle? (Side question may be if/when we're going to allow FMF on fptrunc/fpextend).

I'll drop fcmp then and split this up once we know that it's actually a direction we want to pursue.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70488