[PATCH] D125448: [llvm-profgen] Filter out oversized LBR ranges.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:213
 
+inline bool isValidLBRRange(uint64_t Start, uint64_t End,
+                            ProfiledBinary *Binary) {
----------------
nit: `isValidLBRRange`->`isValidFallThroughRange`.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:100-106
+  uint64_t getMaxFuncRangeSize() {
+    uint64_t Size = 0;
+    for (auto &R : Ranges) {
+      Size = std::max(Size, R.second - R.first);
+    }
+    return Size;
+  }
----------------
I think we can skip the max func size check for simplicity. 99%+ ranges will go through uncond branch check still, so this extra complexity doesn't seem worth it. 


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:252
+  // A set of unconditional branch instruction offsets.
+  std::set<uint64_t> UncondBranchOffsets;
   // A set of branch instruction offsets.
----------------
How about keep `CodeAddrOffsets` and `CallOffsets` as `unordered_set` so other queries are still O(1), but have additional `UncondBranchOffsets` as `set` that contains all calls and rets. This also avoids querying multiple containers for `isValidLBRRange`. The size overhead should be small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125448