This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Implement merging of stores of truncates.
ClosedPublic

Authored by aemerson on Sep 8 2021, 12:21 AM.

Details

Summary

This is a port of a combine which matches a pattern where a wide type scalar value is stored by several narrow stores. It folds it into a single store or a BSWAP and a store if the targets supports it.

Assuming little endian target:

 i8 *p = ...
 i32 val = ...
 p[0] = (val >> 0) & 0xFF;
 p[1] = (val >> 8) & 0xFF;
 p[2] = (val >> 16) & 0xFF;
 p[3] = (val >> 24) & 0xFF;
=>
 *((i32)p) = val;

On CTMark AArch64 -Os this results in a good amount of savings:

Program            before        after       diff
             SPASS 412792       412788       -0.0%
                kc 432528       432512       -0.0%
            lencod 430112       430096       -0.0%
  consumer-typeset 419156       419128       -0.0%
            bullet 475840       475752       -0.0%
        tramp3d-v4 367760       367628       -0.0%
          clamscan 383388       383204       -0.0%
    pairlocalalign 249764       249476       -0.1%
    7zip-benchmark 570100       568860       -0.2%
           sqlite3 287628       286920       -0.2%
Geomean difference                           -0.1%

Diff Detail

Event Timeline

aemerson created this revision.Sep 8 2021, 12:21 AM
aemerson requested review of this revision.Sep 8 2021, 12:21 AM
paquette added inline comments.Sep 8 2021, 9:52 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
83

Looks like this part isn't used in the change?

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

isSimple?

3366

weird that there is no b) case

3373

comment seems incorrect?

3424

maybe not for this patch, but maybe we should have a specific register matcher?

aemerson added inline comments.Sep 8 2021, 10:19 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
83

Yeah I initially used it but then changed it later. I'll commit this as a separate NFC change.

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

Yeah that would be useful.

arsenm added inline comments.Sep 8 2021, 10:23 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
76–80

Reverse the order of the fields?

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

This goes up to 8 bytes (also I don't see why bother limiting it)

3410–3411

isLoadFoldBarrier? Also need to watch out for volatile/atomic

aemerson added inline comments.Sep 8 2021, 11:00 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
76–80

Why?

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
3410–3411

isLoadFoldBarrier() would also trigger for stores so I think its simpler to explicitly check.

arsenm added inline comments.Sep 8 2021, 11:08 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
76–80

They're currently ordered to maximize padding. I also don't think the NeedBSwap and NeedRotate would be the most important fields (also they should get default initializers)

aemerson added inline comments.Sep 8 2021, 11:30 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
3410–3411

Actually I could use that and then explicitly check if we have a GStore. That's probably safer.

aemerson updated this revision to Diff 371407.Sep 8 2021, 11:45 AM
paquette accepted this revision.Sep 8 2021, 3:03 PM

LGTM

This revision is now accepted and ready to land.Sep 8 2021, 3:03 PM
This revision was landed with ongoing or failed builds.Sep 8 2021, 5:08 PM
This revision was automatically updated to reflect the committed changes.