[PATCH] D39752: [ARM] Place jump table as the first operand in additions

[Momchil Velikov via Phabricator via llvm-commits]

chill added a comment.

In https://reviews.llvm.org/D39752#919006, @chill wrote:

> In https://reviews.llvm.org/D39752#918295, @efriedma wrote:
>
> > I understand what you're trying to do with the changes to ARMISelLowering.cpp make sense, but I'm surprised our address-matching code doesn't try to commute the operands.  Do we really not have code to do that somwhere?
>
>
> I prefer to have any kind of IR generated or transformed into some (more or less) canonicalised form, to reduce the number of cases that need to be matched. For such indexed loads
>  it's fairly typical and natural to shift/multiply the index, instead of the base address.
>
> > I'm not sure what the change to ARMAsmPrinter.cpp is supposed to do; is it buggy, and nobody noticed because it never triggers?
>
> Yes, the LDRrs should have the destination reg, the base reg, the index reg, and the shift amount. I'll see about adding a testcase for this.


Now that I looked closer, yes, this case didn't trigger, because we haven't had a shift amount there until this patch. Without this change
the compiler emits, e.g. `ldr pc, [r1, r0]` instead of `ldr pc, [r1, r0,  lsl #2]`. The test `test/CodeGen/ARM/arm-position-independence-jump-table.ll`
tests this code, so no additional test needed.


Repository:
  rL LLVM

https://reviews.llvm.org/D39752