[PATCH] D32688: [Coverage] Comdat section name should be same as the variable name in COFF format

[Adam Folwarczny via Phabricator via llvm-commits]

adamf updated this revision to Diff 98531.
adamf added a comment.

Alternative solution with Internal Linkage for COFF format.


https://reviews.llvm.org/D32688

Files:
  lib/Transforms/Instrumentation/InstrProfiling.cpp
  test/Instrumentation/InstrProfiling/PR23499.ll


Index: test/Instrumentation/InstrProfiling/PR23499.ll
===================================================================
--- test/Instrumentation/InstrProfiling/PR23499.ll
+++ test/Instrumentation/InstrProfiling/PR23499.ll
@@ -20,8 +20,8 @@
 
 
 ; COFF-NOT: __profn__Z3barIvEvv
-; COFF: @__profc__Z3barIvEvv = linkonce_odr hidden global [1 x i64] zeroinitializer, section "{{.*}}prfc", comdat, align 8
-; COFF: @__profd__Z3barIvEvv = linkonce_odr hidden global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($__profc__Z3barIvEvv), align 8
+; COFF: @__profc__Z3barIvEvv = internal global [1 x i64] zeroinitializer, section "{{.*}}prfc", align 8
+; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", align 8
 
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1
Index: lib/Transforms/Instrumentation/InstrProfiling.cpp
===================================================================
--- lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -366,13 +366,11 @@
   if (!needsComdatForCounter(F, M))
     return nullptr;
 
-  // COFF format requires a COMDAT section to have a key symbol with the same
-  // name. The linker targeting COFF also requires that the COMDAT
-  // a section is associated to must precede the associating section. For this
-  // reason, we must choose the counter var's name as the name of the comdat.
-  StringRef ComdatPrefix = (Triple(M.getTargetTriple()).isOSBinFormatCOFF()
-                                ? getInstrProfCountersVarPrefix()
-                                : getInstrProfComdatPrefix());
+  // In COFF format we use Internal Linkage.
+  if (Triple(M.getTargetTriple()).isOSBinFormatCOFF())
+    return nullptr;
+
+  StringRef ComdatPrefix = getInstrProfComdatPrefix();
   return M.getOrInsertComdat(StringRef(getVarName(Inc, ComdatPrefix)));
 }
 
@@ -406,9 +404,13 @@
   // only one copy of counters of the COMDAT function will be emitted after
   // linking.
   Function *Fn = Inc->getParent()->getParent();
-  Comdat *ProfileVarsComdat = nullptr;
-  ProfileVarsComdat = getOrCreateProfileComdat(*M, *Fn, Inc);
-
+  Comdat *ProfileVarsComdat = getOrCreateProfileComdat(*M, *Fn, Inc);
+  GlobalValue::LinkageTypes Linkage = NamePtr->getLinkage();
+  GlobalValue::VisibilityTypes Visibility = NamePtr->getVisibility();
+  if (Triple(M->getTargetTriple()).isOSBinFormatCOFF()) {
+    Linkage = GlobalValue::InternalLinkage;
+    Visibility = GlobalValue::DefaultVisibility;
+  }
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
   LLVMContext &Ctx = M->getContext();
   ArrayType *CounterTy = ArrayType::get(Type::getInt64Ty(Ctx), NumCounters);
@@ -415,10 +417,10 @@
 
   // Create the counters variable.
   auto *CounterPtr =
-      new GlobalVariable(*M, CounterTy, false, NamePtr->getLinkage(),
+      new GlobalVariable(*M, CounterTy, false, Linkage,
                          Constant::getNullValue(CounterTy),
                          getVarName(Inc, getInstrProfCountersVarPrefix()));
-  CounterPtr->setVisibility(NamePtr->getVisibility());
+  CounterPtr->setVisibility(Visibility);
   CounterPtr->setSection(
       getInstrProfSectionName(IPSK_cnts, TT.getObjectFormat()));
   CounterPtr->setAlignment(8);
@@ -436,10 +438,10 @@
       ArrayType *ValuesTy = ArrayType::get(Type::getInt64Ty(Ctx), NS);
 
       auto *ValuesVar =
-          new GlobalVariable(*M, ValuesTy, false, NamePtr->getLinkage(),
+          new GlobalVariable(*M, ValuesTy, false, Linkage,
                              Constant::getNullValue(ValuesTy),
                              getVarName(Inc, getInstrProfValuesVarPrefix()));
-      ValuesVar->setVisibility(NamePtr->getVisibility());
+      ValuesVar->setVisibility(Visibility);
       ValuesVar->setSection(
           getInstrProfSectionName(IPSK_vals, TT.getObjectFormat()));
       ValuesVar->setAlignment(8);
@@ -470,10 +472,10 @@
 #define INSTR_PROF_DATA(Type, LLVMType, Name, Init) Init,
 #include "llvm/ProfileData/InstrProfData.inc"
   };
-  auto *Data = new GlobalVariable(*M, DataTy, false, NamePtr->getLinkage(),
+  auto *Data = new GlobalVariable(*M, DataTy, false, Linkage,
                                   ConstantStruct::get(DataTy, DataVals),
                                   getVarName(Inc, getInstrProfDataVarPrefix()));
-  Data->setVisibility(NamePtr->getVisibility());
+  Data->setVisibility(Visibility);
   Data->setSection(getInstrProfSectionName(IPSK_data, TT.getObjectFormat()));
   Data->setAlignment(INSTR_PROF_DATA_ALIGNMENT);
   Data->setComdat(ProfileVarsComdat);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32688.98531.patch
Type: text/x-patch
Size: 5024 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170510/d39fb1d3/attachment.bin>