[llvm] KnownBits: refine srem for high-bits (PR #109121)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 23 09:08:06 PDT 2024
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/109121
>From 7368130a586ea09ba27b527bf8fa82d14d0d9789 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 17 Sep 2024 16:38:32 +0100
Subject: [PATCH 1/3] ValueTracking/test: cover known-high-bits of rem
There is an underlying bug in KnownBits, and we should theoretically be
able to determine the high-bits of an srem as shown in the test, just
like urem. In preparation to fix this bug, add pre-commit tests testing
high-bits of srem and urem.
---
...wnbits-rem-lowbits.ll => knownbits-rem.ll} | 79 +++++++++++++++++++
1 file changed, 79 insertions(+)
rename llvm/test/Analysis/ValueTracking/{knownbits-rem-lowbits.ll => knownbits-rem.ll} (65%)
diff --git a/llvm/test/Analysis/ValueTracking/knownbits-rem-lowbits.ll b/llvm/test/Analysis/ValueTracking/knownbits-rem.ll
similarity index 65%
rename from llvm/test/Analysis/ValueTracking/knownbits-rem-lowbits.ll
rename to llvm/test/Analysis/ValueTracking/knownbits-rem.ll
index 0521c7130055fe..e5512fa71ae0e2 100644
--- a/llvm/test/Analysis/ValueTracking/knownbits-rem-lowbits.ll
+++ b/llvm/test/Analysis/ValueTracking/knownbits-rem.ll
@@ -12,6 +12,17 @@ define i8 @urem_low_bits_know(i8 %xx, i8 %yy) {
ret i8 %r
}
+define i8 @urem_high_bits_know(i8 %xx, i8 %yy) {
+; CHECK-LABEL: @urem_high_bits_know(
+; CHECK-NEXT: ret i8 0
+;
+ %x = and i8 %xx, 2
+ %y = and i8 %yy, -4
+ %rem = urem i8 %x, %y
+ %r = and i8 %rem, 8
+ ret i8 %r
+}
+
define i8 @urem_low_bits_know2(i8 %xx, i8 %yy) {
; CHECK-LABEL: @urem_low_bits_know2(
; CHECK-NEXT: ret i8 2
@@ -91,6 +102,74 @@ define i8 @srem_low_bits_know2(i8 %xx, i8 %yy) {
ret i8 %r
}
+define i8 @srem_high_bits_know(i8 %xx, i8 %yy) {
+; CHECK-LABEL: @srem_high_bits_know(
+; CHECK-NEXT: [[X:%.*]] = or i8 [[XX:%.*]], -2
+; CHECK-NEXT: [[Y:%.*]] = and i8 [[YY:%.*]], -4
+; CHECK-NEXT: [[REM:%.*]] = srem i8 [[X]], [[Y]]
+; CHECK-NEXT: [[R:%.*]] = and i8 [[REM]], -2
+; CHECK-NEXT: ret i8 [[R]]
+;
+ %x = or i8 %xx, -2
+ %y = and i8 %yy, -4
+ %rem = srem i8 %x, %y
+ %r = and i8 %rem, -2
+ ret i8 %r
+}
+
+define i8 @srem_high_bits_know2(i8 %xx, i8 %yy) {
+; CHECK-LABEL: @srem_high_bits_know2(
+; CHECK-NEXT: [[X:%.*]] = and i8 [[XX:%.*]], 13
+; CHECK-NEXT: [[Y:%.*]] = or i8 [[YY:%.*]], -4
+; CHECK-NEXT: [[REM:%.*]] = srem i8 [[X]], [[Y]]
+; CHECK-NEXT: [[R:%.*]] = and i8 [[REM]], 8
+; CHECK-NEXT: ret i8 [[R]]
+;
+ %x = and i8 %xx, 13
+ %y = or i8 %yy, -4
+ %rem = srem i8 %x, %y
+ %r = and i8 %rem, 8
+ ret i8 %r
+}
+
+define i8 @srem_high_bits_know3(i8 %xx, i8 %yy) {
+; CHECK-LABEL: @srem_high_bits_know3(
+; CHECK-NEXT: [[X:%.*]] = or i8 [[XX:%.*]], -13
+; CHECK-NEXT: [[Y:%.*]] = and i8 [[YY:%.*]], 4
+; CHECK-NEXT: [[REM:%.*]] = srem i8 [[X]], [[Y]]
+; CHECK-NEXT: [[R:%.*]] = and i8 [[REM]], 8
+; CHECK-NEXT: ret i8 [[R]]
+;
+ %x = or i8 %xx, -13
+ %y = and i8 %yy, 4
+ %rem = srem i8 %x, %y
+ %r = and i8 %rem, 8
+ ret i8 %r
+}
+
+define i8 @srem_high_bits_know4(i8 %xx, i8 %yy) {
+; CHECK-LABEL: @srem_high_bits_know4(
+; CHECK-NEXT: ret i8 0
+;
+ %x = and i8 %xx, 4
+ %y = or i8 %yy, -13
+ %rem = srem i8 %x, %y
+ %r = and i8 %rem, 8
+ ret i8 %r
+}
+
+define i8 @srem_high_bits_know5(i8 %xx, i8 %yy) {
+; CHECK-LABEL: @srem_high_bits_know5(
+; CHECK-NEXT: [[X:%.*]] = and i8 [[XX:%.*]], 2
+; CHECK-NEXT: ret i8 [[X]]
+;
+ %x = and i8 %xx, 2
+ %y = and i8 %yy, 4
+ %rem = srem i8 %x, %y
+ %r = and i8 %rem, 2
+ ret i8 %r
+}
+
define i8 @srem_todo_low_bits_partially_know_should_fold_out_srem(i8 %xx, i8 %yy) {
; CHECK-LABEL: @srem_todo_low_bits_partially_know_should_fold_out_srem(
; CHECK-NEXT: [[X:%.*]] = or i8 [[XX:%.*]], 10
>From 3622e4390b005a67c2ed1415a5d0f3eb61e8c58e Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 17 Sep 2024 16:02:54 +0100
Subject: [PATCH 2/3] KnownBits: refine KnownBits::srem for high-bits
KnownBits::srem does not correctly set the leader zero-bits, omitting
the fact that LHS may be known-negative or known-non-negative. Fix this.
Alive2 proof: https://alive2.llvm.org/ce/z/WcDkSX
---
llvm/lib/Support/KnownBits.cpp | 12 +++++++++---
.../Analysis/ValueTracking/knownbits-rem.ll | 18 +++---------------
llvm/test/CodeGen/ARM/select-imm.ll | 17 +++++++----------
3 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/llvm/lib/Support/KnownBits.cpp b/llvm/lib/Support/KnownBits.cpp
index 8e31e0ced2d731..00446fa2868969 100644
--- a/llvm/lib/Support/KnownBits.cpp
+++ b/llvm/lib/Support/KnownBits.cpp
@@ -1075,9 +1075,15 @@ KnownBits KnownBits::srem(const KnownBits &LHS, const KnownBits &RHS) {
// The sign bit is the LHS's sign bit, except when the result of the
// remainder is zero. The magnitude of the result should be less than or
- // equal to the magnitude of the LHS. Therefore any leading zeros that exist
- // in the left hand side must also exist in the result.
- Known.Zero.setHighBits(LHS.countMinLeadingZeros());
+ // equal to the magnitude of either operand.
+ if (LHS.isNegative() && Known.isNonZero()) {
+ Known.One.setHighBits(
+ std::max(LHS.countMinLeadingOnes(), RHS.countMinSignBits()));
+ } else if (LHS.isNonNegative()) {
+ unsigned Lead =
+ std::max(LHS.countMinLeadingZeros(), RHS.countMinLeadingZeros());
+ Known.Zero.setHighBits(std::max(Lead + 1, RHS.countMinLeadingOnes()) - 1);
+ }
return Known;
}
diff --git a/llvm/test/Analysis/ValueTracking/knownbits-rem.ll b/llvm/test/Analysis/ValueTracking/knownbits-rem.ll
index e5512fa71ae0e2..0aa340c46bdeca 100644
--- a/llvm/test/Analysis/ValueTracking/knownbits-rem.ll
+++ b/llvm/test/Analysis/ValueTracking/knownbits-rem.ll
@@ -104,11 +104,7 @@ define i8 @srem_low_bits_know2(i8 %xx, i8 %yy) {
define i8 @srem_high_bits_know(i8 %xx, i8 %yy) {
; CHECK-LABEL: @srem_high_bits_know(
-; CHECK-NEXT: [[X:%.*]] = or i8 [[XX:%.*]], -2
-; CHECK-NEXT: [[Y:%.*]] = and i8 [[YY:%.*]], -4
-; CHECK-NEXT: [[REM:%.*]] = srem i8 [[X]], [[Y]]
-; CHECK-NEXT: [[R:%.*]] = and i8 [[REM]], -2
-; CHECK-NEXT: ret i8 [[R]]
+; CHECK-NEXT: ret i8 -2
;
%x = or i8 %xx, -2
%y = and i8 %yy, -4
@@ -119,11 +115,7 @@ define i8 @srem_high_bits_know(i8 %xx, i8 %yy) {
define i8 @srem_high_bits_know2(i8 %xx, i8 %yy) {
; CHECK-LABEL: @srem_high_bits_know2(
-; CHECK-NEXT: [[X:%.*]] = and i8 [[XX:%.*]], 13
-; CHECK-NEXT: [[Y:%.*]] = or i8 [[YY:%.*]], -4
-; CHECK-NEXT: [[REM:%.*]] = srem i8 [[X]], [[Y]]
-; CHECK-NEXT: [[R:%.*]] = and i8 [[REM]], 8
-; CHECK-NEXT: ret i8 [[R]]
+; CHECK-NEXT: ret i8 0
;
%x = and i8 %xx, 13
%y = or i8 %yy, -4
@@ -134,11 +126,7 @@ define i8 @srem_high_bits_know2(i8 %xx, i8 %yy) {
define i8 @srem_high_bits_know3(i8 %xx, i8 %yy) {
; CHECK-LABEL: @srem_high_bits_know3(
-; CHECK-NEXT: [[X:%.*]] = or i8 [[XX:%.*]], -13
-; CHECK-NEXT: [[Y:%.*]] = and i8 [[YY:%.*]], 4
-; CHECK-NEXT: [[REM:%.*]] = srem i8 [[X]], [[Y]]
-; CHECK-NEXT: [[R:%.*]] = and i8 [[REM]], 8
-; CHECK-NEXT: ret i8 [[R]]
+; CHECK-NEXT: ret i8 8
;
%x = or i8 %xx, -13
%y = and i8 %yy, 4
diff --git a/llvm/test/CodeGen/ARM/select-imm.ll b/llvm/test/CodeGen/ARM/select-imm.ll
index 65288e1884c742..6427a3e34cf8e2 100644
--- a/llvm/test/CodeGen/ARM/select-imm.ll
+++ b/llvm/test/CodeGen/ARM/select-imm.ll
@@ -655,14 +655,11 @@ define i1 @t10() {
; V8MBASE-NEXT: .pad #8
; V8MBASE-NEXT: sub sp, #8
; V8MBASE-NEXT: movs r0, #7
-; V8MBASE-NEXT: mvns r0, r0
-; V8MBASE-NEXT: str r0, [sp]
-; V8MBASE-NEXT: adds r1, r0, #5
-; V8MBASE-NEXT: str r1, [sp, #4]
-; V8MBASE-NEXT: sdiv r2, r1, r0
-; V8MBASE-NEXT: muls r2, r0, r2
-; V8MBASE-NEXT: subs r0, r1, r2
-; V8MBASE-NEXT: subs r1, r0, r1
+; V8MBASE-NEXT: mvns r1, r0
+; V8MBASE-NEXT: str r1, [sp]
+; V8MBASE-NEXT: adds r0, r1, #5
+; V8MBASE-NEXT: str r0, [sp, #4]
+; V8MBASE-NEXT: adds r1, #8
; V8MBASE-NEXT: rsbs r0, r1, #0
; V8MBASE-NEXT: adcs r0, r1
; V8MBASE-NEXT: add sp, #8
@@ -719,7 +716,7 @@ define i1 @t11() {
; ARMT2-NEXT: and r1, r1, r2
; ARMT2-NEXT: orr r0, r1, r0
; ARMT2-NEXT: str r0, [sp]
-; ARMT2-NEXT: bfc r0, #12, #20
+; ARMT2-NEXT: and r0, r0, #15
; ARMT2-NEXT: sub r0, r0, #3
; ARMT2-NEXT: clz r0, r0
; ARMT2-NEXT: lsr r0, r0, #5
@@ -781,7 +778,7 @@ define i1 @t11() {
; THUMB2-NEXT: ands r1, r2
; THUMB2-NEXT: orrs r0, r1
; THUMB2-NEXT: str r0, [sp]
-; THUMB2-NEXT: bfc r0, #12, #20
+; THUMB2-NEXT: and r0, r0, #15
; THUMB2-NEXT: subs r0, #3
; THUMB2-NEXT: clz r0, r0
; THUMB2-NEXT: lsrs r0, r0, #5
>From 3c3e080d492e0e91b95b42615dd773a7db1d671b Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 23 Sep 2024 17:06:49 +0100
Subject: [PATCH 3/3] KnownBits: simplify code
---
llvm/lib/Support/KnownBits.cpp | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Support/KnownBits.cpp b/llvm/lib/Support/KnownBits.cpp
index 00446fa2868969..6863c5c0af5dca 100644
--- a/llvm/lib/Support/KnownBits.cpp
+++ b/llvm/lib/Support/KnownBits.cpp
@@ -1076,14 +1076,12 @@ KnownBits KnownBits::srem(const KnownBits &LHS, const KnownBits &RHS) {
// The sign bit is the LHS's sign bit, except when the result of the
// remainder is zero. The magnitude of the result should be less than or
// equal to the magnitude of either operand.
- if (LHS.isNegative() && Known.isNonZero()) {
+ if (LHS.isNegative() && Known.isNonZero())
Known.One.setHighBits(
std::max(LHS.countMinLeadingOnes(), RHS.countMinSignBits()));
- } else if (LHS.isNonNegative()) {
- unsigned Lead =
- std::max(LHS.countMinLeadingZeros(), RHS.countMinLeadingZeros());
- Known.Zero.setHighBits(std::max(Lead + 1, RHS.countMinLeadingOnes()) - 1);
- }
+ else if (LHS.isNonNegative())
+ Known.Zero.setHighBits(
+ std::max(LHS.countMinLeadingZeros(), RHS.countMinSignBits()));
return Known;
}
More information about the llvm-commits
mailing list