[PATCH] D144500: [BOLT] stale profile matching [part 1 out of 2]

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb accepted this revision.
maksfb added a comment.
This revision is now accepted and ready to land.

LGTM. See comments with mostly nits. We can probably be more graceful while mapping basic blocks, but this issue can be addressed separately.



================
Comment at: bolt/lib/Profile/StaleProfileMatching.cpp:200-201
+    if (BB->isEntryPoint() || InDegree[I] == 0) {
+      uint64_t Src = 0;
+      uint64_t Dst = I;
+      Func.Jumps.emplace_back();
----------------
nit: remove those variables


================
Comment at: bolt/lib/Profile/StaleProfileMatching.cpp:213-214
+  for (FlowJump &Jump : Func.Jumps) {
+    uint64_t Src = Jump.Source;
+    uint64_t Dst = Jump.Target;
+    Func.Blocks.at(Src).SuccJumps.push_back(&Jump);
----------------
likewise


================
Comment at: bolt/lib/Profile/StaleProfileMatching.cpp:332
+    for (FlowJump *Jump : Func.Blocks[Src].SuccJumps) {
+      uint64_t Dst = Jump->Target;
+      if (!VisitedEntry[Dst]) {
----------------



================
Comment at: bolt/lib/Profile/StaleProfileMatching.cpp:465-467
+      if (BC.MIB->hasAnnotation(Instr, Name)) {
+        BC.MIB->removeAnnotation(Instr, Name);
+      }
----------------
nit:


================
Comment at: bolt/lib/Profile/StaleProfileMatching.cpp:488-489
+          // Try to evenly distribute the counts among the call sites
+          uint64_t TotalCount = Block.Flow;
+          uint64_t NumSites = ICSP.size();
+          for (uint64_t Idx = 0; Idx < ICSP.size(); Idx++) {
----------------
nit:


================
Comment at: bolt/lib/Profile/StaleProfileMatching.cpp:524
+
+  BinaryFunction::BasicBlockOrderType BlockOrder(BF.getLayout().block_begin(),
+                                                 BF.getLayout().block_end());
----------------
Will this work?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144500/new/

https://reviews.llvm.org/D144500