[llvm-dev] Trouble when suppressing a portion of fast-math-transformations

Tue Oct 10 16:49:54 PDT 2017

Hi Sanjay,

> IIUC, we need to audit/fix the current transforms that use FMF to respond to the
> individual flags, and those fixes can occur in parallel. They might cosmetically
> conflict with the redefining/replumbing work on the flags struct, but that shouldn't be
> too hard to fix?

Yes, the overall goal is to audit/fix the current transforms that use FMF to use the updated set of flags, which includes not using the "umbrella" flag 'fast' (eliminating the concept of the umbrella).

I agree with you that this can proceed in parallel with the redefining/replumbing (to deal with the "umbrella" aspect of 'fast', and with the 7-bit limitation of the current implementation that you pointed out in the SubclassOptionalData of Value).  But that said, it feels to me like it's going to get confusing to deal with them in parallel.

I see three aspects of the overall task:

1.  Remove the "umbrella" 'fast' LLVM IR flag, and add two new flags ('reassoc' and 'trans').  In this portion, the 'UnsafeAlgebra' bit in FastMathFlags would be removed, and two new bits would be added (for 'reassoc' and 'trans').  Currently, the concept of 'FastMathFlags::setUnsafeAlgebra()' sets the 'UnsafeAlgebra' bit in FastMathFlags, along with the other 5 bits that already exist ('NoNaNs', 'NoInfs', 'NoSignedZeros', 'AllowReciprocal' and 'AllowContract').  With this change, those other 5 would still be set, and the two new ones for 'reassoc' and 'trans' would also be set.  The method 'FastMathFlags::unsafeAlgebra()' would check that all the bits were on, rather than just the one 'UnsafeAlgebra' bit (which will no longer exist).  A related change would be needed for the handling of 'Value::SubclassOptionalData' (to check for all 7 bits, rather than just the one no-longer-existing 'FastMathFlags::UnsafeAlgebra' bit).

2.  Refactor the implementation to remove the 7-bit limitation.  That is, don't use 'Value::SubclassOptionalData', and instead carry this information via some other approach.  A new member variable of 'Instruction' seems sensible to me.  But I don't feel I have the experience to judge if that's the right way (from an efficiency of implementation perspective).

3.  Audit all the uses of the UnsafeAlgebra concept, and change the ones that don't require all the flags to be enabled to only check the status of the needed subset.

Conceptually, points 1 and 2 are transparent to users.  Point 1 is complicated (to me) since it changes the IR, and so would need some work to deal with that change from a compatibility perspective.  Point 2 is somewhat messy (again, to me).

If (1) and (2) were done, then I think point 3 (which I'd call the actual fast-math-related fixes) could be done in an evolutionary way.  That is, individual examples of incorrect behavior can be analyzed and fixed as separate bugs.  For example, if ‑fno‑reciprocal‑math is suppressing a reassociation, that can be fixed by changing some of the checks for UnsafeAlgebra to check for something more fine-grained (something that doesn't include the reciprocal-math flag).  There's no requirement from my POV that all the fixes along these lines are done together.  (If instead, a thorough audit is desired to get as many issues as are found fixed in one commit, that's fine with me, but doing them in smaller groups is also fine, and seems straightforward.)

Regarding:
> Also, if you'll be at the conference next week, we could find some time for interested
> parties to meet to discuss/hack this more if needed.

Yes, I'll be at the conference.  And I'm definitely interested in participating in discussions and hacking on this.  I'd feel I'd be mostly on the sidelines for points 1 and 2, but I'm still very interested in participating in that aspect.  Others may see it differently than my 3-point approach, above.  In that case, discussing it all face-to-face at the conference would be a good way to make progress.

Thanks,
-Warren

From: Sanjay Patel [mailto:spatel at rotateright.com]
Sent: Monday, October 9, 2017 6:28 AM
To: Ristow, Warren
Cc: llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] Trouble when suppressing a portion of fast-math-transformations

Hi Warren -
IIUC, we need to audit/fix the current transforms that use FMF to respond to the individual flags, and those fixes can occur in parallel. They might cosmetically conflict with the redefining/replumbing work on the flags struct, but that shouldn't be too hard to fix?

Let me know what I can do to help without interfering with any patches you might have in progress. Also, if you'll be at the conference next week, we could find some time for interested parties to meet to discuss/hack this more if needed.

On Wed, Oct 4, 2017 at 3:32 AM, Ristow, Warren via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
> It might be clearer, instead of using 'libm', to use something like 'trans' (for transcendental functions).

That does seem clearer.  ‘trans’ is definitely good with me.

-Warren

From: Hal Finkel [mailto:hfinkel at anl.gov<mailto:hfinkel at anl.gov>]
Sent: Tuesday, October 3, 2017 5:13 PM
To: Ristow, Warren; Bruce Hoult
Cc: llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>

Subject: Re: [llvm-dev] Trouble when suppressing a portion of fast-math-transformations

On 10/03/2017 05:02 AM, Ristow, Warren wrote:
>>> I'd like to emphasise in the latter one: "This option also relaxes the precision of
>>> commonly used math functions."
>>
>> Isn't this the "libm" flag that is proposed in this thread?
>
> I don't know. I didn't see any definition of it.
>
> In my case I'm talking about allowing the use of lower precision but very fast machine
> instructions instead of a slow sequence of inline instructions. But I guess instruction
> vs library is equivalent.

I haven't defined "libm" explicitly.  The concept of "reassoc" and "libm" are a result of the discussion last November.  The "libm" aspect in particular, came from:

http://lists.llvm.org/pipermail/llvm-dev/2016-November/107114.html

It was intended to mean actual library functions, which is what I thought you were referring to when you said "This option also relaxes the precision of commonly used math functions."  From your elaboration describing it as a sequence of inline instructions, I think whether "libm" is the right flag to control it would depend on what that sequence of instructions were doing.  If they were implementing 'pow()' or 'sqrt()' or 'sin()' etc., then yes, I think "libm" would be the right flag.  If something else (unrelated to functions generally in libm), then probably some other flag (or set of flags) would control whether a transformation was done.

More generally, my intended approach of doing this change (of removing the "fast" umbrella flag, and adding the two new flags "reassoc" and "libm"), is to audit all the places that currently enable an optimization based on whether ‘hasUnsafeAlgebra()’ is true (which essentially is asking whether _all_ the existing FastMathFlags are enabled), and see which of them can/should be controlled by one (or a subset of) the full set.  So it's possible that a particular slow sequence of inline instructions you are transforming would be controlled by "libm", and a different slow sequence of inline instructions would be controlled by some other flag (or flags).

I agree. We should give the flags semantic meanings. Whether or not something is generally a function call is irrelevant. libm, if we use that name, refers to the functions in libm, and perhaps close extensions, regardless of how the target generally generates code for them.

It might be clearer, instead of using 'libm', to use something like 'trans' (for transcendental functions).

 -Hal

-Warren

From: bruce.hoult at gmail.com<mailto:bruce.hoult at gmail.com> [mailto:bruce.hoult at gmail.com] On Behalf Of Bruce Hoult
Sent: Tuesday, October 3, 2017 10:05 AM
To: Hal Finkel
Cc: Ristow, Warren; llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] Trouble when suppressing a portion of fast-math-transformations

On Tue, Oct 3, 2017 at 4:03 AM, Hal Finkel via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

On 10/02/2017 11:10 AM, Bruce Hoult via llvm-dev wrote:
-cl-fast-relaxed-math
Sets the optimization options -cl-finite-math-only and -cl-unsafe-math-optimizations. This allows optimizations for floating-point arithmetic that may violate the IEEE 754 standard and the OpenCL numerical compliance requirements for single precision and double precision floating-point, as well as floating point edge case behavior. This option also relaxes the precision of commonly used math functions. This option causes the preprocessor macro __FAST_RELAXED_MATH__ to be defined in the OpenCL program. The original and modified values are defined in the SPIR-V OpenCL environment specification

I'd like to emphasise in the latter one: "This option also relaxes the precision of commonly used math functions."
Isn't this the "libm" flag that is proposed in this thread?

I don't know. I didn't see any definition of it.

In my case I'm talking about allowing the use of lower precision but very fast machine instructions instead of a slow sequence of inline instructions. But I guess instruction vs library is equivalent.

--

Hal Finkel

Lead, Compiler Technology and Programming Languages

Leadership Computing Facility

Argonne National Laboratory

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171010/a2e2a436/attachment.html>