This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][GISel] Initial legalization support for G_LOAD and G_STORE.
ClosedPublic

Authored by craig.topper on Aug 13 2023, 6:58 PM.

Details

Summary

This patch focuses on power of 2 bytes up to 2x XLen with and without alignment. Other cases will be handled in future patches.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 13 2023, 6:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2023, 6:58 PM
craig.topper requested review of this revision.Aug 13 2023, 6:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2023, 6:58 PM

Add G_STORE too

craig.topper retitled this revision from [RISCV][GISel] Initial legalization support for G_LOAD. to [RISCV][GISel] Initial legalization support for G_LOAD and G_STORE..Aug 13 2023, 10:59 PM

Are going to add legalise tests for G_PTR_ADD and G_ZEXTLOAD?

Are going to add legalise tests for G_PTR_ADD and G_ZEXTLOAD?

I only implememented what was required to legalize a misaligned access or a load that needs to be narrowed. So I only marked the legal cases which are covered by the tests.

Conplete support will be a different patch.

nitinjohnraj added inline comments.Aug 15 2023, 10:57 AM
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-load.mir
7

Do we need the LLVM IR to specify the alignment here? Other RISCV legalizer tests don't have the LLVM IR, so I'm wondering if we can get rid of it.

54

Perhaps we could use a test for i7? It tests that we can legalize loads for odd sizes.

craig.topper edited the summary of this revision. (Show Details)Aug 15 2023, 3:27 PM
craig.topper added inline comments.Aug 15 2023, 3:30 PM
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-load.mir
7

Did someone ask for the IR to be removed in other legalizer tests or is just something we started doing without discussion?

54

I'm going to keep odd sizes for later. I need to figure out more about hooks like lowerIfMemSizeNotByteSizePow2 and unsupportedIfMemSizeNotPow2

nitinjohnraj accepted this revision.Aug 18 2023, 3:55 PM

Perhaps we want to remove the llvm IR from the tests, but since that's not a functional change, LGTM.

llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-load.mir
7

I believe @arsenm asked for it to be removed on Lewis' original patches. https://reviews.llvm.org/D76354#inline-1124225

This revision is now accepted and ready to land.Aug 18 2023, 3:55 PM
arsenm added inline comments.Aug 18 2023, 4:07 PM
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-load.mir
7

The IR section is pure noise in nearly all situations, especially legalizer tests. It's an infrastructure failing that it gets printed at all in most situations

Rebase and remove IR from tests.

This revision was landed with ongoing or failed builds.Aug 18 2023, 8:21 PM
This revision was automatically updated to reflect the committed changes.