From fbb8e772ec501a1b71643db90e9c6445e17d7cac Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Thu, 19 Aug 2021 16:38:32 -0700 Subject: [PATCH] [InstrProfiling] Make COFF use the ELF comdat scheme (drop link.exe compatibility) The COFF specific `DataReferencedByCode` complexity (D103372 D103717) is due to a link.exe limitation: an external symbol in IMAGE_COMDAT_SELECT_ASSOCIATIVE is not really dropped, so it can cause duplicate definition error. --- llvm/lib/IR/Verifier.cpp | 8 +++--- .../Transforms/Instrumentation/InstrProfiling.cpp | 30 ++++++++-------------- .../test/Instrumentation/InstrProfiling/linkage.ll | 11 ++++---- .../Instrumentation/InstrProfiling/platform.ll | 4 +-- .../Instrumentation/InstrProfiling/profiling.ll | 12 ++++----- 5 files changed, 27 insertions(+), 38 deletions(-) diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index cdf9dc5..073de36 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -1464,10 +1464,10 @@ void Verifier::visitDIImportedEntity(const DIImportedEntity &N) { void Verifier::visitComdat(const Comdat &C) { // In COFF the Module is invalid if the GlobalValue has private linkage. // Entities with private linkage don't have entries in the symbol table. - if (TT.isOSBinFormatCOFF()) - if (const GlobalValue *GV = M.getNamedValue(C.getName())) - Assert(!GV->hasPrivateLinkage(), - "comdat global value has private linkage", GV); + //if (TT.isOSBinFormatCOFF()) + // if (const GlobalValue *GV = M.getNamedValue(C.getName())) + // Assert(!GV->hasPrivateLinkage(), + // "comdat global value has private linkage", GV); } void Verifier::visitModuleIdents(const Module &M) { diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp index d2620fd..599696f 100644 --- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -876,17 +876,13 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) { // nodeduplicate COMDAT which is lowered to a zero-flag section group. This // allows -z start-stop-gc to discard the entire group when the function is // discarded. - bool DataReferencedByCode = profDataReferencedByCode(*M); + bool SupportsComdat = Triple(M->getTargetTriple()).supportsCOMDAT(); bool NeedComdat = needsComdatForCounter(*Fn, *M); std::string CntsVarName = getVarName(Inc, getInstrProfCountersVarPrefix()); std::string DataVarName = getVarName(Inc, getInstrProfDataVarPrefix()); auto MaybeSetComdat = [&](GlobalVariable *GV) { - bool UseComdat = (NeedComdat || TT.isOSBinFormatELF()); - if (UseComdat) { - StringRef GroupName = TT.isOSBinFormatCOFF() && DataReferencedByCode - ? GV->getName() - : CntsVarName; - Comdat *C = M->getOrInsertComdat(GroupName); + if (SupportsComdat) { + Comdat *C = M->getOrInsertComdat(CntsVarName); if (!NeedComdat) C->setSelectionKind(Comdat::NoDeduplicate); GV->setComdat(C); @@ -951,12 +947,8 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) { // If the data variable is not referenced by code (if we don't emit // @llvm.instrprof.value.profile, NS will be 0), and the counter keeps the // data variable live under linker GC, the data variable can be private. This - // optimization applies to ELF. - // - // On COFF, a comdat leader cannot be local so we require DataReferencedByCode - // to be false. - if (NS == 0 && (TT.isOSBinFormatELF() || - (!DataReferencedByCode && TT.isOSBinFormatCOFF()))) { + // optimization applies to COFF and ELF. + if (NS == 0 && (TT.isOSBinFormatCOFF() || TT.isOSBinFormatELF())) { Linkage = GlobalValue::PrivateLinkage; Visibility = GlobalValue::DefaultVisibility; } @@ -1166,13 +1158,11 @@ void InstrProfiling::emitUses() { // GlobalOpt/ConstantMerge) may not discard associated sections as a unit, so // we conservatively retain all unconditionally in the compiler. // - // On ELF, the linker can guarantee the associated sections will be retained - // or discarded as a unit, so llvm.compiler.used is sufficient. Similarly on - // COFF, if prof data is not referenced by code we use one comdat and ensure - // this GC property as well. Otherwise, we have to conservatively make all of - // the sections retained by the linker. - if (TT.isOSBinFormatELF() || - (TT.isOSBinFormatCOFF() && !profDataReferencedByCode(*M))) + // On COFF and ELF, the linker can guarantee the associated sections will be + // retained or discarded as a unit, so llvm.compiler.used is sufficient. + // Otherwise, we have to conservatively make all of the sections retained by + // the linker. + if (TT.isOSBinFormatCOFF() || TT.isOSBinFormatELF()) appendToCompilerUsed(*M, CompilerUsedVars); else appendToUsed(*M, CompilerUsedVars); diff --git a/llvm/test/Instrumentation/InstrProfiling/linkage.ll b/llvm/test/Instrumentation/InstrProfiling/linkage.ll index bb381af..67ef1cb 100644 --- a/llvm/test/Instrumentation/InstrProfiling/linkage.ll +++ b/llvm/test/Instrumentation/InstrProfiling/linkage.ll @@ -24,13 +24,12 @@ @__profn_foo_inline = linkonce_odr hidden constant [10 x i8] c"foo_inline" @__profn_foo_extern = linkonce_odr hidden constant [10 x i8] c"foo_extern" -; ELF: @__profc_foo = private global {{.*}} section "__llvm_prf_cnts", comdat +; ELF: @__profc_foo = private global {{.*}} section "__llvm_prf_cnts", comdat, ; ELF: @__profd_foo = private global {{.*}} section "__llvm_prf_data", comdat($__profc_foo) ; MACHO: @__profc_foo = private global ; MACHO: @__profd_foo = private global -; COFF: @__profc_foo = private global -; COFF-NOT: comdat -; COFF: @__profd_foo = private global +; COFF: @__profc_foo = private global {{.*}} section ".lprfc$M", comdat, +; COFF: @__profd_foo = private global {{.*}} section ".lprfd$M", comdat($__profc_foo) define void @foo() { call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 0, i32 1, i32 0) ret void @@ -62,8 +61,8 @@ define internal void @foo_internal() { ; ELF: @__profd_foo_inline = private global{{.*}}section "__llvm_prf_data", comdat($__profc_foo_inline), align 8 ; MACHO: @__profc_foo_inline = linkonce_odr hidden global ; MACHO: @__profd_foo_inline = linkonce_odr hidden global -; COFF: @__profc_foo_inline = linkonce_odr hidden global{{.*}} section ".lprfc$M", align 8 -; COFF: @__profd_foo_inline = private global{{.*}} section ".lprfd$M", align 8 +; COFF: @__profc_foo_inline = linkonce_odr hidden global{{.*}} section ".lprfc$M", comdat, align 8 +; COFF: @__profd_foo_inline = private global{{.*}} section ".lprfd$M", comdat($__profc_foo_inline), align 8 define linkonce_odr void @foo_inline() { call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0) ret void diff --git a/llvm/test/Instrumentation/InstrProfiling/platform.ll b/llvm/test/Instrumentation/InstrProfiling/platform.ll index 56a5407..e7cdd0b 100644 --- a/llvm/test/Instrumentation/InstrProfiling/platform.ll +++ b/llvm/test/Instrumentation/InstrProfiling/platform.ll @@ -20,11 +20,11 @@ ; MACHO: @__profc_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8 ; ELF: @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8 -; WINDOWS: @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8 +; WINDOWS: @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", comdat, align 8 ; MACHO: @__profd_foo = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8 ; ELF: @__profd_foo = private {{.*}}, section "__llvm_prf_data", comdat($__profc_foo), align 8 -; WINDOWS: @__profd_foo = private global {{.*}}, section ".lprfd$M", align 8 +; WINDOWS: @__profd_foo = private global {{.*}}, section ".lprfd$M", comdat($__profc_foo), align 8 ; ELF: @__llvm_prf_nm = private constant [{{.*}} x i8] c"{{.*}}", section "{{.*}}__llvm_prf_names", align 1 ; WINDOWS: @__llvm_prf_nm = private constant [{{.*}} x i8] c"{{.*}}", section "{{.*}}lprfn$M", align 1 diff --git a/llvm/test/Instrumentation/InstrProfiling/profiling.ll b/llvm/test/Instrumentation/InstrProfiling/profiling.ll index 1775aff..db0adfa 100644 --- a/llvm/test/Instrumentation/InstrProfiling/profiling.ll +++ b/llvm/test/Instrumentation/InstrProfiling/profiling.ll @@ -21,8 +21,8 @@ ; ELF: @__profd_foo = private global { i64, i64, i64, i8*, i8*, i32, [2 x i16] } { i64 [[#]], i64 0, i64 sub (i64 ptrtoint ([1 x i64]* @__profc_foo to i64), i64 ptrtoint ({ i64, i64, i64, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i64)), i8* null, i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc_foo), align 8 ; MACHO: @__profc_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8 ; MACHO: @__profd_foo = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8 -; WIN: @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8 -; WIN: @__profd_foo = private {{.*}}, section ".lprfd$M", align 8 +; WIN: @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", comdat, align 8 +; WIN: @__profd_foo = private {{.*}}, section ".lprfd$M", comdat($__profc_foo), align 8 define void @foo() { call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 0, i32 1, i32 0) ret void @@ -32,8 +32,8 @@ define void @foo() { ; ELF: @__profd_bar = private {{.*}}, section "__llvm_prf_data", comdat($__profc_bar), align 8 ; MACHO: @__profc_bar = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8 ; MACHO: @__profd_bar = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8 -; WIN: @__profc_bar = private global [1 x i64] zeroinitializer, section ".lprfc$M", align 8 -; WIN: @__profd_bar = private {{.*}}, section ".lprfd$M", align 8 +; WIN: @__profc_bar = private global [1 x i64] zeroinitializer, section ".lprfc$M", comdat, align 8 +; WIN: @__profd_bar = private {{.*}}, section ".lprfd$M", comdat($__profc_bar), align 8 define void @bar() { call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_bar, i32 0, i32 0), i64 0, i32 1, i32 0) ret void @@ -43,8 +43,8 @@ define void @bar() { ; ELF: @__profd_baz = private {{.*}}, section "__llvm_prf_data", comdat($__profc_baz), align 8 ; MACHO: @__profc_baz = private global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8 ; MACHO: @__profd_baz = private {{.*}}, section "__DATA,__llvm_prf_data,regular,live_support", align 8 -; WIN: @__profc_baz = private global [3 x i64] zeroinitializer, section ".lprfc$M", align 8 -; WIN: @__profd_baz = private {{.*}}, section ".lprfd$M", align 8 +; WIN: @__profc_baz = private global [3 x i64] zeroinitializer, section ".lprfc$M", comdat, align 8 +; WIN: @__profd_baz = private {{.*}}, section ".lprfd$M", comdat($__profc_baz), align 8 define void @baz() { call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_baz, i32 0, i32 0), i64 0, i32 3, i32 0) call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_baz, i32 0, i32 0), i64 0, i32 3, i32 1) -- 2.7.4