[PATCH] DAGCombine: constant fold FMA

[Hal Finkel]

----- 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