[AArch64][GlobalISel] Add pattern to prevent scalar uqxtn fallback#201546
Merged
Conversation
Previously, these operands would be on gpr, which neon intrinsics cannot handle. To fix, place these operands on fpr. This still doesn't seem to fix the issue - I assume we need some more patterns.
neon_scalar_uqxtn was failing instruction selection as there was no pattern matching it to UQXTN. Add that pattern.
Add a GI run to the only test the checks scalar.uqxtn.
|
@llvm/pr-subscribers-backend-aarch64 Author: Joshua Rodriguez (JoshdRod) ChangesPreviously, attempting to select the intrinsic @llvm.aarch64.neon.scalar.uqxtn would cause GlobalISel to fall back to SDAG.
3 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index f23ae6501b285..151cbd9bc5a7c 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -6696,6 +6696,9 @@ defm UQXTN : SIMDTwoScalarMixedBHS<1, 0b10100, "uqxtn", int_aarch64_neon_scalar
defm USQADD : SIMDTwoScalarBHSDTied< 1, 0b00011, "usqadd", AArch64usqadd,
int_aarch64_neon_usqadd>;
+def : Pat<(i32 (int_aarch64_neon_scalar_uqxtn (i64 FPR64:$Rn))),
+ (i32 (UQXTNv1i32 FPR64:$Rn))>;
+
// ssub_sat(0, R) -> sqneg(R)
def : Pat<(v16i8 (ssubsat immAllZerosV, V128:$reg)),
(v16i8 (SQNEGv16i8 V128:$reg))>;
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 17350435ea0b8..ee9e57e705750 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -653,6 +653,7 @@ static bool isFPIntrinsic(const MachineRegisterInfo &MRI,
case Intrinsic::aarch64_neon_uqrshrn:
case Intrinsic::aarch64_neon_sqneg:
case Intrinsic::aarch64_neon_sqabs:
+ case Intrinsic::aarch64_neon_scalar_uqxtn:
case Intrinsic::aarch64_crypto_sha1h:
case Intrinsic::aarch64_crypto_sha1c:
case Intrinsic::aarch64_crypto_sha1p:
diff --git a/llvm/test/CodeGen/AArch64/arm64-arith-saturating.ll b/llvm/test/CodeGen/AArch64/arm64-arith-saturating.ll
index 07c4dbcf41096..15812d2d52d40 100644
--- a/llvm/test/CodeGen/AArch64/arm64-arith-saturating.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-arith-saturating.ll
@@ -1,5 +1,12 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=arm64-eabi -mcpu=cyclone | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-eabi -mcpu=cyclone | FileCheck %s --check-prefixes=CHECK
+; RUN: llc < %s -mtriple=arm64-eabi -mcpu=cyclone -global-isel -global-isel-abort=2 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-GI
+
+; CHECK-GI: warning: Instruction selection used fallback path for vqmovund
+; CHECK-GI-NEXT: warning: Instruction selection used fallback path for vqmovnd_s
+; CHECK-GI-NEXT: warning: Instruction selection used fallback path for sqxtn_ins
+; CHECK-GI-NEXT: warning: Instruction selection used fallback path for sqxtun_insext
+; CHECK-GI-NEXT: warning: Instruction selection used fallback path for saddluse
define i32 @qadds(<4 x i32> %b, <4 x i32> %c) nounwind readnone optsize ssp {
; CHECK-LABEL: qadds:
|
arsenm
reviewed
Jun 4, 2026
| defm USQADD : SIMDTwoScalarBHSDTied< 1, 0b00011, "usqadd", AArch64usqadd, | ||
| int_aarch64_neon_usqadd>; | ||
|
|
||
| def : Pat<(i32 (int_aarch64_neon_scalar_uqxtn (i64 FPR64:$Rn))), |
Contributor
There was a problem hiding this comment.
I wouldn't expect to write a new pattern for an intrinsic which works with selectiondag already
davemgreen
reviewed
Jun 4, 2026
Contributor
davemgreen
left a comment
davemgreen
left a comment
There was a problem hiding this comment.
Seems OK to me. There is presumably a sqxtn and sqxtun too that could work the same way. Did you want to add them here or do it in a followup?
Contributor
Author
I was planning to get feedback here and do the rest in a followup - as I'm pretty sure the rest will be exactly the same fix.
The vector version of this intrinsic lowers using a generic opcode. I feel like it doesn't make sense to create a new GI node for this one case - what do you think? |
JoshdRod
added a commit
to JoshdRod/llvm-project
that referenced
this pull request
Jun 4, 2026
Follow on from [llvm#201546])(llvm#201546). Add patterns for signed versions of scalar extend intrinsics as well.
llvm-upstreamsync Bot
pushed a commit
to qualcomm/cpullvm-toolchain
that referenced
this pull request
Jun 5, 2026
…d intrinsics (#201617) Follow on from llvm/llvm-project#201546 Add patterns for signed versions of scalar extend intrinsics as well.
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 5, 2026
…d intrinsics (#201617) Follow on from llvm/llvm-project#201546 Add patterns for signed versions of scalar extend intrinsics as well.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, attempting to select the intrinsic @llvm.aarch64.neon.scalar.uqxtn would cause GlobalISel to fall back to SDAG.
This was both due to:
Add pattern, and fix RegBankSelect to place operands on the correct banks