[PGO] Create the profile data variable before the lowering

[xur]

Summary

This patch creates the profile data variable before lowering the profile intrinsics (i.e. InstrProfIncrementInst and InstrProfValueProfileInst).

This is for IR level instrumentation. Lowering InstrProfValueProfileInst assumes the existing of profile data variable. For clang based instrumentation, we always see InstrProfIncrementInst before the first InstrProfValueProfileInst. But this is not true for IR level instrumeatnion (as we may not instrument the entry BB).

[davidxl]

lib/Transforms/Instrumentation/InstrProfiling.cpp

155 ↗

(On Diff #44382)

Why not fuse this loop with the above one?

[betulb]

lib/Transforms/Instrumentation/InstrProfiling.cpp

158 ↗

(On Diff #44382)

if (!CounterVarSet) in the above line is redundant.

[xur]

Changed the patch based on comments.

[davidxl]

lib/Transforms/Instrumentation/InstrProfiling.cpp

141 ↗

(On Diff #45026)

Remove the comments here.

154 ↗

(On Diff #45026)

Why is this flag needed?

161 ↗

(On Diff #45026)

Add a brief comment here: Value profiling intrinsic lowering requires per-function profile data variable to be created first.

[xur]

revised patch based on David's comments.

[davidxl]

LGTM

[silvas]

LGTM with a nit.

lib/Transforms/Instrumentation/InstrProfiling.cpp

152 ↗

(On Diff #45033)

Cast the result to void to indicate that we are throwing away the return value of this function (i.e. we are only calling it for the side effects).

llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp

bool InstrProfiling::runOnModule(Module &M) {
 
   this->M = &M;
   ProfileDataMap.clear();
   UsedVars.clear();
 
   // We did not know how many value sites there would be inside
   // the instrumented function. This is counting the number of instrumented
   // target value sites to enter it as field in the profile data variable.
-  for (Function &F : M)
+  for (Function &F : M) {
+    InstrProfIncrementInst *FirstProfIncInst = nullptr;
     for (BasicBlock &BB : F)
-      for (auto I = BB.begin(), E = BB.end(); I != E;)
-        if (auto *Ind = dyn_cast<InstrProfValueProfileInst>(I++))
+      for (auto I = BB.begin(), E = BB.end(); I != E; I++)
+        if (auto *Ind = dyn_cast<InstrProfValueProfileInst>(I))
           computeNumValueSiteCounts(Ind);
+        else if (FirstProfIncInst == nullptr)
+          FirstProfIncInst = dyn_cast<InstrProfIncrementInst>(I);
+
+    // Value profiling intrinsic lowering requires per-function profile data
+    // variable to be created first.
+    if (FirstProfIncInst != nullptr)
+      static_cast<void>(getOrCreateRegionCounters(FirstProfIncInst));
+  }
 
   for (Function &F : M)
     for (BasicBlock &BB : F)
       for (auto I = BB.begin(), E = BB.end(); I != E;) {
         auto Instr = I++;
         if (auto *Inc = dyn_cast<InstrProfIncrementInst>(Instr)) {
           lowerIncrement(Inc);
           MadeChange = true;