Page MenuHomePhabricator

[RISCV] Lower extern_weak symbols using the GOT for the medany model
AcceptedPublic

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

Details

Summary

Such symbols may be undefined at link time and thus resolve to 0, which
may be further than 2GiB away from PC, causing the immediate to be out
of range for PC-relative addressing. Using the GOT avoids this, and is
the approach taken by AArch64.

Diff Detail

Event Timeline

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

Thanks! Two nits

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2736

PseudoLGA

2759

IsExternWeak can be omitted

This revision is now accepted and ready to land.Aug 2 2021, 9:36 AM
jrtc27 updated this revision to Diff 363499.Aug 2 2021, 9:36 AM

Fixed copy paste error in comment

jrtc27 marked an inline comment as done.Aug 2 2021, 9:38 AM
jrtc27 added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2736

Already spotted and fixed :)

2759

What do you mean? If it's omitted then it defaults to false and we get the old codegen

MaskRay added inline comments.Aug 2 2021, 9:39 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2759

Change the argument. The variable can be removed.

jrtc27 marked an inline comment as done.Aug 2 2021, 9:42 AM
jrtc27 added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2759

Well you could say the same about Ty and IsLocal, they're both used exactly once so could be inlined, but IMO this keeps it more readable; nested function calls can get rather messy.

MaskRay added inline comments.Aug 2 2021, 9:45 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2759

IsExternWeak is a good candidate. Its expr isn't very long. IsLocal is long and leaving it as is is fine.