[prev in list] [next in list] [prev in thread] [next in thread] 

List:       llvm-commits
Subject:    [PATCH] D32688: [Coverage] Comdat section name should be same as the variable name in COFF format
From:       Adam Folwarczny via Phabricator via llvm-commits <llvm-commits () lists ! llvm ! org>
Date:       2017-04-30 21:47:57
Message-ID: differential-rev-PHID-DREV-2nzt3m2dncs2mpmmkhpa-req () reviews ! llvm ! org
[Download RAW message or body]

adamf created this revision.

COFF format requires to have Comdat section names same as the variable name.
For Instruction Profile Data Variable the Comdat name wasn't equal to the variable \
name.  It caused linker error like:  "error LNK2005: ___profd_?compute@@YAHH@Z \
already defined in Source.obj" e.g. for functions included into multiple cpp files.


https://reviews.llvm.org/D32688

Files:
  lib/Transforms/Instrumentation/InstrProfiling.cpp


Index: lib/Transforms/Instrumentation/InstrProfiling.cpp
===================================================================
--- lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -362,7 +362,8 @@
 }
 
 static inline Comdat *getOrCreateProfileComdat(Module &M, Function &F,
-                                               InstrProfIncrementInst *Inc) {
+                                               InstrProfIncrementInst *Inc,
+                                               StringRef VarName) {
   if (!needsComdatForCounter(F, M))
     return nullptr;
 
@@ -370,10 +371,10 @@
   // 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());
-  return M.getOrInsertComdat(StringRef(getVarName(Inc, ComdatPrefix)));
+  StringRef ComdatName = (Triple(M.getTargetTriple()).isOSBinFormatCOFF()
+                                ? VarName
+                                : getVarName(Inc, getInstrProfComdatPrefix()));
+  return M.getOrInsertComdat(ComdatName);
 }
 
 static bool needsRuntimeRegistrationOfSectionRange(const Module &M) {
@@ -406,9 +407,6 @@
   // 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);
-
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
   LLVMContext &Ctx = M->getContext();
   ArrayType *CounterTy = ArrayType::get(Type::getInt64Ty(Ctx), NumCounters);
@@ -422,7 +420,8 @@
   CounterPtr->setSection(
       getInstrProfSectionName(IPSK_cnts, TT.getObjectFormat()));
   CounterPtr->setAlignment(8);
-  CounterPtr->setComdat(ProfileVarsComdat);
+  CounterPtr->setComdat(getOrCreateProfileComdat(*M, *Fn, Inc,
+                                                 CounterPtr->getName()));
 
   auto *Int8PtrTy = Type::getInt8PtrTy(Ctx);
   // Allocate statically the array of pointers to value profile nodes for
@@ -443,7 +442,8 @@
       ValuesVar->setSection(
           getInstrProfSectionName(IPSK_vals, TT.getObjectFormat()));
       ValuesVar->setAlignment(8);
-      ValuesVar->setComdat(ProfileVarsComdat);
+      ValuesVar->setComdat(getOrCreateProfileComdat(*M, *Fn, Inc,
+                                                    ValuesVar->getName()));
       ValuesPtrExpr =
           ConstantExpr::getBitCast(ValuesVar, Type::getInt8PtrTy(Ctx));
     }
@@ -476,7 +476,7 @@
   Data->setVisibility(NamePtr->getVisibility());
   Data->setSection(getInstrProfSectionName(IPSK_data, TT.getObjectFormat()));
   Data->setAlignment(INSTR_PROF_DATA_ALIGNMENT);
-  Data->setComdat(ProfileVarsComdat);
+  Data->setComdat(getOrCreateProfileComdat(*M, *Fn, Inc, Data->getName()));
 
   PD.RegionCounters = CounterPtr;
   PD.DataVar = Data;


["D32688.97244.patch" (text/x-patch)]

Index: lib/Transforms/Instrumentation/InstrProfiling.cpp
===================================================================
--- lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -362,7 +362,8 @@
 }
 
 static inline Comdat *getOrCreateProfileComdat(Module &M, Function &F,
-                                               InstrProfIncrementInst *Inc) {
+                                               InstrProfIncrementInst *Inc,
+                                               StringRef VarName) {
   if (!needsComdatForCounter(F, M))
     return nullptr;
 
@@ -370,10 +371,10 @@
   // 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());
-  return M.getOrInsertComdat(StringRef(getVarName(Inc, ComdatPrefix)));
+  StringRef ComdatName = (Triple(M.getTargetTriple()).isOSBinFormatCOFF()
+                                ? VarName
+                                : getVarName(Inc, getInstrProfComdatPrefix()));
+  return M.getOrInsertComdat(ComdatName);
 }
 
 static bool needsRuntimeRegistrationOfSectionRange(const Module &M) {
@@ -406,9 +407,6 @@
   // 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);
-
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
   LLVMContext &Ctx = M->getContext();
   ArrayType *CounterTy = ArrayType::get(Type::getInt64Ty(Ctx), NumCounters);
@@ -422,7 +420,8 @@
   CounterPtr->setSection(
       getInstrProfSectionName(IPSK_cnts, TT.getObjectFormat()));
   CounterPtr->setAlignment(8);
-  CounterPtr->setComdat(ProfileVarsComdat);
+  CounterPtr->setComdat(getOrCreateProfileComdat(*M, *Fn, Inc,
+                                                 CounterPtr->getName()));
 
   auto *Int8PtrTy = Type::getInt8PtrTy(Ctx);
   // Allocate statically the array of pointers to value profile nodes for
@@ -443,7 +442,8 @@
       ValuesVar->setSection(
           getInstrProfSectionName(IPSK_vals, TT.getObjectFormat()));
       ValuesVar->setAlignment(8);
-      ValuesVar->setComdat(ProfileVarsComdat);
+      ValuesVar->setComdat(getOrCreateProfileComdat(*M, *Fn, Inc,
+                                                    ValuesVar->getName()));
       ValuesPtrExpr =
           ConstantExpr::getBitCast(ValuesVar, Type::getInt8PtrTy(Ctx));
     }
@@ -476,7 +476,7 @@
   Data->setVisibility(NamePtr->getVisibility());
   Data->setSection(getInstrProfSectionName(IPSK_data, TT.getObjectFormat()));
   Data->setAlignment(INSTR_PROF_DATA_ALIGNMENT);
-  Data->setComdat(ProfileVarsComdat);
+  Data->setComdat(getOrCreateProfileComdat(*M, *Fn, Inc, Data->getName()));
 
   PD.RegionCounters = CounterPtr;
   PD.DataVar = Data;

[Attachment #4 (text/plain)]

_______________________________________________
llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


[prev in list] [next in list] [prev in thread] [next in thread]