This is an archive of the discontinued LLVM Phabricator instance.

Differential D131392

[ARM] Emit local aliases (.Lfoo$local) for functions
ClosedPublic

Authored by arichardson on Aug 8 2022, 5:50 AM.

Details

Reviewers
MaskRay
Commits
rG9a2b14afa02e: [ARM] Emit local aliases (.Lfoo$local) for functions
Summary

ARMAsmPrinter::emitFunctionEntryLabel() was not calling the base class
function so the $local alias was not being emitted. This should not have
any function effect right now since ARM does not generate different code
for the $local symbols, but it could be improved in the future.

Diff Detail

Event Timeline

arichardson created this revision.Aug 8 2022, 5:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 5:50 AM
Herald added subscribers: jeroen.dobbelaere, hiraditya, kristof.beyls. · View Herald Transcript
Harbormaster completed remote builds in B179892: Diff 450775.Aug 8 2022, 8:04 AM
arichardson added a child revision: D131432: [Thumb] Baseline test for incorrect relocation with -ffunction-sections.Aug 8 2022, 1:25 PM
arichardson added a reviewer: MaskRay.
Herald added a subscriber: StephenFan. · View Herald TranscriptAug 8 2022, 1:26 PM
arichardson published this revision for review.Aug 8 2022, 1:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 1:26 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
MaskRay accepted this revision.Aug 8 2022, 2:16 PM

Test?

This revision is now accepted and ready to land.Aug 8 2022, 2:16 PM
arichardson added a comment.Aug 8 2022, 2:37 PM

I could move https://reviews.llvm.org/D131432 first in the stack to test this or would you prefer a separate test?

MaskRay added a comment.Aug 8 2022, 4:55 PM
In D131392#3708115, @arichardson wrote:

I could move https://reviews.llvm.org/D131432 first in the stack to test this or would you prefer a separate test?

Pre-committing a test than push this with an updated test.

Or, push this with the new test.

The former is probably preferred.

This revision was landed with ongoing or failed builds.Aug 9 2022, 2:54 AM
Closed by commit rG9a2b14afa02e: [ARM] Emit local aliases (.Lfoo$local) for functions (authored by arichardson). · Explain Why
This revision was automatically updated to reflect the committed changes.
arichardson mentioned this in rG67b075319b17: [ARM] Add a baseline elf-preemption test.
arichardson mentioned this in rG7925341d9332: [ARM] Add a baseline test for D131392.
arichardson added a commit: rG9a2b14afa02e: [ARM] Emit local aliases (.Lfoo$local) for functions.

Revision Contents

PathSize
llvm/
lib/
Target/
ARM/
3 lines
test/
CodeGen/
ARM/
3 lines
utils/
UpdateTestChecks/
1 line

Diff 451077

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

Show First 20 Lines • Show All 76 Lines • ▼ Show 20 Linesvoid ARMAsmPrinter::emitFunctionEntryLabel() {
// Emit symbol for CMSE non-secure entry point // Emit symbol for CMSE non-secure entry point
if (AFI->isCmseNSEntryFunction()) { if (AFI->isCmseNSEntryFunction()) {
MCSymbol *S = MCSymbol *S =
OutContext.getOrCreateSymbol("__acle_se_" + CurrentFnSym->getName()); OutContext.getOrCreateSymbol("__acle_se_" + CurrentFnSym->getName());
emitLinkage(&MF->getFunction(), S); emitLinkage(&MF->getFunction(), S);
OutStreamer->emitSymbolAttribute(S, MCSA_ELF_TypeFunction); OutStreamer->emitSymbolAttribute(S, MCSA_ELF_TypeFunction);
OutStreamer->emitLabel(S); OutStreamer->emitLabel(S);
} }
AsmPrinter::emitFunctionEntryLabel();
OutStreamer->emitLabel(CurrentFnSym);
} }
void ARMAsmPrinter::emitXXStructor(const DataLayout &DL, const Constant *CV) { void ARMAsmPrinter::emitXXStructor(const DataLayout &DL, const Constant *CV) {
uint64_t Size = getDataLayout().getTypeAllocSize(CV->getType()); uint64_t Size = getDataLayout().getTypeAllocSize(CV->getType());
assert(Size && "C++ constructor pointer had zero size!"); assert(Size && "C++ constructor pointer had zero size!");
const GlobalValue *GV = dyn_cast<GlobalValue>(CV->stripPointerCasts()); const GlobalValue *GV = dyn_cast<GlobalValue>(CV->stripPointerCasts());
assert(GV && "C++ constructor pointer was not a GlobalValue!"); assert(GV && "C++ constructor pointer was not a GlobalValue!");
▲ Show 20 Lines • Show All 2,246 Lines • Show Last 20 Lines

llvm/test/CodeGen/ARM/dso-local-func.ll

;; Check that we emit a $local alias for a dso_local function definition ;; Check that we emit a $local alias for a dso_local function definition
; RUN: llc -mtriple=armv7-linux-gnueabi -relocation-model=static < %s \ ; RUN: llc -mtriple=armv7-linux-gnueabi -relocation-model=static < %s \
; RUN: | FileCheck %s --check-prefixes=CHECK,STATIC ; RUN: | FileCheck %s --check-prefixes=CHECK,STATIC
; RUN: llc -mtriple=armv7-linux-gnueabi -relocation-model=pic < %s \ ; RUN: llc -mtriple=armv7-linux-gnueabi -relocation-model=pic < %s \
; RUN: | FileCheck %s --check-prefixes=CHECK,PIC ; RUN: | FileCheck %s --check-prefixes=CHECK,PIC
define dso_local ptr @dsolocal_func() nounwind { define dso_local ptr @dsolocal_func() nounwind {
; CHECK-LABEL: .globl dsolocal_func ; CHECK-LABEL: .globl dsolocal_func
; CHECK-NEXT: .p2align 2 ; CHECK-NEXT: .p2align 2
; CHECK-NEXT: .type dsolocal_func,%function ; CHECK-NEXT: .type dsolocal_func,%function
; CHECK-NEXT: .code 32 ; CHECK-NEXT: .code 32
; CHECK-NEXT: dsolocal_func: ; CHECK-NEXT: dsolocal_func:
; TODO: PIC codegen should emit a $local alias here ; PIC-NEXT: .Ldsolocal_func$local:
; PIC-TODO-NEXT: .Ldsolocal_func$local:
; CHECK-NEXT: .fnstart ; CHECK-NEXT: .fnstart
; CHECK-NEXT: @ %bb.0: ; CHECK-NEXT: @ %bb.0:
; STATIC-NEXT: movw r0, :lower16:dsolocal_func ; STATIC-NEXT: movw r0, :lower16:dsolocal_func
; STATIC-NEXT: movt r0, :upper16:dsolocal_func ; STATIC-NEXT: movt r0, :upper16:dsolocal_func
; STATIC-NEXT: bx lr ; STATIC-NEXT: bx lr
; PIC-NEXT: ldr r0, .LCPI0_0 ; PIC-NEXT: ldr r0, .LCPI0_0
; PIC-NEXT: .LPC0_0: ; PIC-NEXT: .LPC0_0:
; PIC-NEXT: add r0, pc, r0 ; PIC-NEXT: add r0, pc, r0
Show All 11 Lines

llvm/utils/UpdateTestChecks/asm.py

Show All 18 LinesASM_FUNCTION_X86_RE = re.compile(
r'(?:\.L[^$]+\$local:\n)?' # drop .L<func>$local: r'(?:\.L[^$]+\$local:\n)?' # drop .L<func>$local:
r'(?:[ \t]+.cfi_startproc\n|.seh_proc[^\n]+\n)?' # drop optional cfi r'(?:[ \t]+.cfi_startproc\n|.seh_proc[^\n]+\n)?' # drop optional cfi
r'(?P<body>^##?[ \t]+[^:]+:.*?)\s*' r'(?P<body>^##?[ \t]+[^:]+:.*?)\s*'
r'^\s*(?:[^:\n]+?:\s*\n\s*\.size|\.cfi_endproc|\.globl|\.comm|\.(?:sub)?section|#+ -- End function)', r'^\s*(?:[^:\n]+?:\s*\n\s*\.size|\.cfi_endproc|\.globl|\.comm|\.(?:sub)?section|#+ -- End function)',
flags=(re.M | re.S)) flags=(re.M | re.S))
ASM_FUNCTION_ARM_RE = re.compile( ASM_FUNCTION_ARM_RE = re.compile(
r'^(?P<func>[0-9a-zA-Z_$]+):\n' # f: (name of function) r'^(?P<func>[0-9a-zA-Z_$]+):\n' # f: (name of function)
r'(?:\.L(?P=func)\$local:\n)?' # drop .L<func>$local:
r'\s+\.fnstart\n' # .fnstart r'\s+\.fnstart\n' # .fnstart
r'(?P<body>.*?)' # (body of the function) r'(?P<body>.*?)' # (body of the function)
r'^.Lfunc_end[0-9]+:', # .Lfunc_end0: or # -- End function r'^.Lfunc_end[0-9]+:', # .Lfunc_end0: or # -- End function
flags=(re.M | re.S)) flags=(re.M | re.S))
ASM_FUNCTION_AARCH64_RE = re.compile( ASM_FUNCTION_AARCH64_RE = re.compile(
r'^_?(?P<func>[^:]+):[ \t]*\/\/[ \t]*@"?(?P=func)"?( (Function|Tail Call))?\n' r'^_?(?P<func>[^:]+):[ \t]*\/\/[ \t]*@"?(?P=func)"?( (Function|Tail Call))?\n'
r'(?:[ \t]+.cfi_startproc\n)?' # drop optional cfi noise r'(?:[ \t]+.cfi_startproc\n)?' # drop optional cfi noise
▲ Show 20 Lines • Show All 476 Lines • Show Last 20 Lines