This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel][Legalizer] Support reducing load/store width in big endian order
ClosedPublic

Authored by 0x59616e on Jan 10 2022, 5:10 AM.

Details

Summary

LegalizerHelper::reduceLoadStoreWidth uses little endian order when splitting type. This patch adds big endian support.

Diff Detail

Event Timeline

0x59616e created this revision.Jan 10 2022, 5:10 AM
0x59616e requested review of this revision.Jan 10 2022, 5:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2022, 5:10 AM
0x59616e edited the summary of this revision. (Show Details)Jan 10 2022, 5:11 AM
0x59616e updated this revision to Diff 398609.Jan 10 2022, 6:22 AM

update diff

0x59616e edited the summary of this revision. (Show Details)Jan 10 2022, 6:23 AM
0x59616e updated this revision to Diff 398832.Jan 10 2022, 8:45 PM

update diff

0x59616e updated this revision to Diff 399850.Jan 13 2022, 5:20 PM

update diff

arsenm added inline comments.Jan 17 2022, 4:52 PM
llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll
2

Generally legalizer tests are MIR->MIR. I think it would be best to have some end to end IR tests, in addition to MIR->MIR tests that only run the legalizer

0x59616e added inline comments.Jan 19 2022, 7:04 PM
llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll
2

Do you mind explaining the meaning of "end to end IR tests" ?

0x59616e updated this revision to Diff 401596.EditedJan 20 2022, 4:24 AM

update diff

Rewrite tests in mir form suggested by @arsenm. Thanks !

0x59616e edited the summary of this revision. (Show Details)Jan 20 2022, 4:24 AM
0x59616e added inline comments.Jan 20 2022, 4:38 AM
llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll
2

Guess: If you mean IR->assembly test, that needs instruction selector & regbank selector support, which I haven't started to do.

0x59616e retitled this revision from [GlobalISel][Legalizer] Support big endian when reducing load/store width to [GlobalISel][Legalizer] Support reducing load/store width in big endian order.Jan 28 2022, 7:33 PM
RKSimon added inline comments.Feb 7 2022, 3:43 AM
llvm/test/CodeGen/M68k/GlobalISel/legalize-load-store.mir
98 ↗(On Diff #401596)

pre-commit + rebase this so that the patch some the codegen diff?

0x59616e added inline comments.Feb 7 2022, 4:21 AM
llvm/test/CodeGen/M68k/GlobalISel/legalize-load-store.mir
98 ↗(On Diff #401596)

Thanks for commenting !

Although I did some search on google and mailing list, I still don't know what "pre-commit" should do. I tried to commit this patch on the main branch and no error message happened. Do you mean "git clang-format" or other stuff ?

Apologize for my stupidity.

0x59616e updated this revision to Diff 406403.Feb 7 2022, 5:00 AM

move test file into another patch + rebase

0x59616e marked an inline comment as done.
0x59616e added inline comments.
llvm/test/CodeGen/M68k/GlobalISel/legalize-load-store.mir
98 ↗(On Diff #401596)

I guess what you mean is to separate test file into a "pre-commit test" patch ?

I meant rebase this patch on top of D119132 - that way we show the effect of this patch on the test's codegen. So D119132 must pass with current upstream trunk

0x59616e updated this revision to Diff 406417.Feb 7 2022, 6:01 AM

Address feedback from @RKSimon. Thanks !

arsenm accepted this revision.Feb 7 2022, 6:36 AM
This revision is now accepted and ready to land.Feb 7 2022, 6:36 AM
RKSimon accepted this revision.Feb 7 2022, 6:48 AM

LGTM

Thanks @arsenm @RKSimon !

Since I don't have commit access, could someone commit this on behalf of me ?

Thanks again @arsenm @RKSimon ! Really appreciate your help ;)