Skip to content

Commit 0195511

Browse files
snehasishsnehasish
authored
[InstCombine] Fix profile metadata propagation in InstCombine select folding (llvm#179743)
Propagate profile metadata when canonicalizing SPF and drop it when folding select instructions with logical AND/OR conditions. This fixes profile verification failures in Transforms/InstCombine/select-and-or.ll. 1. Select Pattern Factor (SPF) Canonicalization When canonicalizing SPF patterns (like umax/umin), InstCombine transforms sequences like select i1 %cond,(select i1 %cmp, %x, %y), %z into intrinsic calls wrapped in a new select. The new outer select directly replaces the original select instruction, and its condition (%cond) remains structurally identical. Because the condition and its evaluated true/false semantics are unchanged, it is ok to copy the original !prof branch weight metadata to the newly created select. 2. Logical Boolean Folds (foldSelectOfBools) For logical boolean folds (e.g., transforming select (~a | c), a, b into select a, (select c, true, b), false), InstCombine restructures complex conditions into nested selects. In most cases, it is challenging to infer what the profile weights should be for the new select instructions. For a couple of places where should be able to infer profile weights, I've left a TODO to follow up.
1 parent 79d2444 commit 0195511

File tree

3 files changed

+58
-46
lines changed

3 files changed

+58
-46
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp‎

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3595,11 +3595,11 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
35953595
if (match(CondVal, m_OneUse(m_Select(m_Value(A), m_One(), m_Value(B)))) &&
35963596
impliesPoisonOrCond(FalseVal, B, /*Expected=*/false)) {
35973597
// (A || B) || C --> A || (B | C)
3598-
return replaceInstUsesWith(
3599-
SI, Builder.CreateLogicalOr(A, Builder.CreateOr(B, FalseVal), "",
3600-
ProfcheckDisableMetadataFixes
3601-
? nullptr
3602-
: cast<SelectInst>(CondVal)));
3598+
Value *LOr = Builder.CreateLogicalOr(A, Builder.CreateOr(B, FalseVal));
3599+
if (auto *I = dyn_cast<Instruction>(LOr)) {
3600+
setExplicitlyUnknownBranchWeightsIfProfiled(*I, DEBUG_TYPE);
3601+
}
3602+
return replaceInstUsesWith(SI, LOr);
36033603
}
36043604

36053605
// (A && B) || (C && B) --> (A || C) && B
@@ -3611,11 +3611,12 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
36113611
auto AndFactorization = [&](Value *Common, Value *InnerCond,
36123612
Value *InnerVal,
36133613
bool SelFirst = false) -> Instruction * {
3614-
Value *InnerSel = Builder.CreateSelect(InnerCond, One, InnerVal);
3614+
Value *InnerSel = Builder.CreateSelectWithUnknownProfile(
3615+
InnerCond, One, InnerVal, DEBUG_TYPE);
36153616
if (SelFirst)
36163617
std::swap(Common, InnerSel);
36173618
if (FalseLogicAnd || (CondLogicAnd && Common == A))
3618-
return SelectInst::Create(Common, InnerSel, Zero);
3619+
return createSelectInstWithUnknownProfile(Common, InnerSel, Zero);
36193620
else
36203621
return BinaryOperator::CreateAnd(Common, InnerSel);
36213622
};
@@ -3640,11 +3641,11 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
36403641
if (match(CondVal, m_OneUse(m_Select(m_Value(A), m_Value(B), m_Zero()))) &&
36413642
impliesPoisonOrCond(TrueVal, B, /*Expected=*/true)) {
36423643
// (A && B) && C --> A && (B & C)
3643-
return replaceInstUsesWith(
3644-
SI, Builder.CreateLogicalAnd(A, Builder.CreateAnd(B, TrueVal), "",
3645-
ProfcheckDisableMetadataFixes
3646-
? nullptr
3647-
: cast<SelectInst>(CondVal)));
3644+
Value *LAnd = Builder.CreateLogicalAnd(A, Builder.CreateAnd(B, TrueVal));
3645+
if (auto *I = dyn_cast<Instruction>(LAnd)) {
3646+
setExplicitlyUnknownBranchWeightsIfProfiled(*I, DEBUG_TYPE);
3647+
}
3648+
return replaceInstUsesWith(SI, LAnd);
36483649
}
36493650

36503651
// (A || B) && (C || B) --> (A && C) || B
@@ -3656,11 +3657,12 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
36563657
auto OrFactorization = [&](Value *Common, Value *InnerCond,
36573658
Value *InnerVal,
36583659
bool SelFirst = false) -> Instruction * {
3659-
Value *InnerSel = Builder.CreateSelect(InnerCond, InnerVal, Zero);
3660+
Value *InnerSel = Builder.CreateSelectWithUnknownProfile(
3661+
InnerCond, InnerVal, Zero, DEBUG_TYPE);
36603662
if (SelFirst)
36613663
std::swap(Common, InnerSel);
36623664
if (TrueLogicOr || (CondLogicOr && Common == A))
3663-
return SelectInst::Create(Common, One, InnerSel);
3665+
return createSelectInstWithUnknownProfile(Common, One, InnerSel);
36643666
else
36653667
return BinaryOperator::CreateOr(Common, InnerSel);
36663668
};
@@ -3738,28 +3740,36 @@ Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
37383740
// select (~a | c), a, b -> select a, (select c, true, b), false
37393741
if (match(CondVal,
37403742
m_OneUse(m_c_Or(m_Not(m_Specific(TrueVal)), m_Value(C))))) {
3741-
Value *OrV = Builder.CreateSelect(C, One, FalseVal);
3742-
return SelectInst::Create(TrueVal, OrV, Zero);
3743+
// TODO(#183864): We could improve the profile if P(~a | c) < 0.5, which
3744+
// implies strong bounds on both operands (P(a) is high, P(c) is low).
3745+
Value *OrV =
3746+
Builder.CreateSelectWithUnknownProfile(C, One, FalseVal, DEBUG_TYPE);
3747+
return createSelectInstWithUnknownProfile(TrueVal, OrV, Zero);
37433748
}
37443749
// select (c & b), a, b -> select b, (select ~c, true, a), false
37453750
if (match(CondVal, m_OneUse(m_c_And(m_Value(C), m_Specific(FalseVal))))) {
37463751
if (Value *NotC = getFreelyInverted(C, C->hasOneUse(), &Builder)) {
3747-
Value *OrV = Builder.CreateSelect(NotC, One, TrueVal);
3748-
return SelectInst::Create(FalseVal, OrV, Zero);
3752+
Value *OrV = Builder.CreateSelectWithUnknownProfile(NotC, One, TrueVal,
3753+
DEBUG_TYPE);
3754+
return createSelectInstWithUnknownProfile(FalseVal, OrV, Zero);
37493755
}
37503756
}
37513757
// select (a | c), a, b -> select a, true, (select ~c, b, false)
37523758
if (match(CondVal, m_OneUse(m_c_Or(m_Specific(TrueVal), m_Value(C))))) {
37533759
if (Value *NotC = getFreelyInverted(C, C->hasOneUse(), &Builder)) {
3754-
Value *AndV = Builder.CreateSelect(NotC, FalseVal, Zero);
3755-
return SelectInst::Create(TrueVal, One, AndV);
3760+
// TODO(#183864): We could improve the profile if P(a | c) < 0.5, which
3761+
// implies strong bounds on both operands (both P(a) and P(c) are low).
3762+
Value *AndV = Builder.CreateSelectWithUnknownProfile(NotC, FalseVal, Zero,
3763+
DEBUG_TYPE);
3764+
return createSelectInstWithUnknownProfile(TrueVal, One, AndV);
37563765
}
37573766
}
37583767
// select (c & ~b), a, b -> select b, true, (select c, a, false)
37593768
if (match(CondVal,
37603769
m_OneUse(m_c_And(m_Value(C), m_Not(m_Specific(FalseVal)))))) {
3761-
Value *AndV = Builder.CreateSelect(C, TrueVal, Zero);
3762-
return SelectInst::Create(FalseVal, One, AndV);
3770+
Value *AndV =
3771+
Builder.CreateSelectWithUnknownProfile(C, TrueVal, Zero, DEBUG_TYPE);
3772+
return createSelectInstWithUnknownProfile(FalseVal, One, AndV);
37633773
}
37643774

37653775
if (match(FalseVal, m_Zero()) || match(TrueVal, m_One())) {
@@ -4865,9 +4875,11 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
48654875
// Is (select B, T, F) a SPF?
48664876
if (CondVal->hasOneUse() && SelType->isIntOrIntVectorTy()) {
48674877
if (ICmpInst *Cmp = dyn_cast<ICmpInst>(B))
4868-
if (Value *V = canonicalizeSPF(*Cmp, TrueVal, FalseVal, *this))
4869-
return SelectInst::Create(A, IsAnd ? V : TrueVal,
4870-
IsAnd ? FalseVal : V);
4878+
if (Value *V = canonicalizeSPF(*Cmp, TrueVal, FalseVal, *this)) {
4879+
return SelectInst::Create(
4880+
A, IsAnd ? V : TrueVal, IsAnd ? FalseVal : V, "", nullptr,
4881+
ProfcheckDisableMetadataFixes ? nullptr : &SI);
4882+
}
48714883
}
48724884

48734885
return nullptr;

‎llvm/test/Transforms/InstCombine/select-and-or.ll‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -490,27 +490,27 @@ define i1 @demorgan_select_infloop2(i1 %L) {
490490
ret i1 %C15
491491
}
492492

493-
define i1 @and_or1(i1 %a, i1 %b, i1 %c) {
493+
define i1 @and_or1(i1 %a, i1 %b, i1 %c) !prof !0 {
494494
; CHECK-LABEL: @and_or1(
495-
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 true, i1 [[B:%.*]]
496-
; CHECK-NEXT: [[R:%.*]] = select i1 [[A:%.*]], i1 [[TMP1]], i1 false
495+
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 true, i1 [[B:%.*]], !prof [[PROF2:![0-9]+]]
496+
; CHECK-NEXT: [[R:%.*]] = select i1 [[A:%.*]], i1 [[TMP1]], i1 false, !prof [[PROF2]]
497497
; CHECK-NEXT: ret i1 [[R]]
498498
;
499499
%nota = xor i1 %a, true
500500
%cond = or i1 %nota, %c
501-
%r = select i1 %cond, i1 %a, i1 %b
501+
%r = select i1 %cond, i1 %a, i1 %b, !prof !1
502502
ret i1 %r
503503
}
504504

505-
define i1 @and_or2(i1 %a, i1 %b, i1 %c) {
505+
define i1 @and_or2(i1 %a, i1 %b, i1 %c) !prof !0 {
506506
; CHECK-LABEL: @and_or2(
507-
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 true, i1 [[A:%.*]]
508-
; CHECK-NEXT: [[R:%.*]] = select i1 [[B:%.*]], i1 [[TMP1]], i1 false
507+
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 true, i1 [[A:%.*]], !prof [[PROF2]]
508+
; CHECK-NEXT: [[R:%.*]] = select i1 [[B:%.*]], i1 [[TMP1]], i1 false, !prof [[PROF2]]
509509
; CHECK-NEXT: ret i1 [[R]]
510510
;
511511
%notc = xor i1 %c, true
512512
%cond = and i1 %notc, %b
513-
%r = select i1 %cond, i1 %a, i1 %b
513+
%r = select i1 %cond, i1 %a, i1 %b, !prof !1
514514
ret i1 %r
515515
}
516516

@@ -741,27 +741,27 @@ define i1 @and_or3_wrong_operand(i1 %a, i1 %b, i32 %x, i32 %y, i1 %d) {
741741
ret i1 %r
742742
}
743743

744-
define i1 @or_and1(i1 %a, i1 %b, i1 %c) {
744+
define i1 @or_and1(i1 %a, i1 %b, i1 %c) !prof !0 {
745745
; CHECK-LABEL: @or_and1(
746-
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 [[A:%.*]], i1 false
747-
; CHECK-NEXT: [[R:%.*]] = select i1 [[B:%.*]], i1 true, i1 [[TMP1]]
746+
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 [[A:%.*]], i1 false, !prof [[PROF2]]
747+
; CHECK-NEXT: [[R:%.*]] = select i1 [[B:%.*]], i1 true, i1 [[TMP1]], !prof [[PROF2]]
748748
; CHECK-NEXT: ret i1 [[R]]
749749
;
750750
%notb = xor i1 %b, true
751751
%cond = and i1 %notb, %c
752-
%r = select i1 %cond, i1 %a, i1 %b
752+
%r = select i1 %cond, i1 %a, i1 %b, !prof !1
753753
ret i1 %r
754754
}
755755

756-
define i1 @or_and2(i1 %a, i1 %b, i1 %c) {
756+
define i1 @or_and2(i1 %a, i1 %b, i1 %c) !prof !0 {
757757
; CHECK-LABEL: @or_and2(
758-
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 [[B:%.*]], i1 false
759-
; CHECK-NEXT: [[R:%.*]] = select i1 [[A:%.*]], i1 true, i1 [[TMP1]]
758+
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[C:%.*]], i1 [[B:%.*]], i1 false, !prof [[PROF2]]
759+
; CHECK-NEXT: [[R:%.*]] = select i1 [[A:%.*]], i1 true, i1 [[TMP1]], !prof [[PROF2]]
760760
; CHECK-NEXT: ret i1 [[R]]
761761
;
762762
%notc = xor i1 %c, true
763763
%cond = or i1 %notc, %a
764-
%r = select i1 %cond, i1 %a, i1 %b
764+
%r = select i1 %cond, i1 %a, i1 %b, !prof !1
765765
ret i1 %r
766766
}
767767

@@ -803,7 +803,7 @@ define i1 @fold_or_of_ands_with_select_to_logical1(i1 %a, i1 %b, i1 %c) !prof !0
803803

804804
define i1 @fold_or_of_ands_with_select_to_logical2(i1 %a, i1 %b, i1 %c) !prof !0 {
805805
; CHECK-LABEL: @fold_or_of_ands_with_select_to_logical2(
806-
; CHECK-NEXT: [[OR1:%.*]] = select i1 [[C:%.*]], i1 [[A:%.*]], i1 [[B:%.*]], !prof [[PROF2:![0-9]+]]
806+
; CHECK-NEXT: [[OR1:%.*]] = select i1 [[C:%.*]], i1 [[A:%.*]], i1 [[B:%.*]], !prof [[PROF2]]
807807
; CHECK-NEXT: ret i1 [[OR1]]
808808
;
809809
%not = xor i1 %c, true
@@ -1117,15 +1117,15 @@ define i1 @or_and3_wrong_operand(i1 %a, i1 %b, i32 %x, i32 %y, i1 %d) {
11171117
ret i1 %r
11181118
}
11191119

1120-
define i8 @test_or_umax(i8 %x, i8 %y, i1 %cond) {
1120+
define i8 @test_or_umax(i8 %x, i8 %y, i1 %cond) !prof !0 {
11211121
; CHECK-LABEL: @test_or_umax(
11221122
; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.umax.i8(i8 [[X:%.*]], i8 [[Y:%.*]])
1123-
; CHECK-NEXT: [[RET:%.*]] = select i1 [[COND:%.*]], i8 [[X]], i8 [[TMP1]]
1123+
; CHECK-NEXT: [[RET:%.*]] = select i1 [[COND:%.*]], i8 [[X]], i8 [[TMP1]], !prof [[PROF1]]
11241124
; CHECK-NEXT: ret i8 [[RET]]
11251125
;
11261126
%cmp = icmp ugt i8 %x, %y
11271127
%or = select i1 %cond, i1 true, i1 %cmp
1128-
%ret = select i1 %or, i8 %x, i8 %y
1128+
%ret = select i1 %or, i8 %x, i8 %y, !prof !2
11291129
ret i8 %ret
11301130
}
11311131

@@ -1467,6 +1467,7 @@ define i8 @test_logical_commuted_and_ne_a_b(i1 %other_cond, i8 %a, i8 %b) {
14671467

14681468
!0 = !{!"function_entry_count", i64 1000}
14691469
!1 = !{!"branch_weights", i32 2, i32 3}
1470+
!2 = !{!"branch_weights", i32 3, i32 2}
14701471
;.
14711472
; CHECK: attributes #[[ATTR0:[0-9]+]] = { nocallback nocreateundeforpoison nofree nosync nounwind speculatable willreturn memory(none) }
14721473
; CHECK: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

‎llvm/utils/profcheck-xfail.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ Transforms/InstCombine/pow-1.ll
115115
Transforms/InstCombine/pow-3.ll
116116
Transforms/InstCombine/pow-sqrt.ll
117117
Transforms/InstCombine/pull-conditional-binop-through-shift.ll
118-
Transforms/InstCombine/select-and-or.ll
119118
Transforms/InstCombine/select-factorize.ll
120119
Transforms/InstCombine/select-min-max.ll
121120
Transforms/InstCombine/select-of-symmetric-selects.ll

0 commit comments

Comments
 (0)