[PATCH] DAGCombine: constant fold FMA
Hal Finkel
hfinkel at anl.gov
Tue Jan 13 17:03:27 PST 2015
----- Original Message -----
> From: "Mehdi Amini" <mehdi.amini at apple.com>
> To: reviews+D6912+public+bad3a92fab60047c at reviews.llvm.org
> Cc: resistor at mac.com, hfinkel at anl.gov, llvm-commits at cs.uiuc.edu
> Sent: Tuesday, January 13, 2015 6:58:35 PM
> Subject: Re: [PATCH] DAGCombine: constant fold FMA
>
>
> > On Jan 13, 2015, at 4:43 PM, hfinkel at anl.gov wrote:
> >
> > Unfortunately, this won't work either. You can't modify getNode so
> > that it will *fail* to return a node that you might want to
> > constant-fold later. This will cause all kinds of problems.
>
> Completely makes sense.
>
> >
> > Does your target want to combine these things because it is
> > essentially operating in a 'fast math' mode, do you not support
> > floating-point exception tracking, or is something else going on?
>
> Basically yes.
>
> >
> > FWIW, I think your target could always install a target-specific
> > DAG combine handler for ISD::FMA and fold them itself.
>
> Well, that’s the point of the change I was trying to do, right now it
> can’t.
Ah, okay. So the change you want to make is something like this:
if (... things are constant ...) {
SDValue NewValue = DAG.getNode(ISD::FMA, dl, N1, N2, N3);
if (NewValue == N)
return SDValue();
return NewValue;
}
-Hal
>
> The combine starts with:
>
> SDValue DAGCombiner::combine(SDNode *N) {
> SDValue RV = visit(N);
>
> // If nothing happened, try a target-specific DAG combine.
> if (!RV.getNode()) {
> ….
>
> So the API “contract” is that if visit can’t fold it should return an
> empty node. Here getNode() will fail to fold but returns a new
> (identical) node anyway and my target custom combine is never
> visited.
> Maybe the problem is that the API for getNode should be separated
> from the constant folding part.
>
> I’d like to have getNode() calling CstFold() and turned the lowering
> for FMA from calling getNode() to call CstFold().
>
>
> > Once we have fast-math flags at the SDAG level, we can also support
> > this (perhaps).
>
> :)
>
> Mehdi
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list