Skip to content
This repository was archived by the owner on Mar 28, 2020. It is now read-only.

Commit 576f0d9

Browse files
committed
[SimplifyCFG] propagate branch metadata when creating select (retry r268550 with possible fix)
Retrying r268550 which was reverted at r268577 due a memory sanitizer failure. I have not been able to reproduce that failure, but I've taken a guess at fixing the problem in this version of the patch and will watch for another failure. Original commit message: Unlike earlier similar fixes, we need to recalculate the branch weights in this case. Differential Revision: http://reviews.llvm.org/D19674 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@268751 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent b3b95be commit 576f0d9

File tree

2 files changed

+63
-17
lines changed

2 files changed

+63
-17
lines changed

lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@ static void GetBranchWeights(TerminatorInst *TI,
848848
}
849849
}
850850

851-
/// Keep halving the weights until all can fit in uint32_t.
851+
/// Scale each weight so they all fit in uint32_t.
852852
static void FitWeights(MutableArrayRef<uint64_t> Weights) {
853853
uint64_t Max = *std::max_element(Weights.begin(), Weights.end());
854854
if (Max > UINT_MAX) {
@@ -2820,28 +2820,28 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
28202820
PBI->setSuccessor(1, OtherDest);
28212821

28222822
// Update branch weight for PBI.
2823+
MDBuilder MDB(BI->getContext());
28232824
uint64_t PredTrueWeight, PredFalseWeight, SuccTrueWeight, SuccFalseWeight;
2825+
uint64_t PredCommon, PredOther, SuccCommon, SuccOther;
28242826
bool PredHasWeights =
28252827
PBI->extractProfMetadata(PredTrueWeight, PredFalseWeight);
28262828
bool SuccHasWeights =
28272829
BI->extractProfMetadata(SuccTrueWeight, SuccFalseWeight);
28282830
if (PredHasWeights && SuccHasWeights) {
2829-
uint64_t PredCommon = PBIOp ? PredFalseWeight : PredTrueWeight;
2830-
uint64_t PredOther = PBIOp ?PredTrueWeight : PredFalseWeight;
2831-
uint64_t SuccCommon = BIOp ? SuccFalseWeight : SuccTrueWeight;
2832-
uint64_t SuccOther = BIOp ? SuccTrueWeight : SuccFalseWeight;
2831+
PredCommon = PBIOp ? PredFalseWeight : PredTrueWeight;
2832+
PredOther = PBIOp ? PredTrueWeight : PredFalseWeight;
2833+
SuccCommon = BIOp ? SuccFalseWeight : SuccTrueWeight;
2834+
SuccOther = BIOp ? SuccTrueWeight : SuccFalseWeight;
28332835
// The weight to CommonDest should be PredCommon * SuccTotal +
28342836
// PredOther * SuccCommon.
28352837
// The weight to OtherDest should be PredOther * SuccOther.
28362838
uint64_t NewWeights[2] = {PredCommon * (SuccCommon + SuccOther) +
28372839
PredOther * SuccCommon,
28382840
PredOther * SuccOther};
2839-
// Halve the weights if any of them cannot fit in an uint32_t
28402841
FitWeights(NewWeights);
28412842

28422843
PBI->setMetadata(LLVMContext::MD_prof,
2843-
MDBuilder(BI->getContext())
2844-
.createBranchWeights(NewWeights[0], NewWeights[1]));
2844+
MDB.createBranchWeights(NewWeights[0], NewWeights[1]));
28452845
}
28462846

28472847
// OtherDest may have phi nodes. If so, add an entry from PBI's
@@ -2860,9 +2860,24 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
28602860
Value *PBIV = PN->getIncomingValue(PBBIdx);
28612861
if (BIV != PBIV) {
28622862
// Insert a select in PBI to pick the right value.
2863-
Value *NV = cast<SelectInst>
2863+
SelectInst *NV = cast<SelectInst>
28642864
(Builder.CreateSelect(PBICond, PBIV, BIV, PBIV->getName() + ".mux"));
28652865
PN->setIncomingValue(PBBIdx, NV);
2866+
// Although the select has the same condition as PBI, the original branch
2867+
// weights for PBI do not apply to the new select because the select's
2868+
// 'logical' edges are incoming edges of the phi that is eliminated, not
2869+
// the outgoing edges of PBI.
2870+
if (PredHasWeights && SuccHasWeights) {
2871+
// The weight to PredCommonDest should be PredCommon * SuccTotal.
2872+
// The weight to PredOtherDest should be PredOther * SuccCommon.
2873+
uint64_t NewWeights[2] = {PredCommon * (SuccCommon + SuccOther),
2874+
PredOther * SuccCommon};
2875+
2876+
FitWeights(NewWeights);
2877+
2878+
NV->setMetadata(LLVMContext::MD_prof,
2879+
MDB.createBranchWeights(NewWeights[0], NewWeights[1]));
2880+
}
28662881
}
28672882
}
28682883

test/Transforms/SimplifyCFG/preserve-branchweights.ll

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -412,22 +412,48 @@ return:
412412
ret i32 %retval.0
413413
}
414414

415-
; The 1st select should have branch weights equal to the 1st branch.
416-
; The 2nd select should have freshly calculated branch weights.
415+
; The selects should have freshly calculated branch weights.
417416

418417
define i32 @SimplifyCondBranchToCondBranch(i1 %cmpa, i1 %cmpb) {
419418
; CHECK-LABEL: @SimplifyCondBranchToCondBranch(
420419
; CHECK-NEXT: block1:
421-
; CHECK-NEXT: [[BRMERGE:%.*]] = or i1 %cmpb, %cmpa
422-
; CHECK-NEXT: [[DOTMUX:%.*]] = select i1 %cmpb, i32 0, i32 2
423-
; CHECK-NEXT: [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof !12
420+
; CHECK-NEXT: [[BRMERGE:%.*]] = or i1 %cmpa, %cmpb
421+
; CHECK-NEXT: [[DOTMUX:%.*]] = select i1 %cmpa, i32 0, i32 2, !prof !12
422+
; CHECK-NEXT: [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof !13
424423
; CHECK-NEXT: ret i32 [[OUTVAL]]
425424
;
426425
block1:
427-
br i1 %cmpb, label %block3, label %block2, !prof !0
426+
br i1 %cmpa, label %block3, label %block2, !prof !13
428427

429428
block2:
430-
br i1 %cmpa, label %block3, label %exit, !prof !2
429+
br i1 %cmpb, label %block3, label %exit, !prof !14
430+
431+
block3:
432+
%cowval = phi i32 [ 2, %block2 ], [ 0, %block1 ]
433+
br label %exit
434+
435+
exit:
436+
%outval = phi i32 [ %cowval, %block3 ], [ 1, %block2 ]
437+
ret i32 %outval
438+
}
439+
440+
; Swap the operands of the compares to verify that the weights update correctly.
441+
442+
define i32 @SimplifyCondBranchToCondBranchSwap(i1 %cmpa, i1 %cmpb) {
443+
; CHECK-LABEL: @SimplifyCondBranchToCondBranchSwap(
444+
; CHECK-NEXT: block1:
445+
; CHECK-NEXT: [[CMPA_NOT:%.*]] = xor i1 %cmpa, true
446+
; CHECK-NEXT: [[CMPB_NOT:%.*]] = xor i1 %cmpb, true
447+
; CHECK-NEXT: [[BRMERGE:%.*]] = or i1 [[CMPA_NOT]], [[CMPB_NOT]]
448+
; CHECK-NEXT: [[DOTMUX:%.*]] = select i1 [[CMPA_NOT]], i32 0, i32 2, !prof !14
449+
; CHECK-NEXT: [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof !15
450+
; CHECK-NEXT: ret i32 [[OUTVAL]]
451+
;
452+
block1:
453+
br i1 %cmpa, label %block2, label %block3, !prof !13
454+
455+
block2:
456+
br i1 %cmpb, label %exit, label %block3, !prof !14
431457

432458
block3:
433459
%cowval = phi i32 [ 2, %block2 ], [ 0, %block1 ]
@@ -452,6 +478,8 @@ exit:
452478
!10 = !{!"branch_weights", i32 672646, i32 21604207}
453479
!11 = !{!"branch_weights", i32 6960, i32 21597248}
454480
!12 = !{!"these_are_not_the_branch_weights_you_are_looking_for", i32 3, i32 5}
481+
!13 = !{!"branch_weights", i32 2, i32 3}
482+
!14 = !{!"branch_weights", i32 4, i32 7}
455483

456484
; CHECK: !0 = !{!"branch_weights", i32 5, i32 11}
457485
; CHECK: !1 = !{!"branch_weights", i32 1, i32 5}
@@ -467,5 +495,8 @@ exit:
467495
;; treat the weight as an unsigned integer.
468496
; CHECK: !10 = !{!"branch_weights", i32 112017436, i32 -735157296}
469497
; CHECK: !11 = !{!"branch_weights", i32 3, i32 5}
470-
; CHECK: !12 = !{!"branch_weights", i32 14, i32 10}
498+
; CHECK: !12 = !{!"branch_weights", i32 22, i32 12}
499+
; CHECK: !13 = !{!"branch_weights", i32 34, i32 21}
500+
; CHECK: !14 = !{!"branch_weights", i32 33, i32 14}
501+
; CHECK: !15 = !{!"branch_weights", i32 47, i32 8}
471502

0 commit comments

Comments
 (0)