456468a08a
This fixes a runtime error that occurred due to incorrect internalization of linkonce_odr functions where function pointer equality was broken. This was hit because the prevailing copy was in a native object, so the IR copies were not exported, and the existing code internalized all of the IR copies. It could be fixed by guarding this internalization on whether the defs are (local_)unnamed_addr, meaning that their address is not significant (which we have in the summary currently for linkonce_odr via the CanAutoHide flag). Or we can propagate reference attributes as we do when determining whether a global variable is read or write-only (reference edges are annotated with whether they are read-only, write-only, or neither, and taking the address of a function would result in a reference edge to the function that is not read or write-only). However, this exposed a larger issue with the internalization handling. Looking at test cases, it appears the intent is to internalize when there is a single definition of a linkonce/weak ODR symbol (that isn't exported). This makes sense in the case of functions, because the inliner can apply its last call to static heuristic when appropriate. In the case where there is no prevailing copy in IR, internalizing all of the IR copies of a linkonce_odr, even if legal, just increases binary size. In that case it is better to fall back to the normal handling of converting all non-prevailing copies to available_externally so that they are eliminated after inlining. In the case of variables, the existing code was attempting to internalize the non-exported linkonce/weak ODR variables if they were read or write-only. While this is legal (we propagate reference attributes to determine this information), we don't even need to internalize these here as there is later separate handling that internalizes read and write-only variables when we process the module at the start of the ThinLTO backend (processGlobalForThinLTO). Instead, we can also internalize any non-exported variable when there is only one (IR) definition, which is prevailing. And in that case, we don't need to require that it is read or write-only, since we are guaranteed that all uses must use that single definition. In the new LTO API, if there are multiple defs of a linkonce or weak ODR it will be marked exported, but it isn't clear that this will always be true for the legacy LTO API. Therefore, require that there is only a single (non-local) def, and that it is prevailing. The test cases changes are both to reflect the change in the handling of linkonce_odr IR copies where the prevailing def is not in IR (the main correctness bug fix here), and to reflect the more aggressive internalization of variables when there is only a single def, it is in IR, and not exported. I've also added some additional testing via the new LTO API. Differential Revision: https://reviews.llvm.org/D151965
48 lines
2.3 KiB
LLVM
48 lines
2.3 KiB
LLVM
; Test that linkonce_odr and weak_odr variables which are visible to regular
|
|
; object (and so are not readonly) are not internalized by thin LTO.
|
|
; RUN: opt -module-summary %s -o %t.bc
|
|
; RUN: llvm-lto2 run -save-temps %t.bc -o %t.out \
|
|
; RUN: -r=%t.bc,_ZL5initSv,plx \
|
|
; RUN: -r=%t.bc,_ZN9SingletonI1SE11getInstanceEv,lx \
|
|
; RUN: -r=%t.bc,_ZZN9SingletonI1SE11getInstanceEvE8instance,lx \
|
|
; RUN: -r=%t.bc,_ZZN9SingletonI1SE11getInstanceEvE13instance_weak,lx
|
|
; RUN: llvm-dis %t.out.1.1.promote.bc -o - | FileCheck %s
|
|
; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
|
|
|
|
; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
|
|
; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = available_externally dso_local global ptr null, align 8
|
|
|
|
;; We should not internalize a linkonce_odr function when the IR definition(s)
|
|
;; are not prevailing (prevailing def in native object). This can break function
|
|
;; pointer equality (unless it has an unnamed_addr attribute indicating that the
|
|
;; address is not significant), and also can increase code size.
|
|
; CHECK: define available_externally dso_local dereferenceable(16) ptr @_ZN9SingletonI1SE11getInstanceEv()
|
|
; INTERNALIZE: define available_externally dso_local dereferenceable(16) ptr @_ZN9SingletonI1SE11getInstanceEv()
|
|
|
|
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
|
|
target triple = "x86_64-unknown-linux-gnu"
|
|
|
|
%struct.S = type { i64, i64 }
|
|
|
|
$_ZN9SingletonI1SE11getInstanceEv = comdat any
|
|
|
|
$_ZZN9SingletonI1SE11getInstanceEvE8instance = comdat any
|
|
|
|
$_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = comdat any
|
|
|
|
@_ZZN9SingletonI1SE11getInstanceEvE8instance = linkonce_odr dso_local global %struct.S zeroinitializer, comdat, align 8
|
|
|
|
@_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = weak_odr dso_local global ptr null, comdat, align 8
|
|
|
|
define dso_local void @_ZL5initSv() {
|
|
%1 = call dereferenceable(16) ptr @_ZN9SingletonI1SE11getInstanceEv()
|
|
store ptr %1, ptr @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak
|
|
store i64 1, ptr %1, align 8
|
|
ret void
|
|
}
|
|
|
|
define linkonce_odr dso_local dereferenceable(16) ptr @_ZN9SingletonI1SE11getInstanceEv() #0 comdat align 2 {
|
|
ret ptr @_ZZN9SingletonI1SE11getInstanceEvE8instance
|
|
}
|
|
|