[llvm] r236171 - generalize binop reassociation; NFC

[Jonathan Roelofs]

On 4/29/15 4:30 PM, Sanjay Patel wrote:
> Author: spatel
> Date: Wed Apr 29 17:30:02 2015
> New Revision: 236171
>
> URL: http://llvm.org/viewvc/llvm-project?rev=236171&view=rev
> Log:
> generalize binop reassociation; NFC
>
> Move the fold introduced in r236031:
> http://reviews.llvm.org/rL236031
>
> to its own helper function, so we can use it for other binops.
>
> This is a preliminary step before partially solving:
> https://llvm.org/bugs/show_bug.cgi?id=21768
> https://llvm.org/bugs/show_bug.cgi?id=23116
>
>
> Modified:
>      llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=236171&r1=236170&r2=236171&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Apr 29 17:30:02 2015
> @@ -7647,6 +7647,33 @@ SDValue DAGCombiner::visitFSUBForFMAComb
>     return SDValue();
>   }
>
> +static SDValue ReassociateBinops(SDNode *N, SelectionDAG &DAG) {
> +  assert(N->getNumOperands() == 2 && "Invalid node for binop reassociation");
> +  SDValue N0 = N->getOperand(0);
> +  SDValue N1 = N->getOperand(1);
> +  EVT VT = N->getValueType(0);
> +  SDLoc DL(N);
> +  unsigned Opcode = N->getOpcode();

Perhaps add a "the opcode really is associative" assertion?

> +
> +  // Canonicalize chains of this operation to LHS to allow the following fold.
> +  if (N0.getOpcode() != Opcode && N1.getOpcode() == Opcode)
> +    return DAG.getNode(Opcode, DL, VT, N1, N0);
> +
> +  // Convert a chain of 3 dependent operations into 2 independent operations
> +  // and 1 dependent operation:
> +  //  (op N0: (op N00: (op z, w), N01: y), N1: x) ->
> +  //  (op N00: (op z, w), (op N1: x, N01: y))
> +  if (N0.getOpcode() == Opcode && N0.hasOneUse() && N1.getOpcode() != Opcode) {
> +    SDValue N00 = N0.getOperand(0);
> +    if (N00.getOpcode() == Opcode) {

Realizing that your change is NFC...

Why do we care that N00's opcode matches the other two? N00:k doesn't 
really participate in the re-association of N01:y and N1:x:

   (op N0: (op N00: k, N01: y), N1: x) ->
   (op N00: k, (op N1: x, N01: y))

or in infix notation without the N's, that is:

    (k +  y) + x ->
     k + (y  + x)

Is it sufficient for N00:k to be any binop? Can it be relaxed even further?


Cheers,

Jon

> +      SDValue N01 = N0.getOperand(1);
> +      SDValue NewOp = DAG.getNode(Opcode, DL, VT, N1, N01);
> +      return DAG.getNode(Opcode, DL, VT, N00, NewOp);
> +    }
> +  }
> +  return SDValue();
> +}
> +
>   SDValue DAGCombiner::visitFADD(SDNode *N) {
>     SDValue N0 = N->getOperand(0);
>     SDValue N1 = N->getOperand(1);
> @@ -7781,24 +7808,10 @@ SDValue DAGCombiner::visitFADD(SDNode *N
>                              N0.getOperand(0), DAG.getConstantFP(4.0, DL, VT));
>         }
>       }
> -
> -    // Canonicalize chains of adds to LHS to simplify the following transform.
> -    if (N0.getOpcode() != ISD::FADD && N1.getOpcode() == ISD::FADD)
> -      return DAG.getNode(ISD::FADD, DL, VT, N1, N0);
>
> -    // Convert a chain of 3 dependent operations into 2 independent operations
> -    // and 1 dependent operation:
> -    //  (fadd N0: (fadd N00: (fadd z, w), N01: y), N1: x) ->
> -    //  (fadd N00: (fadd z, w), (fadd N1: x, N01: y))
> -    if (N0.getOpcode() == ISD::FADD &&  N0.hasOneUse() &&
> -        N1.getOpcode() != ISD::FADD) {
> -      SDValue N00 = N0.getOperand(0);
> -      if (N00.getOpcode() == ISD::FADD) {
> -        SDValue N01 = N0.getOperand(1);
> -        SDValue NewAdd = DAG.getNode(ISD::FADD, DL, VT, N1, N01);
> -        return DAG.getNode(ISD::FADD, DL, VT, N00, NewAdd);
> -      }
> -    }
> +    if (SDValue Reassociated = ReassociateBinops(N, DAG))
> +      return Reassociated;
> +
>     } // enable-unsafe-fp-math
>
>     // FADD -> FMA combines:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded