[PATCH] D90106: [JumpThreading] Set edge probabilities when creating basic blocks

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D90106#2357780 <https://reviews.llvm.org/D90106#2357780>, @dblaikie wrote:

> Probably would'ev been worth waiting to check that the two reviewers with the question found the answer addressed their concerns (& getting a clear approval) before committing. (bit awkward to commit things that haven't been formally approved - though I realize @MaskRay's comment may be close enough to be construed as approval)

Thanks @dblaikie!



================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:2517
+  // NewBB has exactly one successor.
+  SmallVector<BranchProbability, 1> NewBBSuccProbs;
+  NewBBSuccProbs.push_back(BranchProbability::getOne());
----------------
With D90272, do we still need this chunk?

getEdgeProbability by default knows the probability is 1.


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:2734
+    // NewBB has exactly one successor.
+    SmallVector<BranchProbability, 1> BBSuccProbs;
+    BBSuccProbs.push_back(BranchProbability::getOne());
----------------
With D90272, do we still need this chunk?

getEdgeProbability by default knows the probability is 1.


================
Comment at: llvm/test/Transforms/JumpThreading/thread-prob-2.ll:1
+; RUN: opt -debug-only=branch-prob -jump-threading -S %s 2>&1 | FileCheck %s
+
----------------
debug-only tests require `REQUIRES: asserts`

The 3 new tests are useful but I wonder -debug-only=branch-prob is the best way testing them. Such tests should also be used prudently as debug only codes tends to be very specific internal details.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90106