[PATCH] D136827: [JT][CT] Preserve exisiting BPI/BFI during JumpThreading

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:325
+  bool Changed =
+      Impl.runImpl(F, nullptr, TLI, TTI, LVI, AA, &DTU, BFI.get(), BPI.get());
   if (PrintLVIAfterJumpThreading) {
----------------
I guess old PM support is in decline, not sure if we want this here...


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:2506
+  auto BFI = getBFI();
+  auto BPI = getBPI();
+  assert(
----------------
nit: `auto *`


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:3100
+    // BranchProbabilityAnalysis depends on DT. Make sure DT is consistent.
+    DTU->flush();
+    BPI = &FAM->getResult<BranchProbabilityAnalysis>(*F);
----------------
Would be nice to verify DT here in assertion mode.


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:3106
+
+BlockFrequencyInfo *JumpThreadingPass::getOrCreateBFI() {
+  if (HasProfile && (!BFI || *BFI == nullptr)) {
----------------
They are only called together; do we really need two separate functions?


================
Comment at: llvm/test/Transforms/JumpThreading/threading_prof2.ll:42
 ;CHECK: ![[PROF1]] = !{!"branch_weights", i32 1073205, i32 2146410443}
-;CHECK: ![[PROF2]] = !{!"branch_weights", i32 2146410443, i32 1073205}
+;CHECK: ![[PROF2]] = !{!"branch_weights", i32 -2147483648, i32 0}
----------------
This looks strange. What's the explanation here, is the old profile wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136827