From 77b435aaa19cb5ff897a67fb9878115196e1524a Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Thu, 19 Aug 2021 16:42:57 -0700 Subject: [PATCH] Revert "[InstrProfiling] Make COFF use the ELF comdat scheme (drop link.exe compatibility)" This reverts commit fbb8e772ec501a1b71643db90e9c6445e17d7cac. Accidentally pushed. --- 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, 38 insertions(+), 27 deletions(-) diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 073de36..cdf9dc5 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 599696f..d2620fd 100644 --- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -876,13 +876,17 @@ 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 SupportsComdat = Triple(M->getTargetTriple()).supportsCOMDAT(); + bool DataReferencedByCode = profDataReferencedByCode(*M); bool NeedComdat = needsComdatForCounter(*Fn, *M); std::string CntsVarName = getVarName(Inc, getInstrProfCountersVarPrefix()); std::string DataVarName = getVarName(Inc, getInstrProfDataVarPrefix()); auto MaybeSetComdat = [&](GlobalVariable *GV) { - if (SupportsComdat) { - Comdat *C = M->getOrInsertComdat(CntsVarName); + bool UseComdat = (NeedComdat || TT.isOSBinFormatELF()); + if (UseComdat) { + StringRef GroupName = TT.isOSBinFormatCOFF() && DataReferencedByCode + ? GV->getName() + : CntsVarName; + Comdat *C = M->getOrInsertComdat(GroupName); if (!NeedComdat) C->setSelectionKind(Comdat::NoDeduplicate); GV->setComdat(C); @@ -947,8 +951,12 @@ 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 COFF and ELF. - if (NS == 0 && (TT.isOSBinFormatCOFF() || TT.isOSBinFormatELF())) { + // 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()))) { Linkage = GlobalValue::PrivateLinkage; Visibility = GlobalValue::DefaultVisibility; } @@ -1158,11 +1166,13 @@ void InstrProfiling::emitUses() { // GlobalOpt/ConstantMerge) may not discard associated sections as a unit, so // we conservatively retain all unconditionally in the compiler. // - // 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()) + // 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))) appendToCompilerUsed(*M, CompilerUsedVars); else appendToUsed(*M, CompilerUsedVars); diff --git a/llvm/test/Instrumentation/InstrProfiling/linkage.ll b/llvm/test/Instrumentation/InstrProfiling/linkage.ll index 67ef1cb..bb381af 100644 --- a/llvm/test/Instrumentation/InstrProfiling/linkage.ll +++ b/llvm/test/Instrumentation/InstrProfiling/linkage.ll @@ -24,12 +24,13 @@ @__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 {{.*}} section ".lprfc$M", comdat, -; COFF: @__profd_foo = private global {{.*}} section ".lprfd$M", comdat($__profc_foo) +; COFF: @__profc_foo = private global +; COFF-NOT: comdat +; COFF: @__profd_foo = private global 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 @@ -61,8 +62,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", comdat, align 8 -; COFF: @__profd_foo_inline = private global{{.*}} section ".lprfd$M", comdat($__profc_foo_inline), align 8 +; COFF: @__profc_foo_inline = linkonce_odr hidden global{{.*}} section ".lprfc$M", align 8 +; COFF: @__profd_foo_inline = private global{{.*}} section ".lprfd$M", 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 e7cdd0b..56a5407 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", comdat, align 8 +; WINDOWS: @__profc_foo = private global [1 x i64] zeroinitializer, section ".lprfc$M", 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", comdat($__profc_foo), align 8 +; WINDOWS: @__profd_foo = private global {{.*}}, section ".lprfd$M", 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 db0adfa..1775aff 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", comdat, align 8 -; WIN: @__profd_foo = private {{.*}}, section ".lprfd$M", comdat($__profc_foo), 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 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", comdat, align 8 -; WIN: @__profd_bar = private {{.*}}, section ".lprfd$M", comdat($__profc_bar), 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 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", comdat, align 8 -; WIN: @__profd_baz = private {{.*}}, section ".lprfd$M", comdat($__profc_baz), 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 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