Skip to content

Commit

Permalink
[TargetMachine] Don't imply dso_local on global variable declarations…
Browse files Browse the repository at this point in the history
… in Reloc::Static model

clang/lib/CodeGen/CodeGenModule sets dso_local on applicable global variables,
we don't need to duplicate the work in TargetMachine:shouldAssumeDSOLocal.
(Actually the long-term goal (started by r324535) is to remove as much
additional implied dso_local in TargetMachine:shouldAssumeDSOLocal as possible.)

By not implying dso_local, we will respect dso_local/dso_preemptable specifiers
set by the frontend. This allows the proposed -fno-direct-access-external-data
option to work with -fno-pic and prevent copy relocations.

This patch should be NFC in terms of the Clang behavior because the case we
don't set dso_local is a case Clang sets dso_local. However, some tests don't
set dso_local on some `external global` and expose some differences. Most tests
have been fixed to be more robust in previous commits.
  • Loading branch information
MaskRay committed Dec 5, 2020
1 parent fd32639 commit 961f31d
Show file tree
Hide file tree
Showing 13 changed files with 1,160 additions and 1,085 deletions.
15 changes: 12 additions & 3 deletions llvm/lib/Target/TargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,18 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
if (Arch == Triple::ppc || TT.isPPC64())
return false;

// Check if we can use copy relocations.
if (!(GV && GV->isThreadLocal()) && RM == Reloc::Static)
return true;
// dso_local is traditionally implied for Reloc::Static. Eventually we shall
// drop the if block entirely and respect dso_local/dso_preemptable
// specifiers set by the frontend.
if (RM == Reloc::Static) {
// We currently respect dso_local/dso_preemptable specifiers for
// variables.
if (!GV || F)
return true;
// TODO Remove the special case for x86-32 and wasm.
if ((Arch == Triple::x86 || TT.isWasm()) && !GV->isThreadLocal())
return true;
}
} else if (TT.isOSBinFormatELF()) {
// If dso_local allows AsmPrinter::getSymbolPreferLocal to use a local
// alias, set the flag. We cannot set dso_local for other global values,
Expand Down
10 changes: 4 additions & 6 deletions llvm/test/CodeGen/AArch64/extern-weak.ll
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,10 @@ define i32* @bar() {

ret i32* %addr

; In the large model, the usual relocations are absolute and can
; materialise 0.
; CHECK-LARGE: movz [[ADDR:x[0-9]+]], #:abs_g0_nc:arr_var
; CHECK-LARGE: movk [[ADDR]], #:abs_g1_nc:arr_var
; CHECK-LARGE: movk [[ADDR]], #:abs_g2_nc:arr_var
; CHECK-LARGE: movk [[ADDR]], #:abs_g3:arr_var
; Note, In the large model, if dso_local, the relocations are absolute and can materialise 0.
; CHECK-LARGE: adrp x[[ADDR:[0-9]+]], :got:arr_var
; CHECK-LARGE-NEXT: ldr x[[ADDR]], [x[[ADDR]], :got_lo12:arr_var]
; CHECK-LARGE-NEXT: add x0, x[[ADDR]], #20

; CHECK-TINY: ldr [[BASE:x[0-9]+]], :got:arr_var
; CHECK-TINY: add x0, [[BASE]], #20
Expand Down
25 changes: 13 additions & 12 deletions llvm/test/CodeGen/AArch64/tiny_model.ll
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
define void @foo1() {
; CHECK-LABEL: foo1:
; CHECK: // %bb.0: // %entry
; CHECK-NEXT: adr x8, src
; CHECK-NEXT: ldr x8, :got:src
; CHECK-NEXT: ldrb w8, [x8]
; CHECK-NEXT: adr x9, dst
; CHECK-NEXT: ldr x9, :got:dst
; CHECK-NEXT: strb w8, [x9]
; CHECK-NEXT: ret
;
; CHECK-GLOBISEL-LABEL: foo1:
; CHECK-GLOBISEL: // %bb.0: // %entry
; CHECK-GLOBISEL-NEXT: adr x8, src
; CHECK-GLOBISEL-NEXT: ldr x8, :got:src
; CHECK-GLOBISEL-NEXT: ldrb w8, [x8]
; CHECK-GLOBISEL-NEXT: adr x9, dst
; CHECK-GLOBISEL-NEXT: ldr x9, :got:dst
; CHECK-GLOBISEL-NEXT: strb w8, [x9]
; CHECK-GLOBISEL-NEXT: ret
;
Expand Down Expand Up @@ -53,15 +53,15 @@ entry:
define void @foo2() {
; CHECK-LABEL: foo2:
; CHECK: // %bb.0: // %entry
; CHECK-NEXT: adr x8, ptr
; CHECK-NEXT: adr x9, dst
; CHECK-NEXT: ldr x8, :got:ptr
; CHECK-NEXT: ldr x9, :got:dst
; CHECK-NEXT: str x9, [x8]
; CHECK-NEXT: ret
;
; CHECK-GLOBISEL-LABEL: foo2:
; CHECK-GLOBISEL: // %bb.0: // %entry
; CHECK-GLOBISEL-NEXT: adr x8, ptr
; CHECK-GLOBISEL-NEXT: adr x9, dst
; CHECK-GLOBISEL-NEXT: ldr x8, :got:ptr
; CHECK-GLOBISEL-NEXT: ldr x9, :got:dst
; CHECK-GLOBISEL-NEXT: str x9, [x8]
; CHECK-GLOBISEL-NEXT: ret
;
Expand All @@ -88,16 +88,17 @@ define void @foo3() {
;
; CHECK-LABEL: foo3:
; CHECK: // %bb.0: // %entry
; CHECK-NEXT: adr x8, src
; CHECK-NEXT: ldr x8, :got:src
; CHECK-NEXT: ldr x9, :got:ptr
; CHECK-NEXT: ldrb w8, [x8]
; CHECK-NEXT: ldr x9, ptr
; CHECK-NEXT: ldr x9, [x9]
; CHECK-NEXT: strb w8, [x9]
; CHECK-NEXT: ret
;
; CHECK-GLOBISEL-LABEL: foo3:
; CHECK-GLOBISEL: // %bb.0: // %entry
; CHECK-GLOBISEL-NEXT: adr x8, src
; CHECK-GLOBISEL-NEXT: adr x9, ptr
; CHECK-GLOBISEL-NEXT: ldr x8, :got:src
; CHECK-GLOBISEL-NEXT: ldr x9, :got:ptr
; CHECK-GLOBISEL-NEXT: ldrb w8, [x8]
; CHECK-GLOBISEL-NEXT: ldr x9, [x9]
; CHECK-GLOBISEL-NEXT: strb w8, [x9]
Expand Down
Loading

0 comments on commit 961f31d

Please sign in to comment.