[PowerPC, RFC] Wrong-code bug in pre-inc creation

[Hal Finkel]

----- Original Message -----
> Hal Finkel <hfinkel at anl.gov> wrote on 15.05.2013 15:48:14:
> 
> > I had some impression that things are as they are because that way
> > the constants are setup for the built-in assembler (but given that
> > they values would just be sign extended, I don't know if that
> > matters
> anyway).
> 
> Well, I've changed SelectAddressRegImm to return the true value,
> and nothing else broke, so it seems to be fine.
> 
> I've now also understood why this bug never occured in 64-bit mode:
> the bug happens during an optional optimization implemented in
> DAGCombiner::CombineToPreIndexedLoadStore, at the comment:
>   // If the offset is a constant, there may be other adds of
>   constants that
>   // can be folded with this one. We should do this to avoid having
>   to keep
>   // a copy of the original base pointer.
> 
> However, this optimization will be skipped if the type of the
> displacement doesn't match the type of that "other add":
>       // FIXME: In some cases, we can be smarter about this.

Well, then, I'm glad that I did not make it too smart ;)

>       if (Op1.getValueType() != Offset.getValueType()) {
> 
> This used to be always true on 64-bit, because the "other add"
> is address arithmetic performed on i64, but the displacement
> value returned by SelectAddressRegImm was hardcoded to use i32.
> 
> I've now changed the type as well, and the optimization is now
> performed (correctly!) in 64-bit too.

Great! Thanks again!

 -Hal

> 
> Checked in as r182012.
> 
> SelectAddressRegImmShifted is unchanged (it is not *critical*
> because it's only 64-bit, and thus as mentioned above, the
> optimization is currently never performed).  I'll work on
> that as a separate patch (there's more work involved here).
> 
> 
> Bye,
> Ulrich
> 
>