[llvm] [InstCombine] Invalidate changes to `DoesConsume` if the first try fails (PR #82973)
Yingwei Zheng via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 27 03:41:24 PST 2024
https://github.com/dtcxzyw updated https://github.com/llvm/llvm-project/pull/82973
>From 068f827d17d806db5cd8516173aa8c29cc908942 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Mon, 26 Feb 2024 17:23:43 +0800
Subject: [PATCH 1/2] [InstCombine] Invalidate changes to `DoesConsume` if the
first try fails
---
.../InstCombine/InstructionCombining.cpp | 4 ++
llvm/test/Transforms/InstCombine/pr82877.ll | 40 +++++++++++++++++++
2 files changed, 44 insertions(+)
create mode 100644 llvm/test/Transforms/InstCombine/pr82877.ll
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 87c8dca7efed89..96afd0380ac22b 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2295,9 +2295,11 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
// If `V` is of the form `A + B` then `-1 - V` can be folded into
// `(-1 - B) - A` if we are willing to invert all of the uses.
if (match(V, m_Add(m_Value(A), m_Value(B)))) {
+ bool DoesConsumeOldValue = DoesConsume;
if (auto *BV = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateSub(BV, A) : NonNull;
+ DoesConsume = DoesConsumeOldValue;
if (auto *AV = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateSub(AV, B) : NonNull;
@@ -2307,9 +2309,11 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
// If `V` is of the form `A ^ ~B` then `~(A ^ ~B)` can be folded
// into `A ^ B` if we are willing to invert all of the uses.
if (match(V, m_Xor(m_Value(A), m_Value(B)))) {
+ bool DoesConsumeOldValue = DoesConsume;
if (auto *BV = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateXor(A, BV) : NonNull;
+ DoesConsume = DoesConsumeOldValue;
if (auto *AV = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateXor(AV, B) : NonNull;
diff --git a/llvm/test/Transforms/InstCombine/pr82877.ll b/llvm/test/Transforms/InstCombine/pr82877.ll
new file mode 100644
index 00000000000000..64ef490cb588d8
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr82877.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+define i64 @func(i32 %p) {
+; CHECK-LABEL: define i64 @func(
+; CHECK-SAME: i32 [[P:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[NOT:%.*]] = xor i32 [[P]], -1
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: [[P0:%.*]] = phi i32 [ [[NOT]], [[ENTRY:%.*]] ], [ [[CONV:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[P1:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[INC]] = add i32 [[P1]], 1
+; CHECK-NEXT: [[CMP1_NOT:%.*]] = icmp eq i32 [[INC]], 0
+; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1_NOT]], i32 -1231558963, i32 0
+; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[COND]], [[P0]]
+; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i32 [[XOR]], -2
+; CHECK-NEXT: [[CONV]] = zext i1 [[CMP2]] to i32
+; CHECK-NEXT: br i1 [[CMP2]], label [[FOR_BODY]], label [[FOR_EXIT:%.*]]
+; CHECK: for.exit:
+; CHECK-NEXT: ret i64 0
+;
+entry:
+ %not = xor i32 %p, -1
+ br label %for.body
+
+for.body:
+ %p0 = phi i32 [ %not, %entry ], [ %conv, %for.body ]
+ %p1 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+ %inc = add i32 %p1, 1
+ %cmp1 = icmp ne i32 %inc, 0
+ %cond = select i1 %cmp1, i32 0, i32 -1231558963
+ %xor = xor i32 %cond, %p0
+ %cmp2 = icmp ne i32 %xor, -2
+ %conv = zext i1 %cmp2 to i32
+ br i1 %cmp2, label %for.body, label %for.exit
+
+for.exit:
+ ret i64 0
+}
>From 4d1bab4cb5a97f67738831fb3f3716d5861d7b09 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 27 Feb 2024 19:33:10 +0800
Subject: [PATCH 2/2] fixup! [InstCombine] Invalidate changes to `DoesConsume`
if the first try fails
---
.../InstCombine/InstructionCombining.cpp | 19 ++++++++++++-------
llvm/test/Transforms/InstCombine/pr82877.ll | 16 +++++-----------
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 96afd0380ac22b..c3011d89930859 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2295,11 +2295,9 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
// If `V` is of the form `A + B` then `-1 - V` can be folded into
// `(-1 - B) - A` if we are willing to invert all of the uses.
if (match(V, m_Add(m_Value(A), m_Value(B)))) {
- bool DoesConsumeOldValue = DoesConsume;
if (auto *BV = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateSub(BV, A) : NonNull;
- DoesConsume = DoesConsumeOldValue;
if (auto *AV = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateSub(AV, B) : NonNull;
@@ -2309,11 +2307,9 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
// If `V` is of the form `A ^ ~B` then `~(A ^ ~B)` can be folded
// into `A ^ B` if we are willing to invert all of the uses.
if (match(V, m_Xor(m_Value(A), m_Value(B)))) {
- bool DoesConsumeOldValue = DoesConsume;
if (auto *BV = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateXor(A, BV) : NonNull;
- DoesConsume = DoesConsumeOldValue;
if (auto *AV = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
DoesConsume, Depth))
return Builder ? Builder->CreateXor(AV, B) : NonNull;
@@ -2345,9 +2341,12 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
!shouldAvoidAbsorbingNotIntoSelect(*cast<SelectInst>(V));
// Selects/min/max with invertible operands are freely invertible
if (IsSelect || match(V, m_MaxOrMin(m_Value(A), m_Value(B)))) {
+ bool SaveDoesConsume = DoesConsume;
if (!getFreelyInvertedImpl(B, B->hasOneUse(), /*Builder*/ nullptr,
- DoesConsume, Depth))
+ DoesConsume, Depth)) {
+ DoesConsume = SaveDoesConsume;
return nullptr;
+ }
if (Value *NotA = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
DoesConsume, Depth)) {
if (Builder != nullptr) {
@@ -2362,20 +2361,26 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
}
return NonNull;
}
+ DoesConsume = SaveDoesConsume;
}
if (PHINode *PN = dyn_cast<PHINode>(V)) {
+ bool SaveDoesConsume = DoesConsume;
SmallVector<std::pair<Value *, BasicBlock *>, 8> IncomingValues;
for (Use &U : PN->operands()) {
BasicBlock *IncomingBlock = PN->getIncomingBlock(U);
Value *NewIncomingVal = getFreelyInvertedImpl(
U.get(), /*WillInvertAllUses=*/false,
/*Builder=*/nullptr, DoesConsume, MaxAnalysisRecursionDepth - 1);
- if (NewIncomingVal == nullptr)
+ if (NewIncomingVal == nullptr) {
+ DoesConsume = SaveDoesConsume;
return nullptr;
+ }
// Make sure that we can safely erase the original PHI node.
- if (NewIncomingVal == V)
+ if (NewIncomingVal == V) {
+ DoesConsume = SaveDoesConsume;
return nullptr;
+ }
if (Builder != nullptr)
IncomingValues.emplace_back(NewIncomingVal, IncomingBlock);
}
diff --git a/llvm/test/Transforms/InstCombine/pr82877.ll b/llvm/test/Transforms/InstCombine/pr82877.ll
index 64ef490cb588d8..8594bb68b8e4cc 100644
--- a/llvm/test/Transforms/InstCombine/pr82877.ll
+++ b/llvm/test/Transforms/InstCombine/pr82877.ll
@@ -1,20 +1,17 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
; RUN: opt -S -passes=instcombine < %s | FileCheck %s
-define i64 @func(i32 %p) {
+define i64 @func(i32 %p, i1 %cmp1) {
; CHECK-LABEL: define i64 @func(
-; CHECK-SAME: i32 [[P:%.*]]) {
+; CHECK-SAME: i32 [[P:%.*]], i1 [[CMP1:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[NOT:%.*]] = xor i32 [[P]], -1
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[P0:%.*]] = phi i32 [ [[NOT]], [[ENTRY:%.*]] ], [ [[CONV:%.*]], [[FOR_BODY]] ]
-; CHECK-NEXT: [[P1:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
-; CHECK-NEXT: [[INC]] = add i32 [[P1]], 1
-; CHECK-NEXT: [[CMP1_NOT:%.*]] = icmp eq i32 [[INC]], 0
-; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1_NOT]], i32 -1231558963, i32 0
+; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1]], i32 0, i32 -1231558963
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[COND]], [[P0]]
-; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i32 [[XOR]], -2
+; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i32 [[XOR]], 1
; CHECK-NEXT: [[CONV]] = zext i1 [[CMP2]] to i32
; CHECK-NEXT: br i1 [[CMP2]], label [[FOR_BODY]], label [[FOR_EXIT:%.*]]
; CHECK: for.exit:
@@ -26,12 +23,9 @@ entry:
for.body:
%p0 = phi i32 [ %not, %entry ], [ %conv, %for.body ]
- %p1 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
- %inc = add i32 %p1, 1
- %cmp1 = icmp ne i32 %inc, 0
%cond = select i1 %cmp1, i32 0, i32 -1231558963
%xor = xor i32 %cond, %p0
- %cmp2 = icmp ne i32 %xor, -2
+ %cmp2 = icmp ne i32 %xor, 1
%conv = zext i1 %cmp2 to i32
br i1 %cmp2, label %for.body, label %for.exit
More information about the llvm-commits
mailing list