[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#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 in a 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.

> I guess the change to ARMConstantIslandPass.cpp is just to match the new pattern we generate in Thumb1 mode?

It matches the different order of operands generated in `ARMISelLOwering.cpp`, `ARMTargetLowering::LowerBR_JT`.

> Would this code be less confusing if we address the FIXME in ARMInstrInfo.td about using "addrmode2" in BR_JTm?

I have a patch for that too, will sent it shortly. The two changes are don't conflict or make one another redundant, IIRC.


Repository:
  rL LLVM

https://reviews.llvm.org/D39752