<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
instead of undef.
> @@ -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
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
