[PATCH] DAGCombine: constant fold FMA

[Mehdi Amini]

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

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