[InstrProfiling] Ensure data variables are always created for inlined functions (llvm#72069)

Fixes a bug introduced by
commit f95b2f1 ("Reland [InstrProf][compiler-rt] Enable MC/DC
Support in LLVM Source-based Code Coverage (1/3)")

The InstrProfiling pass was refactored when introducing support for
MC/DC such that the creation of the data variable was abstracted and
called only once per function from ::run(). Because ::run() only
iterated over functions there were not fully inlined, and because it
only created the data variable for the first intrinsic that it saw, data
variables corresponding to functions fully inlined into other
instrumented callers would end up without a data variable, resulting in
loss of coverage information. This patch does the following:

1.) Move the call of createDataVariable() to getOrCreateRegionCounters()
so that the creation of the data variable will happen indirectly either
from ::new() or during profile intrinsic lowering when it is needed.
This effectively restores the behavior prior to the refactor and ensures
that all data variables are created when needed (and not duplicated).

2.) Process all MC/DC bitmap parameter intrinsics in ::run() prior to
calling getOrCreateRegionCounters(). This ensures bitmap regions are
created for each function including functions that are fully inlined. It
also ensures that the bitmap region is created for each function prior
to the creation of the data variable because it is referenced by the
data variable. Again, duplication is prevented if the same parameter
intrinsic is inlined into multiple functions.

3.) No longer pass the MC/DC intrinsic to createDataVariable(). This
decouples the creation of the data variable from a specific MC/DC
intrinsic. Instead, with #2 above, store the number of bitmap bytes
required in the PerFunctionProfileData in the ProfileDataMap along with
the function's CounterRegion and BitmapRegion variables. This ties the
bitmap information directly to the function to which it belongs, and the
data variable created for that function can reference that.

Author: evodius96

llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h

@@ -51,6 +51,7 @@ class InstrProfiling : public PassInfoMixin<InstrProfiling> {
     GlobalVariable *RegionCounters = nullptr;
     GlobalVariable *DataVar = nullptr;
     GlobalVariable *RegionBitmaps = nullptr;
+    uint32_t NumBitmapBytes = 0;
 
     PerFunctionProfileData() {
       memset(NumValueSites, 0, sizeof(uint32_t) * (IPVK_Last + 1));
@@ -156,8 +157,7 @@ class InstrProfiling : public PassInfoMixin<InstrProfiling> {
                                       InstrProfSectKind IPSK);
 
   /// Create INSTR_PROF_DATA variable for counters and bitmaps.
-  void createDataVariable(InstrProfCntrInstBase *Inc,
-                          InstrProfMCDCBitmapParameters *Update);
+  void createDataVariable(InstrProfCntrInstBase *Inc);
 
   /// Emit the section with compressed function names.
   void emitNameData();

llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

@@ -556,7 +556,6 @@ bool InstrProfiling::run(
   // target value sites to enter it as field in the profile data variable.
   for (Function &F : M) {
     InstrProfCntrInstBase *FirstProfInst = nullptr;
-    InstrProfMCDCBitmapParameters *FirstProfMCDCParams = nullptr;
     for (BasicBlock &BB : F) {
       for (auto I = BB.begin(), E = BB.end(); I != E; I++) {
         if (auto *Ind = dyn_cast<InstrProfValueProfileInst>(I))
@@ -565,22 +564,17 @@ bool InstrProfiling::run(
           if (FirstProfInst == nullptr &&
               (isa<InstrProfIncrementInst>(I) || isa<InstrProfCoverInst>(I)))
             FirstProfInst = dyn_cast<InstrProfCntrInstBase>(I);
-          if (FirstProfMCDCParams == nullptr)
-            FirstProfMCDCParams = dyn_cast<InstrProfMCDCBitmapParameters>(I);
+          // If the MCDCBitmapParameters intrinsic seen, create the bitmaps.
+          if (const auto &Params = dyn_cast<InstrProfMCDCBitmapParameters>(I))
+            static_cast<void>(getOrCreateRegionBitmaps(Params));
         }
       }
     }
 
-    // If the MCDCBitmapParameters intrinsic was seen, create the bitmaps.
-    if (FirstProfMCDCParams != nullptr) {
-      static_cast<void>(getOrCreateRegionBitmaps(FirstProfMCDCParams));
-    }
-
     // Use a profile intrinsic to create the region counters and data variable.
     // Also create the data variable based on the MCDCParams.
     if (FirstProfInst != nullptr) {
       static_cast<void>(getOrCreateRegionCounters(FirstProfInst));
-      createDataVariable(FirstProfInst, FirstProfMCDCParams);
     }
   }
 
@@ -1162,6 +1156,7 @@ InstrProfiling::getOrCreateRegionBitmaps(InstrProfMCDCBitmapInstBase *Inc) {
   // the corresponding profile section.
   auto *BitmapPtr = setupProfileSection(Inc, IPSK_bitmap);
   PD.RegionBitmaps = BitmapPtr;
+  PD.NumBitmapBytes = Inc->getNumBitmapBytes()->getZExtValue();
   return PD.RegionBitmaps;
 }
 
@@ -1238,11 +1233,13 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfCntrInstBase *Inc) {
     CompilerUsedVars.push_back(PD.RegionCounters);
   }
 
+  // Create the data variable (if it doesn't already exist).
+  createDataVariable(Inc);
+
   return PD.RegionCounters;
 }
 
-void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc,
-                                        InstrProfMCDCBitmapParameters *Params) {
+void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc) {
   // When debug information is correlated to profile data, a data variable
   // is not needed.
   if (DebugInfoCorrelate)
@@ -1305,9 +1302,7 @@ void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc,
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
   auto *CounterPtr = PD.RegionCounters;
 
-  uint64_t NumBitmapBytes = 0;
-  if (Params != nullptr)
-    NumBitmapBytes = Params->getNumBitmapBytes()->getZExtValue();
+  uint64_t NumBitmapBytes = PD.NumBitmapBytes;
 
   // Create data variable.
   auto *IntPtrTy = M->getDataLayout().getIntPtrType(M->getContext());

llvm/test/Instrumentation/InstrProfiling/inline-data-var-create.ll

@@ -0,0 +1,47 @@
+;; Check that all data variables are created for instrumented functions even
+;; when those functions are fully inlined into their instrumented callers prior
+;; to the instrprof pass.
+; RUN: opt %s -passes='instrprof' -S | FileCheck %s -check-prefix=NOINLINE
+; RUN: opt %s -passes='cgscc(inline),instrprof' -S | FileCheck %s -check-prefix=INLINEFIRST
+; RUN: opt %s -passes='instrprof,cgscc(inline)' -S | FileCheck %s -check-prefix=INLINEAFTER
+
+target triple = "x86_64-unknown-linux-gnu"
+
+; INLINEFIRST: @__profd_foo = private global{{.*}}zeroinitializer, i32 21
+; INLINEFIRST: @__profd_bar = private global{{.*}}zeroinitializer, i32 23
+; INLINEFIRST: @__profd_foobar = private global{{.*}}zeroinitializer, i32 99
+
+; INLINEAFTER: @__profd_foobar = private global{{.*}}zeroinitializer, i32 99
+; INLINEAFTER: @__profd_foo = private global{{.*}}zeroinitializer, i32 21
+; INLINEAFTER: @__profd_bar = private global{{.*}}zeroinitializer, i32 23
+
+; NOINLINE: @__profd_foobar = private global{{.*}}zeroinitializer, i32 99
+; NOINLINE: @__profd_foo = private global{{.*}}zeroinitializer, i32 21
+; NOINLINE: @__profd_bar = private global{{.*}}zeroinitializer, i32 23
+
+declare void @llvm.instrprof.increment(ptr %0, i64 %1, i32 %2, i32 %3)
+declare void @llvm.instrprof.mcdc.parameters(ptr %0, i64 %1, i32 %2)
+@__profn_foobar = private constant [6 x i8] c"foobar"
+@__profn_foo = private constant [3 x i8] c"foo"
+@__profn_bar = private constant [3 x i8] c"bar"
+
+define internal void @foobar() {
+  call void @llvm.instrprof.increment(ptr @__profn_foobar, i64 123456, i32 32, i32 0)
+  call void @llvm.instrprof.mcdc.parameters(ptr @__profn_foobar, i64 123456, i32 99)
+
+  ret void
+}
+
+define void @foo() {
+  call void @llvm.instrprof.increment(ptr @__profn_foo, i64 123456, i32 32, i32 0)
+  call void @llvm.instrprof.mcdc.parameters(ptr @__profn_foo, i64 123456, i32 21)
+  call void @foobar()
+  ret void
+}
+
+define void @bar() {
+  call void @llvm.instrprof.increment(ptr @__profn_bar, i64 123456, i32 32, i32 0)
+  call void @llvm.instrprof.mcdc.parameters(ptr @__profn_bar, i64 123456, i32 23)
+  call void @foobar()
+  ret void
+}