[PATCH] D103289: A post-processing for BFI inference

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h:993
+  void iterativeInference(
+      const std::vector<std::vector<std::pair<size_t, Scaled64>>> &ProbMatrix,
+      std::vector<Scaled64> &Freq) const;
----------------
Nit: how about giving the type a name, like `using ProbMatrixType = std::vector<std::vector<std::pair<size_t, Scaled64>>>;` ?



================
Comment at: llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h:1495
+    if (Change > Threshold) {
+      ActiveSet.push(I);
+      IsActive[I] = true;
----------------
Wondering if it makes sense to not set `I` active. When `I` gets an noticeable update on its counts, its successors should be reprocessed thus they should be set active. But not sure `I` itself should be reprocessed.


================
Comment at: llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h:1595
+      // Ignore parallel edges between BB and SI blocks
+      if (UniqueSuccs.find(SI) != UniqueSuccs.end())
+        continue;
----------------
Should the probability of parallel edges be accumulated?


================
Comment at: llvm/test/Transforms/SampleProfile/profile-correlation-irreducible-loops.ll:2
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/profile-correlation-irreducible-loops.prof | opt -analyze -block-freq -enable-new-pm=0  -use-iterative-bfi-inference | FileCheck %s
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/profile-correlation-irreducible-loops.prof -S | FileCheck %s --check-prefix=CHECK2
+; RUN: opt < %s -analyze -block-freq -enable-new-pm=0  -use-iterative-bfi-inference | FileCheck %s --check-prefix=CHECK3
----------------
The pseudo-probe pass is probably not needed since the test IR comes with pseudo probes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103289