<div class="gmail_quote">On Wed, Jan 11, 2012 at 3:47 AM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr">baldrick@free.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Chandler,<br>
<div class="im"><br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Jan 11 02:41:08 2012<br>
> @@ -4254,6 +4254,29 @@<br>
>       return DAG.getNode(ISD::ZERO_EXTEND, N->getDebugLoc(), VT,<br>
>                          N0.getOperand(0));<br>
><br>
> +  // fold (zext (truncate x)) ->  (zext x) or<br>
> +  //      (zext (truncate x)) ->  (truncate x)<br>
> +  // This is valid when the truncated bits of x are already zero.<br>
> +  // FIXME: We should extend this to work for vectors too.<br>
</div>> +  if (N0.getOpcode() == ISD::TRUNCATE&&  !VT.isVector()) {<br>
<div class="im">> +    SDValue Op = N0.getOperand(0);<br>
> +    APInt TruncatedBits<br>
> +      = APInt::getBitsSet(Op.getValueSizeInBits(),<br>
> +                          N0.getValueSizeInBits(),<br>
> +                          std::min(Op.getValueSizeInBits(),<br>
> +                                   VT.getSizeInBits()));<br>
> +    APInt KnownZero, KnownOne;<br>
> +    DAG.ComputeMaskedBits(Op, TruncatedBits, KnownZero, KnownOne);<br>
> +    if (TruncatedBits == KnownZero) {<br>
> +      if (VT.bitsGT(Op.getValueType()))<br>
> +        return DAG.getNode(ISD::ZERO_EXTEND, N->getDebugLoc(), VT, Op);<br>
> +      if (VT.bitsLT(Op.getValueType()))<br>
> +        return DAG.getNode(ISD::TRUNCATE, N->getDebugLoc(), VT, Op);<br>
<br>
</div>here you could use getZExtOrTrunc.<br></blockquote><div><br></div><div>Will do..</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> +<br>
> +      return Op;<br>
> +    }<br>
> +  }<br>
> +<br>
<br>
</div>How about doing the analogous sext and anyext transforms too?<br></blockquote><div><br></div><div>Maaaybe... ;] Gonna try to flush out the other state I have  first.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
> +static bool FoldMaskAndShiftToScale(SelectionDAG&DAG, SDValue N,<br>
> +                                    X86ISelAddressMode&AM) {<br>
<div class="im">> +  // Scale must not be used already.<br>
> +  if (AM.IndexReg.getNode() != 0 || AM.Scale != 1) return true;<br>
<br>
</div>Returning 'true' on failure - ugh!  Maybe this should be mentioned in the<br>
comment describing the method.<br></blockquote><div><br></div><div>I thought I did, but it might have been in a later patch. I'll revisit the comments and clean them up. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
> +    // We looked through an ANY_EXTEND node, insert a ZERO_EXTEND.<br>
> +    SDValue NewX = DAG.getNode(ISD::ZERO_EXTEND, X.getDebugLoc(), VT, X);<br>
> +    if (NewX.getNode()->getNodeId() == -1 ||<br>
> +        NewX.getNode()->getNodeId()>  N.getNode()->getNodeId()) {<br>
> +      DAG.RepositionNode(N.getNode(), NewX.getNode());<br>
> +      NewX.getNode()->setNodeId(N.getNode()->getNodeId());<br>
<br>
</div>What's all the mucking around with node ids etc for?  If you really have<br>
to do this mysterious stuff, how about adding a little helper for it.</blockquote><div><br></div><div>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. </div>
</div>