This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel] Add a new reassociation for G_PTR_ADDs.
ClosedPublic

Authored by aemerson on Sep 9 2021, 10:18 AM.

Details

Summary

G_PTR_ADD (G_PTR_ADD X, C), Y) -> (G_PTR_ADD (G_PTR_ADD(X, Y), C)

Improves CTMark -Os on AArch64:

Program            before after  diff
           sqlite3 286932 287024  0.0%
                kc 432512 432508 -0.0%
             SPASS 412788 412764 -0.0%
    pairlocalalign 249460 249416 -0.0%
            bullet 475740 475512 -0.0%
    7zip-benchmark 568864 568356 -0.1%
  consumer-typeset 419088 418648 -0.1%
        tramp3d-v4 367628 367224 -0.1%
          clamscan 383184 382732 -0.1%
            lencod 430028 429284 -0.2%
Geomean difference               -0.1%

Diff Detail

Event Timeline

aemerson created this revision.Sep 9 2021, 10:18 AM
aemerson requested review of this revision.Sep 9 2021, 10:18 AM
arsenm added inline comments.Sep 9 2021, 3:27 PM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4139–4146

Would it be cleaner to just insert a fresh instruction where it needs to be and let CSE deal with it?

aemerson added inline comments.Sep 9 2021, 4:11 PM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4139–4146

I don't really think that's cleaner since it has the same effect and would make the original G_PTR_ADD dead. Seems better to avoid creating a new instruction and just move it.

But it just occurred to me that since we have a oneuse check for the inner G_PTR_ADD, there shouldn't be a problem in having the two in different blocks, sinking it can't break any other uses.

aemerson updated this revision to Diff 371768.Sep 9 2021, 6:59 PM

Allow multiple blocks.

test for multiple blocks?

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4122

do you mean iff or if?

4132

test for multiple blocks?

There’s no special code to handle it so there’s no test. I removed the different-blocks check + test in the latest revision.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4122

iif

This revision is now accepted and ready to land.Sep 10 2021, 7:05 PM
foad added inline comments.Sep 13 2021, 1:33 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4122

What does "iif" mean?

foad added a comment.Sep 13 2021, 1:39 AM

Instead of optimizing three specific cases do you think it's worth doing something more general, like traversing a whole tree of G_PTR_ADD and G_ADD expressions, collecting the pointers / ints / constant-offsets, and then reassembling them in an optimal order?

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4188

Mismatched parentheses on this line.

4201–4203

Stray parenthesis.

Instead of optimizing three specific cases do you think it's worth doing something more general, like traversing a whole tree of G_PTR_ADD and G_ADD expressions, collecting the pointers / ints / constant-offsets, and then reassembling them in an optimal order?

While a nice idea in theory, because of the trickery around optimizing for the best addressing modes, I don't think it's feasible without a lot of added complexity.

This revision was landed with ongoing or failed builds.Sep 14 2021, 11:58 PM
This revision was automatically updated to reflect the committed changes.