[llvm] 77b435a - Revert "[InstrProfiling] Make COFF use the ELF comdat scheme (drop link.exe compatibility)"

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 19 16:43:04 PDT 2021


Author: Fangrui Song
Date: 2021-08-19T16:42:57-07:00
New Revision: 77b435aaa19cb5ff897a67fb9878115196e1524a

URL: https://github.com/llvm/llvm-project/commit/77b435aaa19cb5ff897a67fb9878115196e1524a
DIFF: https://github.com/llvm/llvm-project/commit/77b435aaa19cb5ff897a67fb9878115196e1524a.diff

LOG: Revert "[InstrProfiling] Make COFF use the ELF comdat scheme (drop link.exe compatibility)"

This reverts commit fbb8e772ec501a1b71643db90e9c6445e17d7cac.

Accidentally pushed.

Added: 
    

Modified: 
    llvm/lib/IR/Verifier.cpp
    llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
    llvm/test/Instrumentation/InstrProfiling/linkage.ll
    llvm/test/Instrumentation/InstrProfiling/platform.ll
    llvm/test/Instrumentation/InstrProfiling/profiling.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 073de3672b38..cdf9dc522f9b 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 599696fcffba..d2620fd593e1 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 67ef1cbba346..bb381afeafbd 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 e7cdd0bcb2dc..56a540706611 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 db0adfaae11e..1775aff54431 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)


        


More information about the llvm-commits mailing list