[llvm] r323297 - Don't assume a null GV is local for ELF and MachO.
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 16:50:15 PST 2018
I went ahead and reverted this in r324301 while we figure out what to do.
On Sun, Feb 4, 2018 at 2:52 PM, Brooks Moses <bmoses at google.com> wrote:
> 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/d3fd939f18446b05d1d5456f238234
> 98a1eb3fb4/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/Ta
>> rgetMachine.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/20180205/7b1be4b4/attachment-0001.html>
More information about the llvm-commits
mailing list