[InstCombine] Update BranchProbabilityAnalysis cache result

[HaohaiWen]

InstCombine may invert branch condition and profile metadata. In such
case, BranchProbabilityAnalysis should also be updated.

[HaohaiWen]

Add test in #86469

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Haohai Wen (HaohaiWen)

Changes

InstCombine may invert branch condition and profile metadata. In such
case, BranchProbabilityAnalysis should be rerun.

---

Full diff: https://github.com/llvm/llvm-project/pull/86470.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+1)
• (added) llvm/test/Transforms/InstCombine/abandon-bpi.ll (+36)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 7c40fb4fc86082..ff7b0b917b712a 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -5314,6 +5314,7 @@ PreservedAnalyses InstCombinePass::run(Function &F,
   // Mark all the analyses that instcombine updates as preserved.
   PreservedAnalyses PA;
   PA.preserveSet<CFGAnalyses>();
+  PA.abandon<BranchProbabilityAnalysis>();
   return PA;
 }
 
diff --git a/llvm/test/Transforms/InstCombine/abandon-bpi.ll b/llvm/test/Transforms/InstCombine/abandon-bpi.ll
new file mode 100644
index 00000000000000..fadb2ab16bff6f
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/abandon-bpi.ll
@@ -0,0 +1,36 @@
+; RUN: opt < %s -passes="print<branch-prob>,instcombine,print<branch-prob>" -S 2>&1 | FileCheck %s
+
+; CHECK:      Printing analysis 'Branch Probability Analysis' for function 'invert_cond':
+; CHECK-NEXT: ---- Branch Probabilities ----
+; CHECK-NEXT:   edge %entry -> %bb1 probability is 0x06186186 / 0x80000000 = 4.76%
+; CHECK-NEXT:   edge %entry -> %bb2 probability is 0x79e79e7a / 0x80000000 = 95.24% [HOT edge]
+; CHECK-NEXT: Printing analysis 'Branch Probability Analysis' for function 'invert_cond':
+; CHECK-NEXT: ---- Branch Probabilities ----
+; CHECK-NEXT:   edge %entry -> %bb2 probability is 0x79e79e7a / 0x80000000 = 95.24% [HOT edge]
+; CHECK-NEXT:   edge %entry -> %bb1 probability is 0x06186186 / 0x80000000 = 4.76%
+
+define i32 @invert_cond(ptr %p) {
+; CHECK-LABEL: define i32 @invert_cond(
+; CHECK-SAME: ptr [[P:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[COND_NOT:%.*]] = icmp eq ptr [[P]], null
+; CHECK-NEXT:    br i1 [[COND_NOT]], label [[BB2:%.*]], label [[BB1:%.*]], !prof [[PROF0:![0-9]+]]
+; CHECK:       bb1:
+; CHECK-NEXT:    ret i32 0
+; CHECK:       bb2:
+; CHECK-NEXT:    ret i32 1
+;
+entry:
+  %cond = icmp ne ptr %p, null
+  br i1 %cond, label %bb1, label %bb2, !prof !1
+
+bb1:
+  ret i32 0;
+
+bb2:
+  ret i32 1;
+}
+
+!1 = !{!"branch_weights", i32 1, i32 20}
+; CHECK: [[PROF0]] = !{!"branch_weights", i32 20, i32 1}
+

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[williamweixiao]

LGTM.

[nikic]

The change InstCombine does to the CFG is trivial -- can we update the analysis instead?

[HaohaiWen]

The change InstCombine does to the CFG is trivial -- can we update the analysis instead?

I'm afraid that's not easy... We many need to check & update cached BranchProbabilityAnalysis in all callsite of BranchInst.swapSuccessors() or similar operation on SwitchInst, JumpTable...

[WenleiHe]

The change InstCombine does to the CFG is trivial -- can we update the analysis instead?

I'm afraid that's not easy... We many need to check & update cached BranchProbabilityAnalysis in all callsite of BranchInst.swapSuccessors() or similar operation on SwitchInst, JumpTable...

Can we use swapSuccEdgesProbabilities of BPI to update for inverted branches? There aren't many callsites for swapSuccessors

[HaohaiWen]

The change InstCombine does to the CFG is trivial -- can we update the analysis instead?

I'm afraid that's not easy... We many need to check & update cached BranchProbabilityAnalysis in all callsite of BranchInst.swapSuccessors() or similar operation on SwitchInst, JumpTable...

Can we use swapSuccEdgesProbabilities of BPI to update for inverted branches? There aren't many callsites for swapSuccessors

Yes we can, also swapValue for SelectInst. There're may be some places where setSuccessor was used and we may need to update them all....

[nikic]

LG