# [llvm-commits] CVS: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Chris Lattner clattner at apple.com
Mon Jun 25 23:46:29 PDT 2007

```<dag combiner part>

Another great change.

> @@ -856,6 +844,10 @@
>    ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
>    ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
>    MVT::ValueType VT = N0.getValueType();
> +
> +  // fold vector ops
> +  SDOperand FoldedVOp = SimplifyVBinOp(N);
> +  if (FoldedVOp.Val) return FoldedVOp;

I'm concerned that this adds a significant amount of control flow for
non-vector operations.  What do you think of:

// fold vector ops
if (MVT::isVector(N->getValueType(VT))) {
SDOperand FoldedVOp = SimplifyVBinOp(N);
if (FoldedVOp.Val) return FoldedVOp;
}

for each of these?

> @@ -1098,6 +1106,11 @@
>    SDOperand RMUL = ReassociateOps(ISD::MUL, N0, N1);
>    if (RMUL.Val != 0)
>      return RMUL;
> +
> +  // If either operand is undef, the result is undef
> +  if (N0.getOpcode() == ISD::UNDEF || N1.getOpcode() == ISD::UNDEF)
> +    return DAG.getNode(ISD::UNDEF, VT);
> +
>    return SDOperand();
>  }

This isn't safe for multiply.  In particular, undef*X could be well
defined to be 0 if X is dynamically always zero.  As such, this
should return Zero.  Note that this should return vector zero (or
disable the xform) in the vector case.

This xform is safe for add/sub, because there is no defined value
that (when combined with an undef) can produce a defined result.

> @@ -1162,6 +1179,11 @@
>      SDOperand Op = BuildSDIV(N);
>      if (Op.Val) return Op;
>    }
> +
> +  // If either operand is undef, the result is undef
> +  if (N0.getOpcode() == ISD::UNDEF || N1.getOpcode() == ISD::UNDEF)
> +    return DAG.getNode(ISD::UNDEF, VT);
> +
>    return SDOperand();
>  }

This is not safe for sdiv/udiv.  Safe xforms are:

// undef / X -> 0
// X / undef -> undef

If in doubt, plz check instcombine.

> @@ -1229,6 +1260,10 @@
>      return Sub;
>    }
>
> +  // If either operand is undef, the result is undef
> +  if (N0.getOpcode() == ISD::UNDEF || N1.getOpcode() == ISD::UNDEF)
> +    return DAG.getNode(ISD::UNDEF, VT);
> +
>    return SDOperand();
>  }

// undef % X -> 0
// X % undef -> undef

for both srem and urem.

> @@ -1283,6 +1323,10 @@
>      return DAG.getNode(ISD::SRA, N0.getValueType(), N0,
>                         DAG.getConstant(MVT::getSizeInBits
> (N0.getValueType())-1,
>                                         TLI.getShiftAmountTy()));
> +  // If either operand is undef, the result is undef
> +  if (N0.getOpcode() == ISD::UNDEF || N1.getOpcode() == ISD::UNDEF)
> +    return DAG.getNode(ISD::UNDEF, VT);

mulhs/mulhu seem to be the same as mul, they should produce zero

> @@ -1336,6 +1385,10 @@
>      return DAG.getNode(N0.getOpcode(), VT, ORNode, N0.getOperand(1));
>    }
>
> +  // If either operand is undef, the result is undef
> +  if (N0.getOpcode() == ISD::UNDEF || N1.getOpcode() == ISD::UNDEF)
> +    return DAG.getNode(ISD::UNDEF, VT);

I think this is dead.  The only way to get into this code is if N0-
>opcode == N1->opcode.

> @@ -2742,6 +2807,30 @@
>    SDOperand N0 = N->getOperand(0);
>    MVT::ValueType VT = N->getValueType(0);
>
> +  // If the input is a BUILD_VECTOR with all constant elements,
> fold this now.
> +  // Only do this before legalize, since afterward the target may
> be depending
> +  // on the bitconvert.

Interesting.  This is a good solution for now, but maybe this argues
for having a "target build vector", like "target constant", which
would be unmolested by the optimizer?

> +    MVT::ValueType VT = MVT::getVectorType(DstEltVT,
> +                                           Ops.size());
> +    return DAG.getNode(ISD::BUILD_VECTOR, VT, &Ops[0], Ops.size());

This idiom occurs in several places.  Do you think it makes sense to
have a helper method on SelectionDAG to do this?

> +SDOperand DAGCombiner::visitCONCAT_VECTORS(SDNode *N) {
> +  // TODO: Check to see if this is a CONCAT_VECTORS of a bunch of
> +  // EXTRACT_SUBVECTOR operations.  If so, and if the
> EXTRACT_SUBVECTOR vector
> +  // inputs come from at most two distinct vectors, turn this into
> a shuffle
> +  // node.

Also, if they come from a single vector with the right subvectors, it
could be a noop :)

> @@ -4177,24 +4121,28 @@
>    return SDOperand();
>  }
>
> +/// SimplifyVBinOp - Visit a binary vector operation, like ADD.
> +SDOperand DAGCombiner::SimplifyVBinOp(SDNode *N) {
> +  // After legalize, the target may be depending on adds and other
> +  // binary ops to provide legal ways to construct constants or other
> +  // things. Simplifying them may result in a loss of legality.
> +  if (AfterLegalize) return SDOperand();

It would be nice if this wasn't required :(

More tomorrow.  Thanks again for tackling this Dan!

-Chris

```