[LLVMdev] Is r174746 broken on ARM?

Dmitry Antipov antipov at dev.rtsoft.ru
Mon Apr 8 00:45:20 PDT 2013


On 04/04/2013 05:09 PM, Hal Finkel wrote:

> Looking briefly at the code in comment 5 of PR15581, is that the pre-decrement case?
> I can't test that case on PPC, so I can certainly believe that there is a problem somewhere.
> The relevant code is a little farther down:
>
>      APInt OV =
>        cast<ConstantSDNode>(Offset)->getAPIntValue();
>      if (AM == ISD::PRE_DEC)
>        OV = -OV;
>
>      ConstantSDNode *CN =
>        cast<ConstantSDNode>(OtherUses[i]->getOperand(OffsetIdx));
>      APInt CNV = CN->getAPIntValue();
>      if (OtherUses[i]->getOpcode() == ISD::SUB && OffsetIdx == 1)
>        CNV += OV;
>      else
>        CNV -= OV;
>
> perhaps something here is not quite right.

I suspect that the first snippet (where OV is inverted) is wrong because
ARM implementation of getPreIndexedAddressParts inverts Offset for
pre-decrement case, both for ARM and Thumb2, in getARMIndexedAddressParts
and getT2IndexedAddressParts, respectively, in a calls to:

Offset = DAG.getConstant(-RHSC, RHS->getValueType(0));
                          ^^^^^
                          here!

So you don't need to invert it one more time in DAGCombiner::CombineToPreIndexedLoadStore.

Dmitry





More information about the llvm-dev mailing list