[PATCH] Flag to enable IEEE-754 friendly FP optimizations

Mehdi Amini mehdi.amini at apple.com
Wed Feb 4 10:42:35 PST 2015


A few nitpick and some questions I have below. 

Patch 1:

The comment for:
+    unsigned AllowFPExceptAccess : 1;
mentions rounding and the comment for:
+    unsigned AllowFPRoundAccess : 1;
mentions exceptions, they are reversed.

Patch 2:

Is kexc or kround flag allowed to be present at the same time as fast?

Patch 3:

isSafeToOptimizeFPOp() is not clear to me, especially how TLI->hasFloatingPointExceptions() disable any check, including rounding, while the description of this flag is "Whether the target supports or cares about preserving floating point exception behavior”, which seems orthogonal to getTarget().Options.AllowFPRoundAccess?
I assume it is done this way to keep existing behavior? Is it transitional?

Patch 4:

isSafeToSpeculativelyExecute(), comment might deserve to be updated , I don’t think it is true anymore with respect to rounding:
"Return true if the instruction does not have any effects besides calculating the result and does not have undefined behavior”.

canTrap(), again preserving the dynamic “rounding” is included in “trap”? Maybe the comment could be updated because it does not seem obvious to me.

Patch 10: if you really want to test it, maybe the C++ unittests can do the job?

Patch 12: haven’t look into detail where the infinite loop occurs, but it seems like a hammer fix for the issue. The example mentioned in the test ((a + b) => (a - (-b)) => (a + b) => etc) makes me thinks that one of the two forms should be canonical and InstCombine should not make one of these two transforms. What do you think?



> On Jan 23, 2015, at 7:51 AM, Sergey Dmitrouk <sdmitrouk at accesssoftek.com> wrote:
> Please find updated patches in the attachment.  Changes:
> - rounding is checked via inexact exception;
> - added changes for "frem" instruction, which was absent in several places;
> - updated some of tests (to account changes in rounding and fix wrong
>   names).
> For the purpose of possibly saving some time there is also changes.diff with
> the difference from the previous version of the patch set.
> Best regards,
> Sergey
> On Thu, Jan 22, 2015 at 08:23:19PM +0200, Sergey Dmitrouk wrote:
>> On Thu, Jan 22, 2015 at 08:40:15AM -0800, Stephen Canon wrote:
>>>> On Dec 22, 2014, at 4:36 AM, Sergey Dmitrouk <sdmitrouk at accesssoftek.com> wrote:
>>>> At the moment rounding flag just
>>>> disables folding, I'm not sure how to actually check that result of
>>>> operation doesn't depend on rounding mode.  This prevents early
>>>> optimization for something like "1.0 + 2.0", so might need adjustments
>>>> if there is a way to check whether rounding is important for particular
>>>> operation.
>>> This piece is actually straightforward.  A result is affected by the rounding mode if and only if inexact is raised (otherwise, the result is exact, and therefore no rounding occurred, which means that the rounding mode can’t have any effect).
>> Thank you, Steve.  Will change that.  Now that you explained it seems
>> obvious, but I somehow skipped exceptions while thinking about tests for
>> rounding before.
>>> – Steve
>> --
>> Sergey
> <0001-Add-flags-and-command-line-switches-for-FPEnv.patch><0002-Add-FPEnv-access-flags-to-fast-math-flags.patch><0003-Consider-FPEnv-access-in-SelectionDAG.patch><0004-Skip-constant-folding-to-preserve-FPEnv.patch><0005-Teach-IR-builder-and-folders-about-new-flags.patch><0006-Do-not-fold-constants-on-reading-in-IR-asm-bitcode.patch><0007-Prevent-undesired-folding-by-InstSimplify.patch><0008-Do-not-simplify-expressions-with-FPEnv-access.patch><0009-Make-Strict-flag-available-for-more-clients.patch><0010-Use-Strict-in-IRBuilder.patch><0011-Don-t-convert-fpops-to-constexprs-in-SCCP.patch><0012-Fix-hanging-of-InstCombine.patch><changes.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

More information about the llvm-commits mailing list