[llvm] 95f7f7c - Revert "[SimplifyCFG] use profile metadata to refine merging branch conditions"
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 22 14:48:13 PDT 2021
Author: Sanjay Patel
Date: 2021-03-22T17:48:06-04:00
New Revision: 95f7f7c21b47f1739e4a901d428af970b7d7c0b9
URL: https://github.com/llvm/llvm-project/commit/95f7f7c21b47f1739e4a901d428af970b7d7c0b9
DIFF: https://github.com/llvm/llvm-project/commit/95f7f7c21b47f1739e4a901d428af970b7d7c0b9.diff
LOG: Revert "[SimplifyCFG] use profile metadata to refine merging branch conditions"
This reverts commit 27ae17a6b0145a559c501c35ded0ab4e9dd69e8e.
There are bot failures that end with:
#4 0x00007fff7ae3c9b8 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
#5 0x00007fff84e504d8 (linux-vdso64.so.1+0x4d8)
#6 0x00007fff7c419a5c llvm::TargetTransformInfo::getPredictableBranchThreshold() const (/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/stage1.install/bin/../lib/libLLVMAnalysis.so.13git+0x479a5c)
...but not sure how to trigger that yet.
Added:
Modified:
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/PGOProfile/chr.ll
llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index e3606cf3dd6ad..1f9fa611a9b2e 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -63,7 +63,6 @@
#include "llvm/IR/User.h"
#include "llvm/IR/Value.h"
#include "llvm/IR/ValueHandle.h"
-#include "llvm/Support/BranchProbability.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
@@ -2841,52 +2840,30 @@ static bool extractPredSuccWeights(BranchInst *PBI, BranchInst *BI,
}
}
-/// Determine if the two branches share a common destination and deduce a glue
-/// that joins branch's conditions to arrive at the common destination if that
-/// would be profitable.
+// Determine if the two branches share a common destination,
+// and deduce a glue that we need to use to join branch's conditions
+// to arrive at the common destination.
static Optional<std::pair<Instruction::BinaryOps, bool>>
-shouldFoldCondBranchesToCommonDestination(BranchInst *BI, BranchInst *PBI,
- const TargetTransformInfo *TTI) {
+CheckIfCondBranchesShareCommonDestination(BranchInst *BI, BranchInst *PBI) {
assert(BI && PBI && BI->isConditional() && PBI->isConditional() &&
"Both blocks must end with a conditional branches.");
assert(is_contained(predecessors(BI->getParent()), PBI->getParent()) &&
"PredBB must be a predecessor of BB.");
- // We have the potential to fold the conditions together, but if the
- // predecessor branch is predictable, we may not want to merge them.
- uint64_t PTWeight, PFWeight;
- BranchProbability PBITrueProb, Likely;
- if (PBI->extractProfMetadata(PTWeight, PFWeight) &&
- (PTWeight + PFWeight) != 0) {
- PBITrueProb =
- BranchProbability::getBranchProbability(PTWeight, PTWeight + PFWeight);
- Likely = TTI->getPredictableBranchThreshold();
- }
-
- if (PBI->getSuccessor(0) == BI->getSuccessor(0)) {
- // Speculate the 2nd condition unless the 1st is probably true.
- if (PBITrueProb.isUnknown() || PBITrueProb < Likely)
- return {{Instruction::Or, false}};
- } else if (PBI->getSuccessor(1) == BI->getSuccessor(1)) {
- // Speculate the 2nd condition unless the 1st is probably false.
- if (PBITrueProb.isUnknown() || PBITrueProb.getCompl() < Likely)
- return {{Instruction::And, false}};
- } else if (PBI->getSuccessor(0) == BI->getSuccessor(1)) {
- // Speculate the 2nd condition unless the 1st is probably true.
- if (PBITrueProb.isUnknown() || PBITrueProb < Likely)
- return {{Instruction::And, true}};
- } else if (PBI->getSuccessor(1) == BI->getSuccessor(0)) {
- // Speculate the 2nd condition unless the 1st is probably false.
- if (PBITrueProb.isUnknown() || PBITrueProb.getCompl() < Likely)
- return {{Instruction::Or, true}};
- }
+ if (PBI->getSuccessor(0) == BI->getSuccessor(0))
+ return {{Instruction::Or, false}};
+ else if (PBI->getSuccessor(1) == BI->getSuccessor(1))
+ return {{Instruction::And, false}};
+ else if (PBI->getSuccessor(0) == BI->getSuccessor(1))
+ return {{Instruction::And, true}};
+ else if (PBI->getSuccessor(1) == BI->getSuccessor(0))
+ return {{Instruction::Or, true}};
return None;
}
-static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
+static bool PerformBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
DomTreeUpdater *DTU,
- MemorySSAUpdater *MSSAU,
- const TargetTransformInfo *TTI) {
+ MemorySSAUpdater *MSSAU) {
BasicBlock *BB = BI->getParent();
BasicBlock *PredBlock = PBI->getParent();
@@ -2894,7 +2871,7 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
Instruction::BinaryOps Opc;
bool InvertPredCond;
std::tie(Opc, InvertPredCond) =
- *shouldFoldCondBranchesToCommonDestination(BI, PBI, TTI);
+ *CheckIfCondBranchesShareCommonDestination(BI, PBI);
LLVM_DEBUG(dbgs() << "FOLDING BRANCH TO COMMON DEST:\n" << *PBI << *BB);
@@ -3082,8 +3059,8 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
// Determine if the two branches share a common destination.
Instruction::BinaryOps Opc;
bool InvertPredCond;
- if (auto Recipe = shouldFoldCondBranchesToCommonDestination(BI, PBI, TTI))
- std::tie(Opc, InvertPredCond) = *Recipe;
+ if (auto Recepie = CheckIfCondBranchesShareCommonDestination(BI, PBI))
+ std::tie(Opc, InvertPredCond) = *Recepie;
else
continue;
@@ -3100,7 +3077,7 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
continue;
}
- return performBranchToCommonDestFolding(BI, PBI, DTU, MSSAU, TTI);
+ return PerformBranchToCommonDestFolding(BI, PBI, DTU, MSSAU);
}
return Changed;
}
diff --git a/llvm/test/Transforms/PGOProfile/chr.ll b/llvm/test/Transforms/PGOProfile/chr.ll
index ddf4811a0363e..8a05a624209ef 100644
--- a/llvm/test/Transforms/PGOProfile/chr.ll
+++ b/llvm/test/Transforms/PGOProfile/chr.ll
@@ -1277,12 +1277,11 @@ define i32 @test_chr_14(i32* %i, i32* %j, i32 %sum0, i1 %pred, i32 %z) !prof !14
; CHECK-LABEL: @test_chr_14(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[I0:%.*]] = load i32, i32* [[I:%.*]], align 4
-; CHECK-NEXT: [[V1:%.*]] = icmp eq i32 [[Z:%.*]], 1
-; CHECK-NEXT: br i1 [[V1]], label [[BB1:%.*]], label [[ENTRY_SPLIT_NONCHR:%.*]], !prof !15
-; CHECK: entry.split.nonchr:
+; CHECK-NEXT: [[V1:%.*]] = icmp ne i32 [[Z:%.*]], 1
; CHECK-NEXT: [[V0:%.*]] = icmp eq i32 [[Z]], 0
; CHECK-NEXT: [[V3_NONCHR:%.*]] = and i1 [[V0]], [[PRED:%.*]]
-; CHECK-NEXT: br i1 [[V3_NONCHR]], label [[BB0_NONCHR:%.*]], label [[BB1]], !prof !16
+; CHECK-NEXT: [[OR_COND:%.*]] = and i1 [[V1]], [[V3_NONCHR]]
+; CHECK-NEXT: br i1 [[OR_COND]], label [[BB0_NONCHR:%.*]], label [[BB1:%.*]], !prof !19
; CHECK: bb0.nonchr:
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: br label [[BB1]]
@@ -1913,7 +1912,7 @@ define i32 @test_chr_21(i64 %i, i64 %k, i64 %j) !prof !14 {
; CHECK-NEXT: switch i64 [[I]], label [[BB2:%.*]] [
; CHECK-NEXT: i64 2, label [[BB3_NONCHR2:%.*]]
; CHECK-NEXT: i64 86, label [[BB2_NONCHR1:%.*]]
-; CHECK-NEXT: ], !prof !19
+; CHECK-NEXT: ], !prof !20
; CHECK: bb2:
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: call void @foo()
@@ -2490,14 +2489,14 @@ define void @test_chr_24(i32* %i) !prof !14 {
; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* [[I:%.*]], align 4
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[TMP0]], 1
; CHECK-NEXT: [[TMP2:%.*]] = icmp eq i32 [[TMP1]], 0
-; CHECK-NEXT: br i1 [[TMP2]], label [[BB1:%.*]], label [[BB0:%.*]], !prof !20
+; CHECK-NEXT: br i1 [[TMP2]], label [[BB1:%.*]], label [[BB0:%.*]], !prof !21
; CHECK: bb0:
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: br label [[BB1]]
; CHECK: bb1:
; CHECK-NEXT: [[TMP3:%.*]] = and i32 [[TMP0]], 2
; CHECK-NEXT: [[TMP4:%.*]] = icmp eq i32 [[TMP3]], 0
-; CHECK-NEXT: br i1 [[TMP4]], label [[BB3:%.*]], label [[BB2:%.*]], !prof !20
+; CHECK-NEXT: br i1 [[TMP4]], label [[BB3:%.*]], label [[BB2:%.*]], !prof !21
; CHECK: bb2:
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: br label [[BB3]]
@@ -2551,3 +2550,4 @@ bb3:
; CHECK: !16 = !{!"branch_weights", i32 0, i32 1}
; CHECK: !17 = !{!"branch_weights", i32 1, i32 1}
; CHECK: !18 = !{!"branch_weights", i32 1, i32 0}
+; CHECK: !19 = !{!"branch_weights", i32 0, i32 1000}
diff --git a/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll b/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
index ddf28d2426446..1e966c2f4c4a9 100644
--- a/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
+++ b/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
@@ -636,17 +636,16 @@ exit:
ret i32 %outval
}
-; Merging the icmps with logic-op defeats the purpose of the metadata.
+; FIXME: Merging the icmps with logic-op defeats the purpose of the metadata.
; We can't tell which condition is expensive if they are combined.
define void @or_icmps_harmful(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @or_icmps_harmful(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[EXPECTED_TRUE:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT: br i1 [[EXPECTED_TRUE]], label [[EXIT:%.*]], label [[RARE:%.*]], !prof !19
-; CHECK: rare:
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
-; CHECK-NEXT: br i1 [[EXPENSIVE]], label [[EXIT]], label [[FALSE:%.*]]
+; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[EXPECTED_TRUE]], [[EXPENSIVE]]
+; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !19
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -669,17 +668,16 @@ exit:
ret void
}
-; Merging the icmps with logic-op defeats the purpose of the metadata.
+; FIXME: Merging the icmps with logic-op defeats the purpose of the metadata.
; We can't tell which condition is expensive if they are combined.
define void @or_icmps_harmful_inverted(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @or_icmps_harmful_inverted(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[EXPECTED_FALSE:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT: br i1 [[EXPECTED_FALSE]], label [[RARE:%.*]], label [[EXIT:%.*]], !prof !20
-; CHECK: rare:
+; CHECK-NEXT: [[EXPECTED_FALSE:%.*]] = icmp sle i32 [[X:%.*]], -1
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
-; CHECK-NEXT: br i1 [[EXPENSIVE]], label [[EXIT]], label [[FALSE:%.*]]
+; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[EXPECTED_FALSE]], [[EXPENSIVE]]
+; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !19
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -702,8 +700,7 @@ exit:
ret void
}
-; The probability threshold is determined by a TTI setting.
-; In this example, we are just short of strongly expected, so speculate.
+; The probability threshold is set by a builtin_expect setting.
define void @or_icmps_not_that_harmful(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @or_icmps_not_that_harmful(
@@ -711,7 +708,7 @@ define void @or_icmps_not_that_harmful(i32 %x, i32 %y, i8* %p) {
; CHECK-NEXT: [[EXPECTED_TRUE:%.*]] = icmp sgt i32 [[X:%.*]], -1
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[EXPECTED_TRUE]], [[EXPENSIVE]]
-; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !21
+; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !20
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -734,16 +731,13 @@ exit:
ret void
}
-; The probability threshold is determined by a TTI setting.
-; In this example, we are just short of strongly expected, so speculate.
-
define void @or_icmps_not_that_harmful_inverted(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @or_icmps_not_that_harmful_inverted(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[EXPECTED_TRUE:%.*]] = icmp sgt i32 [[X:%.*]], -1
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[EXPECTED_TRUE]], [[EXPENSIVE]]
-; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !22
+; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !21
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -766,15 +760,13 @@ exit:
ret void
}
-; The 1st cmp is probably true, so speculating the 2nd is probably a win.
-
define void @or_icmps_useful(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @or_icmps_useful(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[EXPECTED_TRUE:%.*]] = icmp sle i32 [[X:%.*]], -1
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[EXPECTED_TRUE]], [[EXPENSIVE]]
-; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !23
+; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !22
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -797,15 +789,13 @@ exit:
ret void
}
-; The 1st cmp is probably false, so speculating the 2nd is probably a win.
-
define void @or_icmps_useful_inverted(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @or_icmps_useful_inverted(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[EXPECTED_FALSE:%.*]] = icmp sgt i32 [[X:%.*]], -1
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[EXPECTED_FALSE]], [[EXPENSIVE]]
-; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !23
+; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT:%.*]], label [[FALSE:%.*]], !prof !22
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -859,17 +849,16 @@ exit:
ret void
}
-; Merging the icmps with logic-op defeats the purpose of the metadata.
+; FIXME: Merging the icmps with logic-op defeats the purpose of the metadata.
; We can't tell which condition is expensive if they are combined.
define void @and_icmps_harmful(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @and_icmps_harmful(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[EXPECTED_FALSE:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT: br i1 [[EXPECTED_FALSE]], label [[RARE:%.*]], label [[EXIT:%.*]], !prof !20
-; CHECK: rare:
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
-; CHECK-NEXT: br i1 [[EXPENSIVE]], label [[FALSE:%.*]], label [[EXIT]]
+; CHECK-NEXT: [[OR_COND:%.*]] = and i1 [[EXPECTED_FALSE]], [[EXPENSIVE]]
+; CHECK-NEXT: br i1 [[OR_COND]], label [[FALSE:%.*]], label [[EXIT:%.*]], !prof !23
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -892,17 +881,16 @@ exit:
ret void
}
-; Merging the icmps with logic-op defeats the purpose of the metadata.
+; FIXME: Merging the icmps with logic-op defeats the purpose of the metadata.
; We can't tell which condition is expensive if they are combined.
define void @and_icmps_harmful_inverted(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @and_icmps_harmful_inverted(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[EXPECTED_TRUE:%.*]] = icmp sgt i32 [[X:%.*]], -1
-; CHECK-NEXT: br i1 [[EXPECTED_TRUE]], label [[EXIT:%.*]], label [[RARE:%.*]], !prof !19
-; CHECK: rare:
+; CHECK-NEXT: [[EXPECTED_TRUE:%.*]] = icmp sle i32 [[X:%.*]], -1
; CHECK-NEXT: [[EXPENSIVE:%.*]] = icmp eq i32 [[Y:%.*]], 0
-; CHECK-NEXT: br i1 [[EXPENSIVE]], label [[FALSE:%.*]], label [[EXIT]]
+; CHECK-NEXT: [[OR_COND:%.*]] = and i1 [[EXPECTED_TRUE]], [[EXPENSIVE]]
+; CHECK-NEXT: br i1 [[OR_COND]], label [[FALSE:%.*]], label [[EXIT:%.*]], !prof !23
; CHECK: false:
; CHECK-NEXT: store i8 42, i8* [[P:%.*]], align 1
; CHECK-NEXT: br label [[EXIT]]
@@ -925,9 +913,6 @@ exit:
ret void
}
-; The probability threshold is determined by a TTI setting.
-; In this example, we are just short of strongly expected, so speculate.
-
define void @and_icmps_not_that_harmful(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @and_icmps_not_that_harmful(
; CHECK-NEXT: entry:
@@ -957,9 +942,6 @@ exit:
ret void
}
-; The probability threshold is determined by a TTI setting.
-; In this example, we are just short of strongly expected, so speculate.
-
define void @and_icmps_not_that_harmful_inverted(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @and_icmps_not_that_harmful_inverted(
; CHECK-NEXT: entry:
@@ -989,8 +971,6 @@ exit:
ret void
}
-; The 1st cmp is probably true, so speculating the 2nd is probably a win.
-
define void @and_icmps_useful(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @and_icmps_useful(
; CHECK-NEXT: entry:
@@ -1020,8 +1000,6 @@ exit:
ret void
}
-; The 1st cmp is probably false, so speculating the 2nd is probably a win.
-
define void @and_icmps_useful_inverted(i32 %x, i32 %y, i8* %p) {
; CHECK-LABEL: @and_icmps_useful_inverted(
; CHECK-NEXT: entry:
More information about the llvm-commits
mailing list