[llvm] [clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
leo-ard wrote: I'm closing this PR in favour of #70845 https://github.com/llvm/llvm-project/pull/67594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
leo-ard wrote: > If I understood correctly, what you're trying to do here is to apply an icmp > fold early before the sext -> zext information gets lost. I don't think this > is the correct way to approach the problem. The correct way is to preserve > the fact that the operand is non-negative when converting to zext, which > would allow the InstCombine fold to reliably undo this. In fact, there is a > pending patch for this at https://reviews.llvm.org/D156444. When tackling this problem, my first idea was to track the non-negativeness of the zext and use this information in InstCombine. I didn't know that you could do it through flags in the IR, which I think is a cleaner solution. Do you think this patch will be available soon ? https://github.com/llvm/llvm-project/pull/67594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
https://github.com/nikic requested changes to this pull request. If I understood correctly, what you're trying to do here is to apply an icmp fold early before the sext -> zext information gets lost. I don't think this is the correct way to approach the problem. The correct way is to preserve the fact that the operand is non-negative when converting to zext, which would allow the InstCombine fold to reliably undo this. In fact, there is a pending patch for this at https://reviews.llvm.org/D156444. https://github.com/llvm/llvm-project/pull/67594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
@@ -0,0 +1,185 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --tool ./bin/opt --version 3 +; See PRXXX for more details mariannems wrote: `See PRXXX` Was that autogenerated ? Does it need to be updated ? https://github.com/llvm/llvm-project/pull/67594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
@@ -193,6 +193,66 @@ static bool replaceSignedInst(SCCPSolver ,
NewInst = BinaryOperator::Create(NewOpcode, Op0, Op1, "", );
break;
}
+ case Instruction::ICmp: {
+ICmpInst = cast(Inst);
+
+ZExtInst *Op0_zext = dyn_cast(ICmp.getOperand(0));
+SExtInst *Op0_sext = dyn_cast(ICmp.getOperand(0));
+
+ZExtInst *Op1_zext = dyn_cast(ICmp.getOperand(1));
+SExtInst *Op1_sext = dyn_cast(ICmp.getOperand(1));
+
+CastInst *Op0;
+CastInst *Op1;
+
+if (Op0_zext)
+ Op0 = Op0_zext;
+else
+ Op0 = Op0_sext;
+if (Op1_zext)
+ Op1 = Op1_zext;
+else
+ Op1 = Op1_sext;
+
+bool reversed = false;
+
+if (!Op0 || !Op1) {
+ // Op0 and Op1 must be defined
+ return false;
+}
+
+if (Op1_zext && (!Op0_zext)) {
+ // We force Op0 to be a zext and reverse the arguments
+ // at the end if we swap
+ reversed = true;
mariannems wrote:
IMHO the swapping makes it harder to read. Not swapping would force case to be
checked in the if statement, but would make the code more readable.
https://github.com/llvm/llvm-project/pull/67594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
@@ -193,6 +193,60 @@ static bool replaceSignedInst(SCCPSolver ,
NewInst = BinaryOperator::Create(NewOpcode, Op0, Op1, "", );
break;
}
+ case Instruction::ICmp: {
+ICmpInst = cast(Inst);
+
+ZExtInst *Op0_zext = dyn_cast(ICmp.getOperand(0));
+SExtInst *Op0_sext = dyn_cast(ICmp.getOperand(0));
+
+ZExtInst *Op1_zext = dyn_cast(ICmp.getOperand(1));
+SExtInst *Op1_sext = dyn_cast(ICmp.getOperand(1));
+
+CastInst *Op0;
+CastInst *Op1;
+
+if (Op0_zext) Op0 = Op0_zext; else Op0 = Op0_sext;
+if (Op1_zext) Op1 = Op1_zext; else Op1 = Op1_sext;
+
+bool reversed = false;
+
+if (!Op0 || !Op1){
+ // Op0 and Op1 must be defined
+ return false;
+}
+
+if (Op1_zext && (! Op0_zext)){
+ // We force Op0 to be a zext and reverse the arguments
+ // at the end if we swap
+ reversed = true;
+
+ std::swap(Op0_zext, Op1_zext);
+ std::swap(Op0_sext, Op1_sext);
+ std::swap(Op0, Op1);
+}
+
+
+if(Op0->getType() != Op1->getType()){
mariannems wrote:
Its is always better to exit early. This check should ideally be done before
potential swapping.
https://github.com/llvm/llvm-project/pull/67594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
https://github.com/mariannems commented: Thanks for fixing that regression! https://github.com/llvm/llvm-project/pull/67594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
https://github.com/mariannems edited https://github.com/llvm/llvm-project/pull/67594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
https://github.com/leo-ard updated
https://github.com/llvm/llvm-project/pull/67594
From 399f9d64cfde0761ac8278dd05ba704d879b1f5a Mon Sep 17 00:00:00 2001
From: leo-ard
Date: Wed, 27 Sep 2023 13:35:53 -0400
Subject: [PATCH 1/2] Adding ICMP folding for SCCPSolver
---
clang/test/CodeGen/X86/min_max.c | 19 ++
llvm/lib/Transforms/Utils/SCCPSolver.cpp | 54 +
.../Transforms/SCCP/icmp-fold-with-cast.ll| 185 ++
llvm/test/Transforms/SCCP/widening.ll | 4 +-
4 files changed, 260 insertions(+), 2 deletions(-)
create mode 100644 clang/test/CodeGen/X86/min_max.c
create mode 100644 llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
diff --git a/clang/test/CodeGen/X86/min_max.c b/clang/test/CodeGen/X86/min_max.c
new file mode 100644
index 000..7af8181cc9ff367
--- /dev/null
+++ b/clang/test/CodeGen/X86/min_max.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -O2 -triple=x86_64-apple-darwin -emit-llvm -o - |
FileCheck %s
+
+short vecreduce_smax_v2i16(int n, short* v)
+{
+ // CHECK: @llvm.smax
+ short p = 0;
+ for (int i = 0; i < n; ++i)
+p = p < v[i] ? v[i] : p;
+ return p;
+}
+
+short vecreduce_smin_v2i16(int n, short* v)
+{
+ // CHECK: @llvm.smin
+ short p = 0;
+ for (int i = 0; i < n; ++i)
+p = p > v[i] ? v[i] : p;
+ return p;
+}
\ No newline at end of file
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 8a67fda7c98e787..09f64a925ab1b7c 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -193,6 +193,60 @@ static bool replaceSignedInst(SCCPSolver ,
NewInst = BinaryOperator::Create(NewOpcode, Op0, Op1, "", );
break;
}
+ case Instruction::ICmp: {
+ICmpInst = cast(Inst);
+
+ZExtInst *Op0_zext = dyn_cast(ICmp.getOperand(0));
+SExtInst *Op0_sext = dyn_cast(ICmp.getOperand(0));
+
+ZExtInst *Op1_zext = dyn_cast(ICmp.getOperand(1));
+SExtInst *Op1_sext = dyn_cast(ICmp.getOperand(1));
+
+CastInst *Op0;
+CastInst *Op1;
+
+if (Op0_zext) Op0 = Op0_zext; else Op0 = Op0_sext;
+if (Op1_zext) Op1 = Op1_zext; else Op1 = Op1_sext;
+
+bool reversed = false;
+
+if (!Op0 || !Op1){
+ // Op0 and Op1 must be defined
+ return false;
+}
+
+if (Op1_zext && (! Op0_zext)){
+ // We force Op0 to be a zext and reverse the arguments
+ // at the end if we swap
+ reversed = true;
+
+ std::swap(Op0_zext, Op1_zext);
+ std::swap(Op0_sext, Op1_sext);
+ std::swap(Op0, Op1);
+}
+
+
+if(Op0->getType() != Op1->getType()){
+ // extensions are not of the same type
+ // This optimization is done in InstCombine
+ return false;
+}
+
+// ICMP (sext X) (sext y) => ICMP X, Y
+// ICMP (zext X) (zext y) => ICMP X, Y
+// ICMP (zext X) (sext Y) => ICMP X, Y if X >= 0 and ICMP signed
+if((Op0_zext && Op1_zext)
+ || (Op0_sext && Op1_sext)
+ || (ICmp.isSigned() && Op0_zext && Op1_sext &&
isNonNegative(Op0_zext->getOperand(0{
+
+ Value *X = Op0->getOperand(0);
+ Value *Y = Op1->getOperand(0);
+ NewInst = CmpInst::Create(ICmp.getOpcode(), ICmp.getPredicate(),
reversed ? Y : X, reversed ? X : Y, "", );
+ break;
+}
+
+return false;
+ }
default:
return false;
}
diff --git a/llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
b/llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
new file mode 100644
index 000..90b2c123081fb49
--- /dev/null
+++ b/llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
@@ -0,0 +1,185 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
UTC_ARGS: --tool ./bin/opt --version 3
+; See PRXXX for more details
+; RUN: opt < %s -S -passes=ipsccp | FileCheck %s
+
+
+define signext i32 @sext_sext(i16 %x, i16 %y) {
+; CHECK-LABEL: define signext i32 @sext_sext(
+; CHECK-SAME: i16 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT:[[CONV:%.*]] = sext i16 [[X]] to i32
+; CHECK-NEXT:[[CONV1:%.*]] = sext i16 [[Y]] to i32
+; CHECK-NEXT:[[CMP2:%.*]] = icmp sgt i16 [[X]], [[Y]]
+; CHECK-NEXT:br i1 [[CMP2]], label [[COND_TRUE:%.*]], label
[[COND_FALSE:%.*]]
+; CHECK: cond.true:
+; CHECK-NEXT:br label [[COND_END:%.*]]
+; CHECK: cond.false:
+; CHECK-NEXT:br label [[COND_END]]
+; CHECK: cond.end:
+; CHECK-NEXT:[[COND:%.*]] = phi i32 [ 0, [[COND_TRUE]] ], [ 1,
[[COND_FALSE]] ]
+; CHECK-NEXT:ret i32 [[COND]]
+;
+entry:
+ %conv = sext i16 %x to i32
+ %conv1 = sext i16 %y to i32
+ %cmp2 = icmp sgt i32 %conv, %conv1
+ br i1 %cmp2, label %cond.true, label %cond.false
+
+cond.true:; preds = %for.body
+ br label %cond.end
+
+cond.false: ; preds = %for.body
+ br label %cond.end
+
+cond.end: ; preds = %cond.false,
%cond.true
+ %cond = phi i32 [
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 5ffbdd9ed5fb719b354e4a46acc8737c5b624f94
399f9d64cfde0761ac8278dd05ba704d879b1f5a -- clang/test/CodeGen/X86/min_max.c
llvm/lib/Transforms/Utils/SCCPSolver.cpp
``
View the diff from clang-format here.
``diff
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 09f64a925ab1..0038e8866c2c 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -195,56 +195,62 @@ static bool replaceSignedInst(SCCPSolver ,
}
case Instruction::ICmp: {
ICmpInst = cast(Inst);
-
+
ZExtInst *Op0_zext = dyn_cast(ICmp.getOperand(0));
SExtInst *Op0_sext = dyn_cast(ICmp.getOperand(0));
-
+
ZExtInst *Op1_zext = dyn_cast(ICmp.getOperand(1));
SExtInst *Op1_sext = dyn_cast(ICmp.getOperand(1));
-
+
CastInst *Op0;
CastInst *Op1;
-
-if (Op0_zext) Op0 = Op0_zext; else Op0 = Op0_sext;
-if (Op1_zext) Op1 = Op1_zext; else Op1 = Op1_sext;
-
+
+if (Op0_zext)
+ Op0 = Op0_zext;
+else
+ Op0 = Op0_sext;
+if (Op1_zext)
+ Op1 = Op1_zext;
+else
+ Op1 = Op1_sext;
+
bool reversed = false;
-
-if (!Op0 || !Op1){
+
+if (!Op0 || !Op1) {
// Op0 and Op1 must be defined
return false;
-}
-
-if (Op1_zext && (! Op0_zext)){
+}
+
+if (Op1_zext && (!Op0_zext)) {
// We force Op0 to be a zext and reverse the arguments
// at the end if we swap
- reversed = true;
-
+ reversed = true;
+
std::swap(Op0_zext, Op1_zext);
std::swap(Op0_sext, Op1_sext);
std::swap(Op0, Op1);
}
-
-
-if(Op0->getType() != Op1->getType()){
+
+if (Op0->getType() != Op1->getType()) {
// extensions are not of the same type
// This optimization is done in InstCombine
return false;
}
-
+
// ICMP (sext X) (sext y) => ICMP X, Y
// ICMP (zext X) (zext y) => ICMP X, Y
// ICMP (zext X) (sext Y) => ICMP X, Y if X >= 0 and ICMP signed
-if((Op0_zext && Op1_zext)
- || (Op0_sext && Op1_sext)
- || (ICmp.isSigned() && Op0_zext && Op1_sext &&
isNonNegative(Op0_zext->getOperand(0{
-
+if ((Op0_zext && Op1_zext) || (Op0_sext && Op1_sext) ||
+(ICmp.isSigned() && Op0_zext && Op1_sext &&
+ isNonNegative(Op0_zext->getOperand(0 {
+
Value *X = Op0->getOperand(0);
Value *Y = Op1->getOperand(0);
- NewInst = CmpInst::Create(ICmp.getOpcode(), ICmp.getPredicate(),
reversed ? Y : X, reversed ? X : Y, "", );
+ NewInst = CmpInst::Create(ICmp.getOpcode(), ICmp.getPredicate(),
+reversed ? Y : X, reversed ? X : Y, "", );
break;
}
-
+
return false;
}
default:
``
https://github.com/llvm/llvm-project/pull/67594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [SCCP] [Transform] Adding ICMP folding for zext and sext in SCCPSolver (PR #67594)
https://github.com/leo-ard created
https://github.com/llvm/llvm-project/pull/67594
This PR fixes #55013 : the max intrinsics is not generated for this simple loop
case : https://godbolt.org/z/hxz1xhMPh. This is caused by a ICMP not being
folded into a select, thus not generating the max intrinsics.
Since LLVM 14, SCCP pass got smarter by folding sext into zext for positive
ranges : https://reviews.llvm.org/D81756. After this change, InstCombine was
sometimes unable to fold icmp correctly as both of the arguments pointed to
mismatched zext/sext. To fix this, @rotateright implemented this fix :
https://reviews.llvm.org/D124419 that tries to resolve the mismatch by knowing
if the argument of a zext is positive (in which case, it is like a sext) by
using ValueTracking. However, ValueTracking seems to be not smart enough for
this case and cannot accurately know that the value is positive or not. This PR
implements the folding in SCCP directly, where we have the knowledge that the
value are positive or not.
This PR also contains test cases for sext/zext folding with SCCP as well as a
x86 regression tests for the max/min case.
From 399f9d64cfde0761ac8278dd05ba704d879b1f5a Mon Sep 17 00:00:00 2001
From: leo-ard
Date: Wed, 27 Sep 2023 13:35:53 -0400
Subject: [PATCH] Adding ICMP folding for SCCPSolver
---
clang/test/CodeGen/X86/min_max.c | 19 ++
llvm/lib/Transforms/Utils/SCCPSolver.cpp | 54 +
.../Transforms/SCCP/icmp-fold-with-cast.ll| 185 ++
llvm/test/Transforms/SCCP/widening.ll | 4 +-
4 files changed, 260 insertions(+), 2 deletions(-)
create mode 100644 clang/test/CodeGen/X86/min_max.c
create mode 100644 llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
diff --git a/clang/test/CodeGen/X86/min_max.c b/clang/test/CodeGen/X86/min_max.c
new file mode 100644
index 000..7af8181cc9ff367
--- /dev/null
+++ b/clang/test/CodeGen/X86/min_max.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -O2 -triple=x86_64-apple-darwin -emit-llvm -o - |
FileCheck %s
+
+short vecreduce_smax_v2i16(int n, short* v)
+{
+ // CHECK: @llvm.smax
+ short p = 0;
+ for (int i = 0; i < n; ++i)
+p = p < v[i] ? v[i] : p;
+ return p;
+}
+
+short vecreduce_smin_v2i16(int n, short* v)
+{
+ // CHECK: @llvm.smin
+ short p = 0;
+ for (int i = 0; i < n; ++i)
+p = p > v[i] ? v[i] : p;
+ return p;
+}
\ No newline at end of file
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 8a67fda7c98e787..09f64a925ab1b7c 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -193,6 +193,60 @@ static bool replaceSignedInst(SCCPSolver ,
NewInst = BinaryOperator::Create(NewOpcode, Op0, Op1, "", );
break;
}
+ case Instruction::ICmp: {
+ICmpInst = cast(Inst);
+
+ZExtInst *Op0_zext = dyn_cast(ICmp.getOperand(0));
+SExtInst *Op0_sext = dyn_cast(ICmp.getOperand(0));
+
+ZExtInst *Op1_zext = dyn_cast(ICmp.getOperand(1));
+SExtInst *Op1_sext = dyn_cast(ICmp.getOperand(1));
+
+CastInst *Op0;
+CastInst *Op1;
+
+if (Op0_zext) Op0 = Op0_zext; else Op0 = Op0_sext;
+if (Op1_zext) Op1 = Op1_zext; else Op1 = Op1_sext;
+
+bool reversed = false;
+
+if (!Op0 || !Op1){
+ // Op0 and Op1 must be defined
+ return false;
+}
+
+if (Op1_zext && (! Op0_zext)){
+ // We force Op0 to be a zext and reverse the arguments
+ // at the end if we swap
+ reversed = true;
+
+ std::swap(Op0_zext, Op1_zext);
+ std::swap(Op0_sext, Op1_sext);
+ std::swap(Op0, Op1);
+}
+
+
+if(Op0->getType() != Op1->getType()){
+ // extensions are not of the same type
+ // This optimization is done in InstCombine
+ return false;
+}
+
+// ICMP (sext X) (sext y) => ICMP X, Y
+// ICMP (zext X) (zext y) => ICMP X, Y
+// ICMP (zext X) (sext Y) => ICMP X, Y if X >= 0 and ICMP signed
+if((Op0_zext && Op1_zext)
+ || (Op0_sext && Op1_sext)
+ || (ICmp.isSigned() && Op0_zext && Op1_sext &&
isNonNegative(Op0_zext->getOperand(0{
+
+ Value *X = Op0->getOperand(0);
+ Value *Y = Op1->getOperand(0);
+ NewInst = CmpInst::Create(ICmp.getOpcode(), ICmp.getPredicate(),
reversed ? Y : X, reversed ? X : Y, "", );
+ break;
+}
+
+return false;
+ }
default:
return false;
}
diff --git a/llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
b/llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
new file mode 100644
index 000..90b2c123081fb49
--- /dev/null
+++ b/llvm/test/Transforms/SCCP/icmp-fold-with-cast.ll
@@ -0,0 +1,185 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
UTC_ARGS: --tool ./bin/opt --version 3
+; See PRXXX for more details
+; RUN: opt < %s -S -passes=ipsccp | FileCheck %s
+
+
+define signext i32 @sext_sext(i16 %x, i16 %y) {
+; CHECK-LABEL: define signext i32 @sext_sext(
+;
