This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add new lga pseudoinstruction
ClosedPublic

Authored by jrtc27 on Aug 2 2021, 7:43 AM.

Details

Summary

This mirrors lla and is always GOT-relative, allowing an explicit
request to use the GOT without having to expand the instruction. This
then means la is just defined in terms of lla and lga in the assembler,
based on whether PIC is enabled, and at the codegen level we replace la
entirely with lga since we only ever use la there when we want to load
from the GOT (and assert that to be the case).

See https://github.com/riscv-non-isa/riscv-asm-manual/issues/50

Diff Detail

Event Timeline

jrtc27 created this revision.Aug 2 2021, 7:43 AM
jrtc27 requested review of this revision.Aug 2 2021, 7:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2021, 7:43 AM
MaskRay accepted this revision.Aug 2 2021, 9:30 AM

Looks great!

llvm/test/MC/RISCV/rvi-pseudos.s
51

This test may be redundant.

Having a5 with addend (users should do it, but the assembler currently accepts the form ) below is sufficient.

This revision is now accepted and ready to land.Aug 2 2021, 9:30 AM

Hi @jrtc27. Can you land this patch? :)

jrtc27 updated this revision to Diff 447490.Jul 25 2022, 3:11 PM

Rebased, including new riscv_lga ISD node to match riscv_l(l)a (though I don't understand quite why we bother with these... it seems like extra indirection for no gain?)

jrtc27 updated this revision to Diff 522319.May 15 2023, 1:37 PM

Rebased (with substantive changes)

jrtc27 edited the summary of this revision. (Show Details)May 15 2023, 1:38 PM
jrtc27 requested review of this revision.May 15 2023, 1:46 PM
MaskRay accepted this revision.May 15 2023, 10:07 PM

LGTM. I have traced through the control flow.

llvm/test/MC/RISCV/rvi-pseudos.s
48

Add -NEXT for both auipc and lw/ld. It gives the user a hint that the instructions are contiguous. Also, in case of an failure, the FileCheck diagnostic will be more useful.

This revision is now accepted and ready to land.May 15 2023, 10:07 PM

[RISCV] Add new lga pseudoinstruction

Perhaps it's no longer new after 2 years :) Consider adding a link to https://github.com/riscv-non-isa/riscv-asm-manual/issues/50

asb accepted this revision.May 17 2023, 3:40 AM

LGTM.

jrtc27 added inline comments.May 17 2023, 11:14 AM
llvm/test/MC/RISCV/rvi-pseudos.s
48

Hm, I'd rather stick with the style of the existing checks... but you're right using -NEXT for all these would be better

evandro removed a subscriber: evandro.May 17 2023, 3:50 PM
jrtc27 edited the summary of this revision. (Show Details)May 30 2023, 3:52 PM
MaskRay added inline comments.May 30 2023, 8:12 PM
llvm/test/MC/RISCV/rvi-pseudos.s
48

Ok, I don't stress this too much. If you want to change existing patterns to use -NEXT:, that's fine as well:)

This revision was landed with ongoing or failed builds.May 31 2023, 10:49 AM
This revision was automatically updated to reflect the committed changes.