[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

I believe your analysis on the second point is unfortunately missing half the problem.  Functions like `fesetround` will be treated as having arbitrary side-effects by default, which mean arbitrary code can't be reordered with calls to them.  It'd be good to model that more precisely — to flag that the *only* side-effect they have is changing the FP state — but that's not really the big problem; that's just enabling better optimization by e.g. allowing them to be reordered with non-FP code.  The big problem is that intrinsic calls are not arbitrary code: the vast majority of intrinsics (e.g. all the ARM vector intrinsics, many of which can be floating-point) are marked `IntrNoMem` (which I believe corresponds to `readnone`), which means calls to those intrinsics can be reordered with other code whether or not that code has arbitrary side-effects.  It's good that people are looking at achieving better modeling for the x86 backend, but we need to have a plan that doesn't require heroic effort just to get basic correctness.

I would suggest that we need a function/call attribute roughly on the level of `readonly` / `readnone`, maybe `readfponly`, that says that a function has no side-effects and no dependencies on anything *except* the FP state.  Basic queries like `Instruction::mayReadMemory()` that are supposed to be used generically in code-motion transforms would then return true for calls marked that way only if they're FP-constrained functions.  So outside of an FP-constrained function we'd preserve optimizability, and inside one we'd be appropriately conservative.  The generic backend could similarly just default to treating those intrinsic calls as having side-effects in FP-constrained functions, and there'd just be some way for a backend to opt out of that when it provides precise modeling of FP state.  It'd then be a fairly straightforward change to go through the target intrinsic tables and mark which ones have dependencies on FP state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731