[LLVMdev] Representing -ffast-math at the IR level

[Duncan Sands]

Hi Renato,

> I'm not sure about this:
>
> +  if (!Accuracy)
> +    // If it's not a floating point number then it must be 'fast'.
> +    return getFastAccuracy();
>
> Since you allow accuracies bigger than 1 in setFPAccuracy(), integers
> should be treated as float. Or at least assert.

the verifier checks that the accuracy operand is either a floating point
number (ConstantFP) or the keyword "fast".  If "Accuracy" is zero here
then that means it wasn't ConstantFP.  Thus it must have been the keyword
"fast".

> Also, I'm thinking you should carry the annotation forward on all uses
> of an annotated result, or make sure the floating point library
> searches recursively for annotations on any dependency of the value
> being analysed.

Yes, this is a possible optimization (especially useful if functions from a
-ffast-math compiled module are inlined into functions from a non -ffast-math
compiled module or vice versa) but it is not needed for correctness.  I plan to
implement optimizations using the metadata later.

> About creating annotations every time, I think this could be a nice
> idea for a metadata factory functionality. Something that would cache
> metadata, and in case of repetition, point to the same metadata. This
> could be used for other optimisations (if I recall correctly, the
> debug metadata does that already).

Yes, Chandler suggested it already, and I think it is a good idea.

> The problem with this is that, if an optimisation pass changes one,
> you must make sure the other can also be changed, or split-on-write,
> and that can cause some bloated code in the optimiser, which is not
> ideal.

Optimizers don't (or shouldn't) change metadata because metadata is
uniqued: if you change it you change it for all users.  Instead new
metadata has to be created.  So I doubt that this is a problem in
practice.  Also, I think metadata is intrinsically a weak value handle,
so if someone changes the metadata underneath the builder then its
copy will become null.  When it sees that the cached metadata is null
then it can create it anew.  So I think it should be possible to ensure
that this works well.

> I think, for now, it's acceptable. But should be on request basis
> (aka, only present if -fmath options are explicitly specified).
>
> The rest of the patch looks sane, though. I like the idea of using
> metadata, since the target code can easily ignore if it doesn't
> support FP optimisations or IEEE strictness.

This kind of metadata must only relax IEEE strictness (and never tighten
it) because *metadata can always be discarded*.  Discarding it must never
result in wrong IR/transforms, thus metadata can only give additional
permissions.

Ciao, Duncan.