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

Ulrich Weigand Ulrich.Weigand at de.ibm.com
Thu May 16 08:00:05 PDT 2013


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.
      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.

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




More information about the llvm-commits mailing list