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

[Sergey Dmitrouk]

Hello Hal,

On Sun, Jul 26, 2015 at 07:29:13AM -0700, Hal Finkel wrote:
> I apologize for the exceedingly-long delay in getting to these.

Better later than never :).  Thanks for reviewing it.

> 0004-Skip-const...serve-FPEnv.patch:
>
> +  /// Evaluates floating point operation to examine it, does nothing for non-FP
> +  /// operations.  Returns false for unsupported values.
> +  static bool getFPOpExceptions(const Value *V, APFloat::opStatus &S);
>
> The comment on this function does not really explain what it does. Is it getting exceptions that are definitely generated, or potentially generated? The comment should say.

Definitely generated exceptions at run-time, that are lost if expression
is folded at compile-time.

> 0005-Teach-IR-b...t-new-flags.patch:
>
> The IRBuilder parts of this need to change slightly. You can set the fast-math flags that the builder sets on all relevant instructions using SetFastMathFlags/getFastMathFlags/clearFastMathFlags, and so you don't need the extra parameters (we don't have them for any of the other flags).
>
> Maybe the get* functions in Constants.h should just take a FastMathFlags structure, instead of adding parameters for each flag?

If I remember correctly, FastMathFlags is not used in those sources and I
didn't want to introduce such dependency, but if that's OK, I'd prefer to have
FastMathFlags there.

> 0013-Don-t-hois...cts-in-LICM.patch:
>
> I don't understand why we're using a custom check here. It should just check if the operation is safe to speculatively execute, no?

Don't remember, but will check that.

> In the future, it is much easier to review these patches on reviews.llvm.org, see for instructions:
>
>   http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

I'm familiar with Phabricator, but is it able to handle multiple patches
well?  Unless I'm missing something, it can only show all them squashed (and
I don't think it's good for review) or completely separated.  In the
latter case if first patch is changed the rest twelve will require updates
as well.  What's the recommended way of publishing changes consisting of
multiple patches via Phabricator?

Will address comments and update patches soon.

Thanks,
Sergey