[llvm-commits] [llvm] r147936 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/Target/X86/X86ISelDAGToDAG.cpp test/CodeGen/X86/fold-and-shift.ll

[Duncan Sands]

Hi Chandler,

> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Jan 11 02:41:08 2012
> @@ -4254,6 +4254,29 @@
>       return DAG.getNode(ISD::ZERO_EXTEND, N->getDebugLoc(), VT,
>                          N0.getOperand(0));
>
> +  // fold (zext (truncate x)) ->  (zext x) or
> +  //      (zext (truncate x)) ->  (truncate x)
> +  // This is valid when the truncated bits of x are already zero.
> +  // FIXME: We should extend this to work for vectors too.
> +  if (N0.getOpcode() == ISD::TRUNCATE&&  !VT.isVector()) {
> +    SDValue Op = N0.getOperand(0);
> +    APInt TruncatedBits
> +      = APInt::getBitsSet(Op.getValueSizeInBits(),
> +                          N0.getValueSizeInBits(),
> +                          std::min(Op.getValueSizeInBits(),
> +                                   VT.getSizeInBits()));
> +    APInt KnownZero, KnownOne;
> +    DAG.ComputeMaskedBits(Op, TruncatedBits, KnownZero, KnownOne);
> +    if (TruncatedBits == KnownZero) {
> +      if (VT.bitsGT(Op.getValueType()))
> +        return DAG.getNode(ISD::ZERO_EXTEND, N->getDebugLoc(), VT, Op);
> +      if (VT.bitsLT(Op.getValueType()))
> +        return DAG.getNode(ISD::TRUNCATE, N->getDebugLoc(), VT, Op);

here you could use getZExtOrTrunc.

> +
> +      return Op;
> +    }
> +  }
> +

How about doing the analogous sext and anyext transforms too?

> +static bool FoldMaskAndShiftToScale(SelectionDAG&DAG, SDValue N,
> +                                    X86ISelAddressMode&AM) {
> +  // Scale must not be used already.
> +  if (AM.IndexReg.getNode() != 0 || AM.Scale != 1) return true;

Returning 'true' on failure - ugh!  Maybe this should be mentioned in the
comment describing the method.

> +    // We looked through an ANY_EXTEND node, insert a ZERO_EXTEND.
> +    SDValue NewX = DAG.getNode(ISD::ZERO_EXTEND, X.getDebugLoc(), VT, X);
> +    if (NewX.getNode()->getNodeId() == -1 ||
> +        NewX.getNode()->getNodeId()>  N.getNode()->getNodeId()) {
> +      DAG.RepositionNode(N.getNode(), NewX.getNode());
> +      NewX.getNode()->setNodeId(N.getNode()->getNodeId());

What's all the mucking around with node ids etc for?  If you really have
to do this mysterious stuff, how about adding a little helper for it.

Ciao, Duncan.