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

[SelectionDAG] Do not crash on large integers in CheckInteger #75787

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

uweigand
Copy link
Member

The CheckInteger routine called from TableGen-generated selection logic uses getSExtValue - which will abort if the underlying APInt does not fit into an int64_t.

This case is now triggered by the SystemZ back-end since i128 is a legal type on certain machines. While we do not have any regular instructions that take 128-bit immediates (like most other platforms), there are patterns in the .td files that recognize an i128 "xor ..., -1" as a "not".

These patterns cause code to be generated that calls the CheckInteger routine on some i128-valued integer, which may trigger the assert.

Fix by using trySExtValue instead.

Fixes #75710

The CheckInteger routine called from TableGen-generated
selection logic uses getSExtValue - which will abort if
the underlying APInt does not fit into an int64_t.

This case is now triggered by the SystemZ back-end since
i128 is a legal type on certain machines.  While we do not
have any regular instructions that take 128-bit immediates
(like most other platforms), there are patterns in the .td
files that recognize an i128 "xor ..., -1" as a "not".

These patterns cause code to be generated that calls the
CheckInteger routine on some i128-valued integer, which
may trigger the assert.

Fix by using trySExtValue instead.

Fixes llvm#75710
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Dec 18, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 18, 2023

@llvm/pr-subscribers-llvm-selectiondag

Author: Ulrich Weigand (uweigand)

Changes

The CheckInteger routine called from TableGen-generated selection logic uses getSExtValue - which will abort if the underlying APInt does not fit into an int64_t.

This case is now triggered by the SystemZ back-end since i128 is a legal type on certain machines. While we do not have any regular instructions that take 128-bit immediates (like most other platforms), there are patterns in the .td files that recognize an i128 "xor ..., -1" as a "not".

These patterns cause code to be generated that calls the CheckInteger routine on some i128-valued integer, which may trigger the assert.

Fix by using trySExtValue instead.

Fixes #75710


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (+1-1)
  • (modified) llvm/test/CodeGen/SystemZ/xor-09.ll (+14)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index a1cf4cbbee1b85..af49ef17a3f2dd 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -2786,7 +2786,7 @@ CheckInteger(const unsigned char *MatcherTable, unsigned &MatcherIndex,
   Val = decodeSignRotatedValue(Val);
 
   ConstantSDNode *C = dyn_cast<ConstantSDNode>(N);
-  return C && C->getSExtValue() == Val;
+  return C && C->getAPIntValue().trySExtValue() == Val;
 }
 
 LLVM_ATTRIBUTE_ALWAYS_INLINE static bool
diff --git a/llvm/test/CodeGen/SystemZ/xor-09.ll b/llvm/test/CodeGen/SystemZ/xor-09.ll
index d0287f7fdd77ec..7b7aaa404c00f6 100644
--- a/llvm/test/CodeGen/SystemZ/xor-09.ll
+++ b/llvm/test/CodeGen/SystemZ/xor-09.ll
@@ -15,3 +15,17 @@ define i128 @f1(i128 %a, i128 %b) {
   %res = xor i128 %a, %b
   ret i128 %res
 }
+
+; Verify that xor with a large constant does not crash.
+define i128 @f2(i128 %x) {
+; CHECK-LABEL: f2:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    larl %r1, .LCPI1_0
+; CHECK-NEXT:    vl %v0, 0(%r3), 3
+; CHECK-NEXT:    vl %v1, 0(%r1), 3
+; CHECK-NEXT:    vx %v0, %v0, %v1
+; CHECK-NEXT:    vst %v0, 0(%r2), 3
+; CHECK-NEXT:    br %r14
+  %res = xor i128 %x, 17440380254424117642
+  ret i128 %res
+}

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@uweigand uweigand merged commit 82a1bff into llvm:main Dec 18, 2023
4 of 5 checks passed
@uweigand uweigand deleted the fix-75710 branch December 18, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with i128 constant during isel
3 participants