Page MenuHomePhabricator

[AArch64][ELF] Prefer to lower MC_GlobalAddress operands to .Lfoo$local
ClosedPublic

Authored by MaskRay on May 4 2021, 4:43 PM.

Details

Summary

Similar to X86 D73230 & 46788a21f9152be3950e57dc526454655682bdd4

With this change, we can set dso_local in clang's -fpic -fno-semantic-interposition mode,
for default visibility external linkage non-ifunc-non-COMDAT definitions.

For such dso_local definitions, variable access/taking the address of a
function/calling a function will go through a local alias to avoid GOT/PLT.

Note: the 'S' inline assembly constraint refers to an absolute symbolic address
or a label reference (D46745).

Diff Detail

Event Timeline

MaskRay created this revision.May 4 2021, 4:43 PM
MaskRay requested review of this revision.May 4 2021, 4:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2021, 4:43 PM
MaskRay updated this revision to Diff 343231.May 5 2021, 4:18 PM

improve tests

I'm not sure I understand the practical effect of this. I mean, I get the immediate effect on the object file, but I'm not sure I understand how this impacts linking. Without this patch, will we end up with runtime relocations in some cases? If so, which cases?

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
592

This is target-independent code; what targets does it affect?

MaskRay added a comment.EditedMay 5 2021, 5:37 PM

I'm not sure I understand the practical effect of this. I mean, I get the immediate effect on the object file, but I'm not sure I understand how this impacts linking. Without this patch, will we end up with runtime relocations in some cases? If so, which cases?

Currently clang -fpic -fno-semantic-interposition is a no-op on non-x86.

On x86, clang -fpic -fno-semantic-interposition sets dso_local on default visibility external linkage definitions. (Non-x86 targets don't set dso_local on such definitions.)

This change by itself does not affect clang behavior. (dso_local definitions with changed behavior are not emitted by Clang) With D101873, -fpic -fno-semantic-interposition will be effective on aarch64.

If D101873 is enabled without this patch, clang may produce assembly like

; CHECK-NEXT:    adrp x0, gv0  # instead of .Lgv0$local
; CHECK-NEXT:    ldr w0, [x0, :lo12:gv0]

This will be rejected by {ld.bfd,gold,ld.lld} -shared because in ELF, gv0 (STV_DEFAULT STB_GLOBAL) is by default preemptible and direct access relocations referencing a preemptible symbol cannot be used in -shared mode. By switching from gv0 to .Lgv0$local, we can make linkers happy.

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
592

Only aarch64. No-op for others.

With D101873, aarch64 will behave like x86: -fpic -fno-semantic-interposition will set dso_local on default visibility external linkage definitions.

MaskRay marked an inline comment as done.May 5 2021, 5:40 PM

If D101873 is enabled without this patch, clang may produce assembly like

; CHECK-NEXT:    adrp x0, gv0  # instead of .Lgv0$local
; CHECK-NEXT:    ldr w0, [x0, :lo12:gv0]

This will be rejected by {ld.bfd,gold,ld.lld} -shared because in ELF, gv0 (STV_DEFAULT STB_GLOBAL) is by default preemptible and direct access relocations referencing a preemptible symbol cannot be used in -shared mode.

So this is basically a bugfix for dso_local handling? Okay.

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
592

Do you mean this code is unreachable for non-aarch64 targets? Or that getSymbolPreferLocal() does the same thing as getSymbol() on other targets? Or that clang won't emit IR that's impacted by this change?

MaskRay marked an inline comment as done.May 5 2021, 6:05 PM

If D101873 is enabled without this patch, clang may produce assembly like

; CHECK-NEXT:    adrp x0, gv0  # instead of .Lgv0$local
; CHECK-NEXT:    ldr w0, [x0, :lo12:gv0]

This will be rejected by {ld.bfd,gold,ld.lld} -shared because in ELF, gv0 (STV_DEFAULT STB_GLOBAL) is by default preemptible and direct access relocations referencing a preemptible symbol cannot be used in -shared mode.

So this is basically a bugfix for dso_local handling? Okay.

I may perceive this as an unimplemented feature (lowering of default visibility external linkage dso_local definitions).

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
592

On other targets, clang doesn't emit IR which may observe different behavior due to getSymbol -> getSymbolPreferLocal.

On aarch64, D101873 will cause clang to emit IR which may observe different behavior.

efriedma accepted this revision.May 6 2021, 10:54 AM

LGTM

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
592

Okay.

This revision is now accepted and ready to land.May 6 2021, 10:54 AM
MaskRay updated this revision to Diff 343470.May 6 2021, 12:03 PM
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision. (Show Details)

add a function call test