Skip to content

Commit abe4677

Browse files
authored
[InstCombine] Fix infinite loop due to incorrect DoesConsume (#82973)
When a call to `getFreelyInvertedImpl` with a select/phi node fails, `DoesConsume` should not be changed. Fixes #82877.
1 parent f32c6b2 commit abe4677

File tree

2 files changed

+42
-3
lines changed

2 files changed

+42
-3
lines changed

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2341,11 +2341,13 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
23412341
!shouldAvoidAbsorbingNotIntoSelect(*cast<SelectInst>(V));
23422342
// Selects/min/max with invertible operands are freely invertible
23432343
if (IsSelect || match(V, m_MaxOrMin(m_Value(A), m_Value(B)))) {
2344+
bool LocalDoesConsume = DoesConsume;
23442345
if (!getFreelyInvertedImpl(B, B->hasOneUse(), /*Builder*/ nullptr,
2345-
DoesConsume, Depth))
2346+
LocalDoesConsume, Depth))
23462347
return nullptr;
23472348
if (Value *NotA = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
2348-
DoesConsume, Depth)) {
2349+
LocalDoesConsume, Depth)) {
2350+
DoesConsume = LocalDoesConsume;
23492351
if (Builder != nullptr) {
23502352
Value *NotB = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
23512353
DoesConsume, Depth);
@@ -2361,12 +2363,13 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
23612363
}
23622364

23632365
if (PHINode *PN = dyn_cast<PHINode>(V)) {
2366+
bool LocalDoesConsume = DoesConsume;
23642367
SmallVector<std::pair<Value *, BasicBlock *>, 8> IncomingValues;
23652368
for (Use &U : PN->operands()) {
23662369
BasicBlock *IncomingBlock = PN->getIncomingBlock(U);
23672370
Value *NewIncomingVal = getFreelyInvertedImpl(
23682371
U.get(), /*WillInvertAllUses=*/false,
2369-
/*Builder=*/nullptr, DoesConsume, MaxAnalysisRecursionDepth - 1);
2372+
/*Builder=*/nullptr, LocalDoesConsume, MaxAnalysisRecursionDepth - 1);
23702373
if (NewIncomingVal == nullptr)
23712374
return nullptr;
23722375
// Make sure that we can safely erase the original PHI node.
@@ -2375,6 +2378,8 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
23752378
if (Builder != nullptr)
23762379
IncomingValues.emplace_back(NewIncomingVal, IncomingBlock);
23772380
}
2381+
2382+
DoesConsume = LocalDoesConsume;
23782383
if (Builder != nullptr) {
23792384
IRBuilderBase::InsertPointGuard Guard(*Builder);
23802385
Builder->SetInsertPoint(PN);
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
2+
; RUN: opt -S -passes=instcombine < %s | FileCheck %s
3+
4+
define i64 @func(i32 %p, i1 %cmp1) {
5+
; CHECK-LABEL: define i64 @func(
6+
; CHECK-SAME: i32 [[P:%.*]], i1 [[CMP1:%.*]]) {
7+
; CHECK-NEXT: entry:
8+
; CHECK-NEXT: [[NOT:%.*]] = xor i32 [[P]], -1
9+
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
10+
; CHECK: for.body:
11+
; CHECK-NEXT: [[P0:%.*]] = phi i32 [ [[NOT]], [[ENTRY:%.*]] ], [ [[CONV:%.*]], [[FOR_BODY]] ]
12+
; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1]], i32 0, i32 -1231558963
13+
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[COND]], [[P0]]
14+
; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i32 [[XOR]], 1
15+
; CHECK-NEXT: [[CONV]] = zext i1 [[CMP2]] to i32
16+
; CHECK-NEXT: br i1 [[CMP2]], label [[FOR_BODY]], label [[FOR_EXIT:%.*]]
17+
; CHECK: for.exit:
18+
; CHECK-NEXT: ret i64 0
19+
;
20+
entry:
21+
%not = xor i32 %p, -1
22+
br label %for.body
23+
24+
for.body:
25+
%p0 = phi i32 [ %not, %entry ], [ %conv, %for.body ]
26+
%cond = select i1 %cmp1, i32 0, i32 -1231558963
27+
%xor = xor i32 %cond, %p0
28+
%cmp2 = icmp ne i32 %xor, 1
29+
%conv = zext i1 %cmp2 to i32
30+
br i1 %cmp2, label %for.body, label %for.exit
31+
32+
for.exit:
33+
ret i64 0
34+
}

0 commit comments

Comments
 (0)