Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only check assertions that were meant to apply to the normal case of non-splat vector SREM expansion when we aren't hitting the special case. #86238

Merged
merged 2 commits into from
Mar 24, 2024

Conversation

resistor
Copy link
Collaborator

Fixes #84830
Introduced in #82706

of non-splat vector SREM expansion when we aren't hitting the
special case.

Fixes llvm#84830
Introduced in llvm#82706
@llvmbot llvmbot added backend:AArch64 llvm:SelectionDAG SelectionDAGISel as well labels Mar 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-llvm-selectiondag

Author: Owen Anderson (resistor)

Changes

Fixes #84830
Introduced in #82706


Full diff: https://github.com/llvm/llvm-project/pull/86238.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+5-5)
  • (added) llvm/test/CodeGen/AArch64/srem-vec-crash.ll (+14)
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index da29b1d5b312f8..8be03b66e155f6 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -6916,6 +6916,11 @@ TargetLowering::prepareSREMEqFold(EVT SETCCVT, SDValue REMNode,
     // Q = floor((2 * A) / (2^K))
     APInt Q = (2 * A).udiv(APInt::getOneBitSet(W, K));
 
+    assert(APInt::getAllOnes(SVT.getSizeInBits()).ugt(A) &&
+           "We are expecting that A is always less than all-ones for SVT");
+    assert(APInt::getAllOnes(ShSVT.getSizeInBits()).ugt(K) &&
+           "We are expecting that K is always less than all-ones for ShSVT");
+
     // If D was a power of two, apply the alternate constant derivation.
     if (D0.isOne()) {
       // A = 2^(W-1)
@@ -6924,11 +6929,6 @@ TargetLowering::prepareSREMEqFold(EVT SETCCVT, SDValue REMNode,
       Q = APInt::getAllOnes(W - K).zext(W);
     }
 
-    assert(APInt::getAllOnes(SVT.getSizeInBits()).ugt(A) &&
-           "We are expecting that A is always less than all-ones for SVT");
-    assert(APInt::getAllOnes(ShSVT.getSizeInBits()).ugt(K) &&
-           "We are expecting that K is always less than all-ones for ShSVT");
-
     // If the divisor is 1 the result can be constant-folded. Likewise, we
     // don't care about INT_MIN lanes, those can be set to undef if appropriate.
     if (D.isOne()) {
diff --git a/llvm/test/CodeGen/AArch64/srem-vec-crash.ll b/llvm/test/CodeGen/AArch64/srem-vec-crash.ll
new file mode 100644
index 00000000000000..3b8a1c83b2f697
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/srem-vec-crash.ll
@@ -0,0 +1,14 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+;RUN: llc -mtriple=aarch64-unknown-unknown < %s | FileCheck %s
+
+define i32 @f(i1 %0) {
+; CHECK-LABEL: f:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w0, #1 // =0x1
+; CHECK-NEXT:    ret
+  %new0 = srem i1 %0, true
+  %last = zext i1 %new0 to i32
+  %2 = icmp ne i32 %last, 0
+  %3 = select i1 %2, i32 0, i32 1
+  ret i32 %3
+}

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Owen Anderson (resistor)

Changes

Fixes #84830
Introduced in #82706


Full diff: https://github.com/llvm/llvm-project/pull/86238.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+5-5)
  • (added) llvm/test/CodeGen/AArch64/srem-vec-crash.ll (+14)
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index da29b1d5b312f8..8be03b66e155f6 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -6916,6 +6916,11 @@ TargetLowering::prepareSREMEqFold(EVT SETCCVT, SDValue REMNode,
     // Q = floor((2 * A) / (2^K))
     APInt Q = (2 * A).udiv(APInt::getOneBitSet(W, K));
 
+    assert(APInt::getAllOnes(SVT.getSizeInBits()).ugt(A) &&
+           "We are expecting that A is always less than all-ones for SVT");
+    assert(APInt::getAllOnes(ShSVT.getSizeInBits()).ugt(K) &&
+           "We are expecting that K is always less than all-ones for ShSVT");
+
     // If D was a power of two, apply the alternate constant derivation.
     if (D0.isOne()) {
       // A = 2^(W-1)
@@ -6924,11 +6929,6 @@ TargetLowering::prepareSREMEqFold(EVT SETCCVT, SDValue REMNode,
       Q = APInt::getAllOnes(W - K).zext(W);
     }
 
-    assert(APInt::getAllOnes(SVT.getSizeInBits()).ugt(A) &&
-           "We are expecting that A is always less than all-ones for SVT");
-    assert(APInt::getAllOnes(ShSVT.getSizeInBits()).ugt(K) &&
-           "We are expecting that K is always less than all-ones for ShSVT");
-
     // If the divisor is 1 the result can be constant-folded. Likewise, we
     // don't care about INT_MIN lanes, those can be set to undef if appropriate.
     if (D.isOne()) {
diff --git a/llvm/test/CodeGen/AArch64/srem-vec-crash.ll b/llvm/test/CodeGen/AArch64/srem-vec-crash.ll
new file mode 100644
index 00000000000000..3b8a1c83b2f697
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/srem-vec-crash.ll
@@ -0,0 +1,14 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+;RUN: llc -mtriple=aarch64-unknown-unknown < %s | FileCheck %s
+
+define i32 @f(i1 %0) {
+; CHECK-LABEL: f:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w0, #1 // =0x1
+; CHECK-NEXT:    ret
+  %new0 = srem i1 %0, true
+  %last = zext i1 %new0 to i32
+  %2 = icmp ne i32 %last, 0
+  %3 = select i1 %2, i32 0, i32 1
+  ret i32 %3
+}

@resistor resistor requested a review from arsenm March 22, 2024 03:38
llvm/test/CodeGen/AArch64/srem-vec-crash.ll Outdated Show resolved Hide resolved
llvm/test/CodeGen/AArch64/srem-vec-crash.ll Outdated Show resolved Hide resolved
@resistor resistor merged commit 7c9b522 into llvm:main Mar 24, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assertion failing in multiple backends "We are expecting that A is always less than all-ones for SVT"
3 participants