[llvm] r323297 - Don't assume a null GV is local for ELF and MachO.
Brooks Moses via llvm-commits
llvm-commits at lists.llvm.org
Sun Feb 4 14:52:43 PST 2018
Unfortunately, I think this commit needs to be reverted, as it's breaking
our grub build and I don't think the problem is us. Can someone please
check the explanation below to confirm that it makes sense, and do the
revert? Thanks!
The underlying issue is in the assumption in the comment: "we don't need to
assume those are local since the liker [sic] can trivially convert a call
to a PLT to a direct call if the target (in the runtime library) turns out
to be local." This is relevant only when the ELF file is actually being
processed by a "normal" linker -- but the grub build does its own linking
of the object files that get built into bootloader modules, and it doesn't
know how to handle this.
The patch also affects the generated assembly code, which with the
non-integrated assembler also causes changes in the undefined symbols in
the object files. Since grub also does pre-processing of the object files
to figure out dependencies, that gets broken as well.
Suppose we have a very basic C program that calls memset:
$ cat foo.c
void *memset (void *s, int c, unsigned int n);
void foo(char *c, int len) {
memset(c, 32, len);
}
If we compile this with -m32, -S and some level of optimization (not -O0),
we get a call to "memset at PLT" in the assembly file (where previously we
just got "memset"):
$ clang -m32 -O3 -S -o foo.s foo.c; grep memset foo.s
calll memset at PLT
When I then build an object file using -no-integrated-as (and binutils
2.24), we get an extra undefined reference to _GLOBAL_OFFSET_TABLE_:
$ clang -m32 -O3 -no-integrated-as -c -o foo.o foo.c; nm foo.o
00000000 T foo
U _GLOBAL_OFFSET_TABLE_
U memset
Grub's dependency checking then complains that this symbol isn't defined
anywhere in other object files that make up the bootloader.
(Note: With -O0, we get "memset", not "memset at PLT" in the call; with the
integrated assembler we don't get the undefined reference to
_GLOBAL_OFFSET_TABLE_.
For what it's worth, I also verified that compiler-generated calls to
memset work the same way, by making a test program with a memset-like loop.)
So that's the first problem. I worked around that by editing grub's
dependency analyzer to just ignore the symbol, and ended up with another
problem: Grub's linker now exits with "unsupported relocation 0x4" in this
code block:
https://github.com/coreos/grub/blob/d3fd939f18446b05d1d5456f23823498a1eb3fb4/util/grub-module-verifierXX.c#L277
I don't think that issue is nearly as amenable to simple workarounds -- and
even if it is, requiring patches to grub in order to support the latest
LLVM version seems like a problem.
Also, for what it's worth, while I was investigating this I became doubtful
of the claim in the commit message that this "should help with avoiding a
plt reference when calling an intrinsic with -fno-plt". Adding -fno-plt to
my test compilations still results in having a "calll memset at PLT" in the
generated assembly where we previously had "calll memset", which seems
directly counter to that.
Thanks,
- Brooks
On Tue, Jan 23, 2018 at 6:11 PM, Rafael Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: rafael
> Date: Tue Jan 23 18:11:18 2018
> New Revision: 323297
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323297&view=rev
> Log:
> Don't assume a null GV is local for ELF and MachO.
>
> This is already a simplification, and should help with avoiding a plt
> reference when calling an intrinsic with -fno-plt.
>
> With this change we return false for null GVs, so the caller only
> needs to check the new metadata to decide if it should use foo at plt or
> *foo at got.
>
> Modified:
> llvm/trunk/lib/Target/TargetMachine.cpp
> llvm/trunk/test/CodeGen/X86/finite-libcalls.ll
> llvm/trunk/test/CodeGen/X86/fp-intrinsics.ll
> llvm/trunk/test/CodeGen/X86/half.ll
> llvm/trunk/test/CodeGen/X86/memset-nonzero.ll
> llvm/trunk/test/CodeGen/X86/negative-sin.ll
> llvm/trunk/test/CodeGen/X86/vector-half-conversions.ll
>
> Modified: llvm/trunk/lib/Target/TargetMachine.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/
> TargetMachine.cpp?rev=323297&r1=323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Target/TargetMachine.cpp (original)
> +++ llvm/trunk/lib/Target/TargetMachine.cpp Tue Jan 23 18:11:18 2018
> @@ -137,20 +137,27 @@ bool TargetMachine::shouldAssumeDSOLocal
> if (TT.isOSBinFormatCOFF() || (TT.isOSWindows() &&
> TT.isOSBinFormatMachO()))
> return true;
>
> + // If GV is null we know that this is a call to an intrinsic. For ELF
> and
> + // MachO we don't need to assume those are local since the liker can
> trivially
> + // convert a call to a PLT to a direct call if the target (in the
> runtime
> + // library) turns out to be local.
> + if (!GV)
> + return false;
> +
> // Most PIC code sequences that assume that a symbol is local cannot
> // produce a 0 if it turns out the symbol is undefined. While this
> // is ABI and relocation depended, it seems worth it to handle it
> // here.
> - if (GV && isPositionIndependent() && GV->hasExternalWeakLinkage())
> + if (isPositionIndependent() && GV->hasExternalWeakLinkage())
> return false;
>
> - if (GV && !GV->hasDefaultVisibility())
> + if (!GV->hasDefaultVisibility())
> return true;
>
> if (TT.isOSBinFormatMachO()) {
> if (RM == Reloc::Static)
> return true;
> - return GV && GV->isStrongDefinitionForLinker();
> + return GV->isStrongDefinitionForLinker();
> }
>
> assert(TT.isOSBinFormatELF());
> @@ -160,19 +167,19 @@ bool TargetMachine::shouldAssumeDSOLocal
> RM == Reloc::Static || M.getPIELevel() != PIELevel::Default;
> if (IsExecutable) {
> // If the symbol is defined, it cannot be preempted.
> - if (GV && !GV->isDeclarationForLinker())
> + if (!GV->isDeclarationForLinker())
> return true;
>
> // A symbol marked nonlazybind should not be accessed with a plt. If
> the
> // symbol turns out to be external, the linker will convert a direct
> // access to an access via the plt, so don't assume it is local.
> - const Function *F = dyn_cast_or_null<Function>(GV);
> + const Function *F = dyn_cast<Function>(GV);
> if (F && F->hasFnAttribute(Attribute::NonLazyBind))
> return false;
>
> - bool IsTLS = GV && GV->isThreadLocal();
> + bool IsTLS = GV->isThreadLocal();
> bool IsAccessViaCopyRelocs =
> - Options.MCOptions.MCPIECopyRelocations && GV &&
> isa<GlobalVariable>(GV);
> + Options.MCOptions.MCPIECopyRelocations &&
> isa<GlobalVariable>(GV);
> Triple::ArchType Arch = TT.getArch();
> bool IsPPC =
> Arch == Triple::ppc || Arch == Triple::ppc64 || Arch ==
> Triple::ppc64le;
>
> Modified: llvm/trunk/test/CodeGen/X86/finite-libcalls.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/X86/finite-libcalls.ll?rev=323297&r1=323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/CodeGen/X86/finite-libcalls.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/finite-libcalls.ll Tue Jan 23 18:11:18
> 2018
> @@ -9,7 +9,7 @@
> define float @exp_f32(float %x) #0 {
> ; GNU-LABEL: exp_f32:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __expf_finite # TAILCALL
> +; GNU-NEXT: jmp __expf_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: exp_f32:
> ; WIN: # %bb.0:
> @@ -25,7 +25,7 @@ define float @exp_f32(float %x) #0 {
> define double @exp_f64(double %x) #0 {
> ; GNU-LABEL: exp_f64:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __exp_finite # TAILCALL
> +; GNU-NEXT: jmp __exp_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: exp_f64:
> ; WIN: # %bb.0:
> @@ -72,7 +72,7 @@ define x86_fp80 @exp_f80(x86_fp80 %x) #0
> define float @exp2_f32(float %x) #0 {
> ; GNU-LABEL: exp2_f32:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __exp2f_finite # TAILCALL
> +; GNU-NEXT: jmp __exp2f_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: exp2_f32:
> ; WIN: # %bb.0:
> @@ -88,7 +88,7 @@ define float @exp2_f32(float %x) #0 {
> define double @exp2_f64(double %x) #0 {
> ; GNU-LABEL: exp2_f64:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __exp2_finite # TAILCALL
> +; GNU-NEXT: jmp __exp2_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: exp2_f64:
> ; WIN: # %bb.0:
> @@ -135,7 +135,7 @@ define x86_fp80 @exp2_f80(x86_fp80 %x) #
> define float @log_f32(float %x) #0 {
> ; GNU-LABEL: log_f32:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __logf_finite # TAILCALL
> +; GNU-NEXT: jmp __logf_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: log_f32:
> ; WIN: # %bb.0:
> @@ -151,7 +151,7 @@ define float @log_f32(float %x) #0 {
> define double @log_f64(double %x) #0 {
> ; GNU-LABEL: log_f64:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __log_finite # TAILCALL
> +; GNU-NEXT: jmp __log_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: log_f64:
> ; WIN: # %bb.0:
> @@ -198,7 +198,7 @@ define x86_fp80 @log_f80(x86_fp80 %x) #0
> define float @log2_f32(float %x) #0 {
> ; GNU-LABEL: log2_f32:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __log2f_finite # TAILCALL
> +; GNU-NEXT: jmp __log2f_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: log2_f32:
> ; WIN: # %bb.0:
> @@ -214,7 +214,7 @@ define float @log2_f32(float %x) #0 {
> define double @log2_f64(double %x) #0 {
> ; GNU-LABEL: log2_f64:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __log2_finite # TAILCALL
> +; GNU-NEXT: jmp __log2_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: log2_f64:
> ; WIN: # %bb.0:
> @@ -261,7 +261,7 @@ define x86_fp80 @log2_f80(x86_fp80 %x) #
> define float @log10_f32(float %x) #0 {
> ; GNU-LABEL: log10_f32:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __log10f_finite # TAILCALL
> +; GNU-NEXT: jmp __log10f_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: log10_f32:
> ; WIN: # %bb.0:
> @@ -277,7 +277,7 @@ define float @log10_f32(float %x) #0 {
> define double @log10_f64(double %x) #0 {
> ; GNU-LABEL: log10_f64:
> ; GNU: # %bb.0:
> -; GNU-NEXT: jmp __log10_finite # TAILCALL
> +; GNU-NEXT: jmp __log10_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: log10_f64:
> ; WIN: # %bb.0:
> @@ -325,7 +325,7 @@ define float @pow_f32(float %x) #0 {
> ; GNU-LABEL: pow_f32:
> ; GNU: # %bb.0:
> ; GNU-NEXT: movaps %xmm0, %xmm1
> -; GNU-NEXT: jmp __powf_finite # TAILCALL
> +; GNU-NEXT: jmp __powf_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: pow_f32:
> ; WIN: # %bb.0:
> @@ -344,7 +344,7 @@ define double @pow_f64(double %x) #0 {
> ; GNU-LABEL: pow_f64:
> ; GNU: # %bb.0:
> ; GNU-NEXT: movaps %xmm0, %xmm1
> -; GNU-NEXT: jmp __pow_finite # TAILCALL
> +; GNU-NEXT: jmp __pow_finite at PLT # TAILCALL
> ;
> ; WIN-LABEL: pow_f64:
> ; WIN: # %bb.0:
>
> Modified: llvm/trunk/test/CodeGen/X86/fp-intrinsics.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/X86/fp-intrinsics.ll?rev=323297&r1=323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/CodeGen/X86/fp-intrinsics.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/fp-intrinsics.ll Tue Jan 23 18:11:18 2018
> @@ -245,7 +245,7 @@ entry:
> ; Verify that fma(3.5) isn't simplified when the rounding mode is
> ; unknown.
> ; CHECK-LABEL: f17
> -; FMACALL32: jmp fmaf # TAILCALL
> +; FMACALL32: jmp fmaf at PLT # TAILCALL
> ; FMA32: vfmadd213ss
> define float @f17() {
> entry:
> @@ -261,7 +261,7 @@ entry:
> ; Verify that fma(42.1) isn't simplified when the rounding mode is
> ; unknown.
> ; CHECK-LABEL: f18
> -; FMACALL64: jmp fma # TAILCALL
> +; FMACALL64: jmp fma at PLT # TAILCALL
> ; FMA64: vfmadd213sd
> define double @f18() {
> entry:
>
> Modified: llvm/trunk/test/CodeGen/X86/half.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/X86/half.ll?rev=323297&r1=323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/CodeGen/X86/half.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/half.ll Tue Jan 23 18:11:18 2018
> @@ -75,7 +75,7 @@ define float @test_extend32(half* %addr)
> ; CHECK-LIBCALL-LABEL: test_extend32:
> ; CHECK-LIBCALL: # %bb.0:
> ; CHECK-LIBCALL-NEXT: movzwl (%rdi), %edi
> -; CHECK-LIBCALL-NEXT: jmp __gnu_h2f_ieee # TAILCALL
> +; CHECK-LIBCALL-NEXT: jmp __gnu_h2f_ieee at PLT # TAILCALL
> ;
> ; BWON-F16C-LABEL: test_extend32:
> ; BWON-F16C: # %bb.0:
>
> Modified: llvm/trunk/test/CodeGen/X86/memset-nonzero.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/X86/memset-nonzero.ll?rev=323297&r1=323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/CodeGen/X86/memset-nonzero.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/memset-nonzero.ll Tue Jan 23 18:11:18 2018
> @@ -394,7 +394,7 @@ define void @memset_256_nonconst_bytes(i
> ; SSE-LABEL: memset_256_nonconst_bytes:
> ; SSE: # %bb.0:
> ; SSE-NEXT: movl $256, %edx # imm = 0x100
> -; SSE-NEXT: jmp memset # TAILCALL
> +; SSE-NEXT: jmp memset at PLT # TAILCALL
> ;
> ; SSE2FAST-LABEL: memset_256_nonconst_bytes:
> ; SSE2FAST: # %bb.0:
>
> Modified: llvm/trunk/test/CodeGen/X86/negative-sin.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/X86/negative-sin.ll?rev=323297&r1=323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/CodeGen/X86/negative-sin.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/negative-sin.ll Tue Jan 23 18:11:18 2018
> @@ -28,7 +28,7 @@ define double @strict(double %e) nounwin
> define double @fast(double %e) nounwind {
> ; CHECK-LABEL: fast:
> ; CHECK: # %bb.0:
> -; CHECK-NEXT: jmp sin # TAILCALL
> +; CHECK-NEXT: jmp sin at PLT # TAILCALL
> %f = fsub fast double 0.0, %e
> %g = call double @sin(double %f) readonly
> %h = fsub fast double 0.0, %g
> @@ -40,7 +40,7 @@ define double @fast(double %e) nounwind
> define double @nsz(double %e) nounwind {
> ; CHECK-LABEL: nsz:
> ; CHECK: # %bb.0:
> -; CHECK-NEXT: jmp sin # TAILCALL
> +; CHECK-NEXT: jmp sin at PLT # TAILCALL
> %f = fsub nsz double 0.0, %e
> %g = call double @sin(double %f) readonly
> %h = fsub nsz double 0.0, %g
> @@ -88,7 +88,7 @@ define double @semi_strict2(double %e) n
> define double @fn_attr(double %e) nounwind #0 {
> ; CHECK-LABEL: fn_attr:
> ; CHECK: # %bb.0:
> -; CHECK-NEXT: jmp sin # TAILCALL
> +; CHECK-NEXT: jmp sin at PLT # TAILCALL
> %f = fsub double 0.0, %e
> %g = call double @sin(double %f) readonly
> %h = fsub double 0.0, %g
>
> Modified: llvm/trunk/test/CodeGen/X86/vector-half-conversions.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/X86/vector-half-conversions.ll?rev=323297&r1=
> 323296&r2=323297&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/CodeGen/X86/vector-half-conversions.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/vector-half-conversions.ll Tue Jan 23
> 18:11:18 2018
> @@ -2953,7 +2953,7 @@ define void @store_cvt_16f32_to_16i16(<1
> define i16 @cvt_f64_to_i16(double %a0) nounwind {
> ; ALL-LABEL: cvt_f64_to_i16:
> ; ALL: # %bb.0:
> -; ALL-NEXT: jmp __truncdfhf2 # TAILCALL
> +; ALL-NEXT: jmp __truncdfhf2 at PLT # TAILCALL
> %1 = fptrunc double %a0 to half
> %2 = bitcast half %1 to i16
> ret i16 %2
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180204/2ce17c5a/attachment.html>
More information about the llvm-commits
mailing list