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

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 27 08:11:53 PDT 2017


hfinkel added inline comments.


================
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).
+
----------------
wristow wrote:
> hfinkel wrote:
> > andrew.w.kaylor wrote:
> > > wristow wrote:
> > > > wristow wrote:
> > > > > hfinkel wrote:
> > > > > > 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)."
> > > > > For context, the original suggestion of the flag-name 'libm' was changed to 'trans' based on an observation that OpenCL has an option '-cl-fast-relaxed-math' that includes the semantics //"This option also relaxes the precision of commonly used math functions"// (see http://man.opencl.org/clCompileProgram.html), and so 'libm' may incorrectly imply it only applies to a more limited set of functions (math operations) //only //implemented in "libm.a".  That led to the suggestion of 'trans', for transcendental functions.  I originally liked the 'trans' suggestion, moving away from the libm.a implication.  But I do feel that sqrt() should also be optimized on this flag, so as people said, 'trans' isn't perfect either.
> > > > > 
> > > > > I don't have a precise formal definition of what things would be controlled by this, but loosely, I'd say it's "All mathematical operations that have runtime library support on many platforms."  In practice in LLVM, I think this means many (most?) of the math operations handled by "SimplifyLibCalls.cpp".  So for me, in addition to transcendental functions, it would include sqrt(), and even simpler things like fmin() and fmax().
> > > > > 
> > > > > So after giving it more thought, I'm now back to preferring 'libm' over 'trans'.  If someone has a better suggestion, that would be great.  But I haven't thought of one.
> > > > >So after giving it more thought, I'm now back to preferring 'libm' over 'trans'. If someone has a better suggestion, that would be great. But I haven't thought of one.
> > > > 
> > > > In the spirit of the flag 'arcp' for 'AllowReciprocal', and the possibility of 'AllowMathLib' for the internal enumeration name, how about 'amlib'?
> > > > 
> > > > With that, there's no direct implication of "this is only for libm.a operations", or "this is only for transcendental functions".  That said, the 'am' part does give it an awkward "morning library" feeling.  Maybe 'amathlib' instead?
> > > It seems that we're allowing something kind of open-ended here.  That is, we don't seem to have an exact set of functions that will be covered.  If that's the case then we should probably document it as such -- something like "allow substitution of approximate calculations for functions whose meaning are recognized by the optimizer."  And maybe the flag could be "approx".
> > I like this suggestion.
> Yes, I think it's open-ended.  And I very much like the description "allow substitution of approximate calculations for functions whose meaning are recognized by the optimizer.".
> 
> I'm less enthusiastic about the flag-name 'approx', although I'm not horribly opposed to it (especially since I cannot come up with anything I really like).  On it's own, a flag named 'approx' sounds too wide of scope.  To me, it sounds like it might be describing //all //of what is enabled by -ffast-math.  In short, it doesn't explicitly convey the concept of it being approximate calculations for //functions //whose meanings are recognized.  To describe my concern "from the other direction", the reciprocal transformation is also an approximation (as is virtually everything else enabled by fast-math), and we don't intend to control the reciprocal transformation via this flag.
We could call it funcapprox or approxfunc or something like that. We could use something shorter too (apfn perhaps).


https://reviews.llvm.org/D39304





More information about the llvm-commits mailing list