rui314
commented
6 years ago It's open for two years, so I'm closing as Fixed. If it is not fixed, please reopen.
Closed llvmbot closed 6 years ago
rui314
commented
6 years ago It's open for two years, so I'm closing as Fixed. If it is not fixed, please reopen.
llvmbot
commented
8 years ago I had a fix for this approved (D23015), but when I went to merge this with the new LTO API I committed for pcc last week I discovered that his new API already has the same effect. Can you confirm that with a compiler built after 278338 that this problem no longer occurs?
llvmbot
commented
8 years ago I believe the best fix is for this code to conservatively return true (indicating that GV is needed and shouldn't be internalized) from the MustPreserveGV when it can't be found. In this case that is essentially a nop since the local copy for the alias is already internal.
We could also try stripping off any .\d+ suffix and querying the module index with that. It works but I am a little wary as the variable could originally be named with a .\d+ suffix as well and we could inadvertently use the index for a different variable. Although they should both have index entries, but I'd rather do something conservatively correct rather than cause a subtle bug due to using the wrong index entry.
llvmbot
commented
8 years ago Test case to repro, that I will add with fix:
$ cat test/tools/gold/X86/thinlto_alias2.ll ; RUN: opt -module-summary %s -o %t.o ; RUN: opt -module-summary %p/Inputs/thinlto_alias2.ll -o %t2.o
; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \ ; RUN: -shared \ ; RUN: --plugin-opt=thinlto \ ; RUN: --plugin-opt=-import-instr-limit=0 \ ; RUN: --plugin-opt=save-temps \ ; RUN: -o %t3.o %t.o %t2.o ; RUN: llvm-dis %t2.o.opt.bc -o - | FileCheck %s
$c1 = comdat any
define weak_odr i32 @f1(i8*) unnamed_addr comdat($c1) { ret i32 42 }
@a1 = alias i32 (i8), i32 (i8)* @f1
; CHECK: @a2 = alias i32 (i8), i32 (i8) @f1.1.llvm.0 ; CHECK: declare i32 @f1(i8) unnamed_addr ; CHECK: define hidden i32 @f1.1.llvm.0(i8* nocapture readnone) unnamed_addr #0 comdat($c2) {
$ cat test/tools/gold/X86/Inputs/thinlto_alias2.ll $c2 = comdat any
define weak_odr i32 @f1(i8*) unnamed_addr comdat($c2) { ret i32 41 }
@r2 = global i32(i8) @f1 @a2 = alias i32(i8), i32(i8)* @f1
llvmbot
commented
8 years ago I'm trying, but find it hard to nail down because it is a big build.
joker-eph
commented
8 years ago Do you have a repro?
Extended Description
Encountered assert(GS != DefinedGlobals.end()) failure while running ThinLTO. The assertion statement is in MustPreserveGV lambda function in llvm::thinLTOInternalizeModule (lib/Transforms/IPO/FunctionImport.cpp).
It seems that the assertion fails because it fails to recover the "original name" of the global value. ModuleSummaryIndex::getOriginalNameBeforePromote attempts to get the original name by stripping .llvm.{MODHASH}, but what I observe is that ".1" is still appended to the expected original name.
Then where this extra ".1" comes from? It is appended when the global value is materialized. IRLinker::materialize function calls IRLinker::linkGlobalValueProto function, and inside that function if DGV is nullptr or ShouldLink is true then IRLinker::copyGlobalValueProto function is called to create a global variable in the destination module that corresponds to SGV. I found that newly created global variable has the extra ".1" added to the name of the SGV.
When this thing happens? I don't have a complete understanding about it but I observed that the same global value is materialized twice during the single invocation of IRLinker::run function, once with GlobalValueMaterializer and once with LocalValueMaterializer. First, the global value
; Materializable ; Function Attrs: nounwind uwtable define weak_odr void @foo(%1) unnamed_addr #7 comdat($comdat1) align 2 personality i32 (...) @__gxx_personality_v0 {} (I renamed the function and comdat)
it materialized with GlobalValueMaterializer, (so the IRLinker::materialize function is called with ForAlias == false), and the materialized value is
; Function Attrs: nounwind uwtable declare void @foo(%"type1"*) unnamed_addr #2 align 2
Then, later, the same value is materialized again with LocalValueMaterializer (so ForAlias == true for IRLinker::materialize), and the materialized value is
; Function Attrs: nounwind uwtable define internal void @foo.1(%"type 0x7efb6ee89d80") unnamed_addr #2 comdat($comdat1) align 2 personality i32 (...) @__gxx_personality_v0 !dbg !12345 { // function definition }