[llvm-commits] [llvm] r147936 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/Target/X86/X86ISelDAGToDAG.cpp test/CodeGen/X86/fold-and-shift.ll
Chandler Carruth
chandlerc at google.com
Wed Jan 11 04:10:51 PST 2012
On Wed, Jan 11, 2012 at 3:47 AM, Duncan Sands <baldrick at free.fr> wrote:
> 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.
>
Will do..
>
> > +
> > + return Op;
> > + }
> > + }
> > +
>
> How about doing the analogous sext and anyext transforms too?
>
Maaaybe... ;] Gonna try to flush out the other state I have first.
>
> > +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.
>
I thought I did, but it might have been in a later patch. I'll revisit the
comments and clean them up.
>
> > + // 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.
As discussed on IRC, this is to preserve the strict-weak-ordering
relationship between Ids for uses and defs once we're down at this layer.
I've factored all of this out in subsequent patches, as I was able to
factor it out of many different places, not just here.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120111/a408ca61/attachment.html>
More information about the llvm-commits
mailing list