[PATCH] propagate IR-level fast-math-flags to DAG nodes, disabled by default

[Nick Lewycky]

On 15 June 2015 at 16:01, Sanjay Patel <spatel at rotateright.com> wrote:

> In http://reviews.llvm.org/D10403#188316, @nlewycky wrote:
>
> > Are you sure you don't need to re-enable any tests with this commit? I'm
> concerned that there's no test change.
>
>
> There should be no visible changes from adding the FP flags because we're
> not detecting them for optimizations anywhere in the DAG. That's why I
> naively marked the previous patches as 'NFC'. :)
>
> The existing tests for Exact / NSW / NUW verify that we're still able to
> do optimizations based on those bits.
>
> My first proposed patch for optimization using the FP flags (
> http://reviews.llvm.org/D9708 ) just happened to be the same spot where
> you discovered a bug ( http://reviews.llvm.org/D9893 )!
>

Ok.

================
> Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:410-412
> @@ +409,5 @@
> +  unsigned RawFlags = Flags->getRawFlags();
> +  // If no flags are set, do not alter the ID. This saves time and allows
> +  // a gradual increase in API usage of the optional optimization flags.
> +  if (RawFlags != 0)
> +    ID.AddInteger(RawFlags);
> ----------------
> nlewycky wrote:
> > Are you saying that the FoldingSetNodeID is computed elsewhere too,
> without using this function? And that we don't want to call AddInteger one
> more time because we don't want to make it different from that other place
> of computation?
> >
> Yep, that's the idea.


The comment should say that outright; as written it reads that it's a
performance issue and an API rollout issue, but leaves out the 'we must not
add this integer to the set for correctness' issue.

LGTM


> The new 'getNode' API has a nullptr default for the flags:
> SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1, SDValue N2,
>                   const SDNodeFlags *Flags = nullptr);
>
> So we're not updating every use of that function to explicitly include a
> Flags ptr. If some path into here is using Flags but none are set, then we
> want to match the folding set node that would be created by those
> un-Flag'ified uses of getNode().
>
> http://reviews.llvm.org/D10403
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150615/2d30acc6/attachment.html>