[PATCH] D39304: [IR] redefine 'reassoc' fast-math-flag and add 'trans' fast-math-flag

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

I don't see a lot of value in having:

  // TODO: This should use hasAllowReassoc()?

before every call to isFast. I can grep for isFast more easily than I can grep for the comment. Moreover, if something really needs 'isFast' because the transformation really needs "every possible liberty", then there should definitely be a comment explaining that.

Also, unless we make hasAllowReassoc() also imply hasNoSignedZeros() (as GCC does, FWIW), we'll almost certainly need that check as well in many places. Many need NoNans and NoInfs too.



================
Comment at: docs/LangRef.rst:2283
+   Allow operations to be simplified assuming relaxed precision for
+   transcendental functions usually found in math libraries (sin, log, etc).
+
----------------
spatel wrote:
> efriedma wrote:
> > Could you mark up the LLVM intrinsics affected by the "trans" flag with the meaning with/without the flag?
> > 
> > Please clarify the language here to indicate this affects the semantics of both LLVM intrinsics and known library functions.  And include a more complete description of what counts as a known library function. And explain what "relaxed" means, given that libm generally doesn't provide correctly rounded versions of transcendental functions.
> > 
> > Also, do we want to optimize sqrt() based on this flag?  It technically isn't transcendental, but we currently generate an approximation in some cases based on fast-math flags.
> I think we'd include sqrt() in the 'trans' bucket (so maybe 'libm' was the better name). But looking back through the dev thread, I don't see an actual definition of that term or what this flag would map to as a clang command-line param. @hfinkel / @wristow - suggestions?
You're correct, sqrt is not a  transcendental function (it is an algebraic function because it is the root of a polynomial equation). The problem is that algebraic functions also include things that we don't want to include here (e.g., addition, division). I don't like 'libm' because it refers to a very specific set of functions, but maybe that's the best we can do. The best description I have of this set is: "All of the mathematical operations that generally produce irrational numbers and are not in the set of functions specified by the IEEE specification (e.g., +,-,*,/,%,sqrt)."


https://reviews.llvm.org/D39304