Revert "[InstrProf] Attach debug info to counters"

ellishg · ellishg · commit c809da7d9ce7 · 2021-12-13T18:15:17.000-08:00
This reverts commit 800bf8e. The `Instrumentation/InstrProfiling/debug-info-correlate.ll` test was failing because I forgot the `llc` commands are architecture specific. I'll follow up with a fix. Differential Revision: https://reviews.llvm.org/D115689
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
@@ -94,17 +94,10 @@ using namespace llvm;
   llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
 #include "llvm/Support/Extension.def"
 
-namespace llvm {
-extern cl::opt<bool> DebugInfoCorrelate;
-}
-
 namespace {
 
 // Default filename used for profile generation.
-Twine getDefaultProfileGenName() {
-  const Twine Extension = DebugInfoCorrelate ? "proflite" : "profraw";
-  return "default_%m." + Extension;
-}
+static constexpr StringLiteral DefaultProfileGenName = "default_%m.profraw";
 
 class EmitAssemblyHelper {
   DiagnosticsEngine &Diags;
@@ -893,7 +886,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
     if (!CodeGenOpts.InstrProfileOutput.empty())
       PMBuilder.PGOInstrGen = CodeGenOpts.InstrProfileOutput;
     else
-      PMBuilder.PGOInstrGen = getDefaultProfileGenName().str();
+      PMBuilder.PGOInstrGen = std::string(DefaultProfileGenName);
   }
   if (CodeGenOpts.hasProfileIRUse()) {
     PMBuilder.PGOInstrUse = CodeGenOpts.ProfileInstrumentUsePath;
@@ -1238,7 +1231,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   if (CodeGenOpts.hasProfileIRInstr())
     // -fprofile-generate.
     PGOOpt = PGOOptions(CodeGenOpts.InstrProfileOutput.empty()
-                            ? getDefaultProfileGenName().str()
+                            ? std::string(DefaultProfileGenName)
                             : CodeGenOpts.InstrProfileOutput,
                         "", "", PGOOptions::IRInstr, PGOOptions::NoCSAction,
                         CodeGenOpts.DebugInfoForProfiling);
@@ -1276,13 +1269,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
              "Cannot run CSProfileGen pass with ProfileGen or SampleUse "
              " pass");
       PGOOpt->CSProfileGenFile = CodeGenOpts.InstrProfileOutput.empty()
-                                     ? getDefaultProfileGenName().str()
+                                     ? std::string(DefaultProfileGenName)
                                      : CodeGenOpts.InstrProfileOutput;
       PGOOpt->CSAction = PGOOptions::CSIRInstr;
     } else
       PGOOpt = PGOOptions("",
                           CodeGenOpts.InstrProfileOutput.empty()
-                              ? getDefaultProfileGenName().str()
+                              ? std::string(DefaultProfileGenName)
                               : CodeGenOpts.InstrProfileOutput,
                           "", PGOOptions::NoAction, PGOOptions::CSIRInstr,
                           CodeGenOpts.DebugInfoForProfiling);
diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
@@ -653,17 +653,15 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
- * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
+ * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentaiton
  * generated profile, and 0 if this is a Clang FE generated profile.
  * 1 in bit 57 indicates there are context-sensitive records in the profile.
- * The 59th bit indicates whether to use debug info to correlate profiles.
  */
 #define VARIANT_MASKS_ALL 0xff00000000000000ULL
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
 #define VARIANT_MASK_INSTR_ENTRY (0x1ULL << 58)
-#define VARIANT_MASK_DBG_CORRELATE (0x1ULL << 59)
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
 #define INSTR_PROF_PROFILE_COUNTER_BIAS_VAR __llvm_profile_counter_bias
diff --git a/compiler-rt/lib/profile/InstrProfiling.c b/compiler-rt/lib/profile/InstrProfiling.c
@@ -38,7 +38,7 @@ __llvm_profile_get_num_padding_bytes(uint64_t SizeInBytes) {
 }
 
 COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_version(void) {
-  return INSTR_PROF_RAW_VERSION_VAR;
+  return __llvm_profile_raw_version;
 }
 
 COMPILER_RT_VISIBILITY void __llvm_profile_reset_counters(void) {
diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c b/compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -95,11 +95,6 @@ static uintptr_t signextIfWin64(void *V) {
 COMPILER_RT_VISIBILITY
 int __llvm_profile_merge_from_buffer(const char *ProfileData,
                                      uint64_t ProfileSize) {
-  int DebugInfoCorrelate =
-      (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) != 0ULL;
-  if (DebugInfoCorrelate)
-    return 1;
-
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
   uint64_t *SrcCountersStart;
diff --git a/compiler-rt/lib/profile/InstrProfilingWriter.c b/compiler-rt/lib/profile/InstrProfilingWriter.c
@@ -259,19 +259,16 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
                    const uint64_t *CountersBegin, const uint64_t *CountersEnd,
                    VPDataReaderType *VPDataReader, const char *NamesBegin,
                    const char *NamesEnd, int SkipNameDataWrite) {
-  int DebugInfoCorrelate =
-      (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) != 0ULL;
 
   /* Calculate size of sections. */
-  const uint64_t DataSize =
-      DebugInfoCorrelate ? 0 : __llvm_profile_get_data_size(DataBegin, DataEnd);
+  const uint64_t DataSize = __llvm_profile_get_data_size(DataBegin, DataEnd);
   const uint64_t CountersSize = CountersEnd - CountersBegin;
-  const uint64_t NamesSize = DebugInfoCorrelate ? 0 : NamesEnd - NamesBegin;
+  const uint64_t NamesSize = NamesEnd - NamesBegin;
 
   /* Create the header. */
   __llvm_profile_header Header;
 
-  if (!DataSize && (!DebugInfoCorrelate || !CountersSize))
+  if (!DataSize)
     return 0;
 
   /* Determine how much padding is needed before/after the counters and after
@@ -292,12 +289,6 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
   Header.CountersDelta = (uint32_t)Header.CountersDelta;
 #endif
 
-  /* The data and names sections are omitted in lightweight mode. */
-  if (DebugInfoCorrelate) {
-    Header.CountersDelta = 0;
-    Header.NamesDelta = 0;
-  }
-
   /* Write the profile header. */
   ProfDataIOVec IOVec[] = {{&Header, sizeof(__llvm_profile_header), 1, 0}};
   if (Writer->Write(Writer, IOVec, sizeof(IOVec) / sizeof(*IOVec)))
@@ -309,13 +300,11 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
 
   /* Write the profile data. */
   ProfDataIOVec IOVecData[] = {
-      {DebugInfoCorrelate ? NULL : DataBegin, sizeof(__llvm_profile_data),
-       DataSize, 0},
+      {DataBegin, sizeof(__llvm_profile_data), DataSize, 0},
       {NULL, sizeof(uint8_t), PaddingBytesBeforeCounters, 1},
       {CountersBegin, sizeof(uint64_t), CountersSize, 0},
       {NULL, sizeof(uint8_t), PaddingBytesAfterCounters, 1},
-      {(SkipNameDataWrite || DebugInfoCorrelate) ? NULL : NamesBegin,
-       sizeof(uint8_t), NamesSize, 0},
+      {SkipNameDataWrite ? NULL : NamesBegin, sizeof(uint8_t), NamesSize, 0},
       {NULL, sizeof(uint8_t), PaddingBytesAfterNames, 1}};
   if (Writer->Write(Writer, IOVecData, sizeof(IOVecData) / sizeof(*IOVecData)))
     return -1;
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -1149,8 +1149,7 @@ void getMemOPSizeRangeFromOption(StringRef Str, int64_t &RangeStart,
 // Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
 // aware this is an ir_level profile so it can set the version flag.
 GlobalVariable *createIRLevelProfileFlagVar(Module &M, bool IsCS,
-                                            bool InstrEntryBBEnabled,
-                                            bool DebugInfoCorrelate);
+                                            bool InstrEntryBBEnabled);
 
 // Create the variable for the profile file name.
 void createProfileFileNameVar(Module &M, StringRef InstrProfileOutput);
diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -653,17 +653,15 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
- * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentation
+ * bits (i.e. bit 56) to 1 to indicate if this is an IR-level instrumentaiton
  * generated profile, and 0 if this is a Clang FE generated profile.
  * 1 in bit 57 indicates there are context-sensitive records in the profile.
- * The 59th bit indicates whether to use debug info to correlate profiles.
  */
 #define VARIANT_MASKS_ALL 0xff00000000000000ULL
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
 #define VARIANT_MASK_INSTR_ENTRY (0x1ULL << 58)
-#define VARIANT_MASK_DBG_CORRELATE (0x1ULL << 59)
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
 #define INSTR_PROF_PROFILE_COUNTER_BIAS_VAR __llvm_profile_counter_bias
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -842,17 +842,13 @@ void DwarfUnit::addAnnotation(DIE &Buffer, DINodeArray Annotations) {
   for (const Metadata *Annotation : Annotations->operands()) {
     const MDNode *MD = cast<MDNode>(Annotation);
     const MDString *Name = cast<MDString>(MD->getOperand(0));
-    const auto &Value = MD->getOperand(1);
+
+    // Currently, only MDString is supported with btf_decl_tag attribute.
+    const MDString *Value = cast<MDString>(MD->getOperand(1));
 
     DIE &AnnotationDie = createAndAddDIE(dwarf::DW_TAG_LLVM_annotation, Buffer);
     addString(AnnotationDie, dwarf::DW_AT_name, Name->getString());
-    if (const auto *Data = dyn_cast<MDString>(Value))
-      addString(AnnotationDie, dwarf::DW_AT_const_value, Data->getString());
-    else if (const auto *Data = dyn_cast<ConstantAsMetadata>(Value))
-      addConstantValue(AnnotationDie, Data->getValue()->getUniqueInteger(),
-                       /*Unsigned=*/true);
-    else
-      assert(false && "Unsupported annotation value type");
+    addString(AnnotationDie, dwarf::DW_AT_const_value, Value->getString());
   }
 }
 
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1175,17 +1175,14 @@ bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) {
 // Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
 // aware this is an ir_level profile so it can set the version flag.
 GlobalVariable *createIRLevelProfileFlagVar(Module &M, bool IsCS,
-                                            bool InstrEntryBBEnabled,
-                                            bool DebugInfoCorrelate) {
+                                            bool InstrEntryBBEnabled) {
   const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
   Type *IntTy64 = Type::getInt64Ty(M.getContext());
   uint64_t ProfileVersion = (INSTR_PROF_RAW_VERSION | VARIANT_MASK_IR_PROF);
   if (IsCS)
     ProfileVersion |= VARIANT_MASK_CSIR_PROF;
   if (InstrEntryBBEnabled)
     ProfileVersion |= VARIANT_MASK_INSTR_ENTRY;
-  if (DebugInfoCorrelate)
-    ProfileVersion |= VARIANT_MASK_DBG_CORRELATE;
   auto IRLevelVersionVariable = new GlobalVariable(
       M, IntTy64, true, GlobalValue::WeakAnyLinkage,
       Constant::getIntegerValue(IntTy64, APInt(64, ProfileVersion)), VarName);
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -32,7 +32,6 @@
 #include <vector>
 
 using namespace llvm;
-extern cl::opt<bool> DebugInfoCorrelate;
 
 // A struct to define how the data stream should be patched. For Indexed
 // profiling, only uint64_t data type is needed.
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -26,9 +26,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
-#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DerivedTypes.h"
-#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
@@ -59,13 +57,6 @@ using namespace llvm;
 
 #define DEBUG_TYPE "instrprof"
 
-namespace llvm {
-cl::opt<bool>
-    DebugInfoCorrelate("debug-info-correlate", cl::ZeroOrMore,
-                       cl::desc("Use debug info to correlate profiles."),
-                       cl::init(false));
-} // namespace llvm
-
 namespace {
 
 cl::opt<bool> DoHashBasedCounterSplit(
@@ -650,12 +641,6 @@ void InstrProfiling::computeNumValueSiteCounts(InstrProfValueProfileInst *Ind) {
 }
 
 void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
-  // TODO: Value profiling heavily depends on the data section which is omitted
-  // in lightweight mode. We need to move the value profile pointer to the
-  // Counter struct to get this working.
-  assert(
-      !DebugInfoCorrelate &&
-      "Value profiling is not yet supported with lightweight instrumentation");
   GlobalVariable *Name = Ind->getName();
   auto It = ProfileDataMap.find(Name);
   assert(It != ProfileDataMap.end() && It->second.DataVar &&
@@ -870,12 +855,6 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
   GlobalValue::LinkageTypes Linkage = NamePtr->getLinkage();
   GlobalValue::VisibilityTypes Visibility = NamePtr->getVisibility();
 
-  // Use internal rather than private linkage so the counter variable shows up
-  // in the symbol table when using debug info for correlation.
-  if (DebugInfoCorrelate && TT.isOSBinFormatMachO() &&
-      Linkage == GlobalValue::PrivateLinkage)
-    Linkage = GlobalValue::InternalLinkage;
-
   // Due to the limitation of binder as of 2021/09/28, the duplicate weak
   // symbols in the same csect won't be discarded. When there are duplicate weak
   // symbols, we can NOT guarantee that the relocations get resolved to the
@@ -937,42 +916,6 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
   MaybeSetComdat(CounterPtr);
   CounterPtr->setLinkage(Linkage);
   PD.RegionCounters = CounterPtr;
-  if (DebugInfoCorrelate) {
-    if (auto *SP = Fn->getSubprogram()) {
-      DIBuilder DB(*M, true, SP->getUnit());
-      Metadata *FunctionNameAnnotation[] = {
-          MDString::get(Ctx, "Function Name"),
-          MDString::get(Ctx, getPGOFuncNameVarInitializer(NamePtr)),
-      };
-      Metadata *CFGHashAnnotation[] = {
-          MDString::get(Ctx, "CFG Hash"),
-          ConstantAsMetadata::get(Inc->getHash()),
-      };
-      Metadata *NumCountersAnnotation[] = {
-          MDString::get(Ctx, "Num Counters"),
-          ConstantAsMetadata::get(Inc->getNumCounters()),
-      };
-      auto Annotations = DB.getOrCreateArray({
-          MDNode::get(Ctx, FunctionNameAnnotation),
-          MDNode::get(Ctx, CFGHashAnnotation),
-          MDNode::get(Ctx, NumCountersAnnotation),
-      });
-      auto *DICounter = DB.createGlobalVariableExpression(
-          SP, CounterPtr->getName(), /*LinkageName=*/StringRef(), SP->getFile(),
-          /*LineNo=*/0, DB.createUnspecifiedType("Profile Data Type"),
-          CounterPtr->hasLocalLinkage(), /*IsDefined=*/true, /*Expr=*/nullptr,
-          /*Decl=*/nullptr, /*TemplateParams=*/nullptr, /*AlignInBits=*/0,
-          Annotations);
-      CounterPtr->addDebugInfo(DICounter);
-      DB.finalize();
-    } else {
-      std::string Msg = ("Missing debug info for function " + Fn->getName() +
-                         "; required for profile correlation.")
-                            .str();
-      Ctx.diagnose(
-          DiagnosticInfoPGOProfile(M->getName().data(), Msg, DS_Warning));
-    }
-  }
 
   auto *Int8PtrTy = Type::getInt8PtrTy(Ctx);
   // Allocate statically the array of pointers to value profile nodes for
@@ -996,9 +939,6 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
         ConstantExpr::getBitCast(ValuesVar, Type::getInt8PtrTy(Ctx));
   }
 
-  if (DebugInfoCorrelate)
-    return PD.RegionCounters;
-
   // Create data variable.
   auto *IntPtrTy = M->getDataLayout().getIntPtrType(M->getContext());
   auto *Int16Ty = Type::getInt16Ty(Ctx);
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -291,8 +291,6 @@ extern cl::opt<PGOViewCountsType> PGOViewCounts;
 // Command line option to specify the name of the function for CFG dump
 // Defined in Analysis/BlockFrequencyInfo.cpp:  -view-bfi-func-name=
 extern cl::opt<std::string> ViewBlockFreqFuncName;
-
-extern cl::opt<bool> DebugInfoCorrelate;
 } // namespace llvm
 
 static cl::opt<bool>
@@ -469,9 +467,8 @@ class PGOInstrumentationGenCreateVarLegacyPass : public ModulePass {
     createProfileFileNameVar(M, InstrProfileOutput);
     // The variable in a comdat may be discarded by LTO. Ensure the
     // declaration will be retained.
-    appendToCompilerUsed(M, createIRLevelProfileFlagVar(M, /*IsCS=*/true,
-                                                        PGOInstrumentEntry,
-                                                        DebugInfoCorrelate));
+    appendToCompilerUsed(
+        M, createIRLevelProfileFlagVar(M, /*IsCS=*/true, PGOInstrumentEntry));
     return false;
   }
   std::string InstrProfileOutput;
@@ -1619,8 +1616,7 @@ static bool InstrumentAllFunctions(
   // For the context-sensitve instrumentation, we should have a separated pass
   // (before LTO/ThinLTO linking) to create these variables.
   if (!IsCS)
-    createIRLevelProfileFlagVar(M, /*IsCS=*/false, PGOInstrumentEntry,
-                                DebugInfoCorrelate);
+    createIRLevelProfileFlagVar(M, /*IsCS=*/false, PGOInstrumentEntry);
   std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
   collectComdatMembers(M, ComdatMembers);
 
@@ -1642,9 +1638,8 @@ PGOInstrumentationGenCreateVar::run(Module &M, ModuleAnalysisManager &AM) {
   createProfileFileNameVar(M, CSInstrName);
   // The variable in a comdat may be discarded by LTO. Ensure the declaration
   // will be retained.
-  appendToCompilerUsed(M, createIRLevelProfileFlagVar(M, /*IsCS=*/true,
-                                                      PGOInstrumentEntry,
-                                                      DebugInfoCorrelate));
+  appendToCompilerUsed(
+      M, createIRLevelProfileFlagVar(M, /*IsCS=*/true, PGOInstrumentEntry));
   return PreservedAnalyses::all();
 }
 
diff --git a/llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll b/llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ __llvm_profile_get_num_padding_bytes(uint64_t SizeInBytes) {`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_version(void) {`
`41`		`- return INSTR_PROF_RAW_VERSION_VAR;`
	`41`	`+ return __llvm_profile_raw_version;`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`COMPILER_RT_VISIBILITY void __llvm_profile_reset_counters(void) {`