[PATCH] D11800: [ARM] Reorganise and simplify thumb-1 load/store selection

John Brawn via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 7 03:10:44 PDT 2015


john.brawn added a comment.

In http://reviews.llvm.org/D11800#219148, @jmolloy wrote:

> There appears to be quite a bit of reordering going on in this diff, and I had difficulty working out if it was cleanup or if it had semantic meaning. Like moving defm _rr after defm _ri. Can you please undo this or mark what is just cleanup (and submit that separately when committing)?


All of the reordering is deliberate as I say (probably not clearly enough) in the summary. All of the patterns (except for the one for pc-relative load) have the same complexity so are matched in the order they appear in the .td file. Rearranging them to be pc-relative -> sp-relative -> immediate offset -> register offset means we have the least logic of the form "don't select this instruction because next I'm going to try selecting a different instruction that would be better" - in fact we only need that to avoid generating x+y as [x+y, #0] and instead do [x, y].

I'll add some comments to try and explain all this.

> I'm having difficulty understanding why you're removing the selection of register+scaled-immediate addressing modes entirely. Was that code always pessimistic? Is RO always better? A bit of explanation of the rationale would really help (and that rationale should be repeated in the commit message!)


I didn't, they're still there. ARMDAGToDAGISel::SelectThumbAddrModeRI was used to select the register+register addressing mode but was written in a way that meant that it was only selected when the offset was an immediate too-large for an immediate offset.


Repository:
  rL LLVM

http://reviews.llvm.org/D11800





More information about the llvm-commits mailing list