Ping 2: [PATCH] DAGCombiner: Recognise rotates by X+C

Richard Sandiford rsandifo at linux.vnet.ibm.com
Wed Jan 8 07:52:36 PST 2014


Owen Anderson <resistor at mac.com> writes:
> Patch 1 LGTM.

r198768, thanks.

> Patch 2:
>
>> +// Check whether Neg == OpSize - Pos.
>
> Please add a more explanatory function comment.
>
>> +  // Check for cases where Pos = NegOp1 + PosC and where:
>> +  //    Neg = OpSize - Pos
>> +  //        = OpSize - (NegOp1 + PosC)
>> +  //        = (OpSize - PosC) - NegOp1
>> +  //   NegC = OpSize - PosC
>> +  // OpSize = NegC + PosC
>
> I think this comment could do with an s-expression-style example of the
> overall pattern you’re trying to match.
>
> Patch 3:
>
>> +  // Remove a mask of OpSize - 1.
>> +  unsigned LoBits = 0;
>> +  if (Neg.getOpcode() == ISD::AND &&
>> +      isPowerOf2_64(OpSize) &&
>> +      Neg.getOperand(1).getOpcode() == ISD::Constant &&
>> + cast<ConstantSDNode>(Neg.getOperand(1))->getAPIntValue() == OpSize -
>> 1) {
>> +    Neg = Neg.getOperand(0);
>> +    LoBits = Log2_64(OpSize);
>> +  }
>> +
>
> Again, please explain the logic here.
>
>> +  if (LoBits)
>> +    return Width.getLoBits(LoBits) == 0;
>> +  return Width == OpSize;
>
> ENOCOMMENT

OK, here's patches 2 and 3 with extra (and in a couple of cases, fixed)
commentary.  Hope this isn't going too far the other way.

Thanks,
Richard

-------------- next part --------------
A non-text attachment was scrubbed...
Name: rotate-2.diff
Type: text/x-patch
Size: 5038 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140108/4b267cfc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rotate-3.diff
Type: text/x-patch
Size: 6774 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140108/4b267cfc/attachment-0001.bin>


More information about the llvm-commits mailing list