-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[AArch64] Fix predicates for SME2p2. #145315
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
Conversation
The following two expressions are not equal: (hasSVE2p2() || hasSME2p2()) && isSVEorStreamingSVEAvailable() (hasSVE2p2() || (hasSME2p2() && isStreaming()) If the target has +sve,+sme2p2 and the function is not a streaming function, then the former expression would evaluate to true, thus incorrectly assuming the target has SVE2p2. This patch also removes redundant `hasSVE() from `isSVEAvailable() && hasSVE()`, which is an NFC change.
@llvm/pr-subscribers-backend-aarch64 Author: Sander de Smalen (sdesmalen-arm) ChangesThe following two expressions are not equal: (hasSVE2p2() || hasSME2p2()) && isSVEorStreamingSVEAvailable() If the target has +sve,+sme2p2 and the function is not a streaming function, then the former expression would evaluate to true, thus incorrectly assuming the target has SVE2p2. This patch also removes redundant Full diff: https://github.com/llvm/llvm-project/pull/145315.diff 3 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 0f3f24f0853c9..b1f2cdec9f033 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -249,13 +249,13 @@ def HasSVE_or_SME
AssemblerPredicateWithAll<(any_of FeatureSVE, FeatureSME),
"sve or sme">;
def HasNonStreamingSVE_or_SME2p2
- : Predicate<"(Subtarget->isSVEAvailable() && Subtarget->hasSVE()) ||"
- "(Subtarget->isSVEorStreamingSVEAvailable() && Subtarget->hasSME2p2())">,
+ : Predicate<"Subtarget->isSVEAvailable() ||"
+ "(Subtarget->isStreaming() && Subtarget->hasSME2p2())">,
AssemblerPredicateWithAll<(any_of FeatureSVE, FeatureSME2p2),
"sve or sme2p2">;
def HasNonStreamingSVE_or_SSVE_FEXPA
- : Predicate<"(Subtarget->isSVEAvailable() && Subtarget->hasSVE()) ||"
- "(Subtarget->isSVEorStreamingSVEAvailable() && Subtarget->hasSSVE_FEXPA())">,
+ : Predicate<"Subtarget->isSVEAvailable() ||"
+ "(Subtarget->isStreaming() && Subtarget->hasSSVE_FEXPA())">,
AssemblerPredicateWithAll<(any_of FeatureSVE, FeatureSSVE_FEXPA),
"sve or ssve-fexpa">;
@@ -287,12 +287,13 @@ def HasSVE2p1_or_SME2p1
"sme2p1 or sve2p1">;
def HasSVE2p2_or_SME2p2
- : Predicate<"Subtarget->isSVEorStreamingSVEAvailable() && (Subtarget->hasSVE2p2() || Subtarget->hasSME2p2())">,
+ : Predicate<"Subtarget->hasSVE2p2() ||"
+ "(Subtarget->isStreaming() && Subtarget->hasSME2p2())">,
AssemblerPredicateWithAll<(any_of FeatureSME2p2, FeatureSVE2p2),
"sme2p2 or sve2p2">;
def HasNonStreamingSVE2p2_or_SME2p2
: Predicate<"(Subtarget->isSVEAvailable() && Subtarget->hasSVE2p2()) ||"
- "(Subtarget->isSVEorStreamingSVEAvailable() && Subtarget->hasSME2p2())">,
+ "(Subtarget->isStreaming() && Subtarget->hasSME2p2())">,
AssemblerPredicateWithAll<(any_of FeatureSVE2p2, FeatureSME2p2),
"sme2p2 or sve2p2">;
diff --git a/llvm/test/CodeGen/AArch64/sve2p2-intrinsics.ll b/llvm/test/CodeGen/AArch64/sve2p2-intrinsics.ll
index 8d863dab7b47d..62a9cd15d3956 100644
--- a/llvm/test/CodeGen/AArch64/sve2p2-intrinsics.ll
+++ b/llvm/test/CodeGen/AArch64/sve2p2-intrinsics.ll
@@ -1,6 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve2p2 < %s | FileCheck %s --check-prefixes=CHECK
; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sme2p2 -force-streaming < %s | FileCheck %s --check-prefixes=CHECK
+; RUN: not --crash llc -mtriple=aarch64-linux-gnu -mattr=+sve,+sme2p2 < %s
;
; COMPACT
diff --git a/llvm/test/CodeGen/AArch64/zeroing-forms-ext.ll b/llvm/test/CodeGen/AArch64/zeroing-forms-ext.ll
index 20a5475706c9c..e19a9527532c8 100644
--- a/llvm/test/CodeGen/AArch64/zeroing-forms-ext.ll
+++ b/llvm/test/CodeGen/AArch64/zeroing-forms-ext.ll
@@ -1,9 +1,12 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
; RUN: llc -mattr=+sve < %s | FileCheck %s -check-prefixes=CHECK,SVE
-; RUN: llc -mattr=+sve2p2 < %s | FileCheck %s -check-prefix CHECK-2p2
+; RUN: llc -mattr=+sve2p2 < %s | FileCheck %s -check-prefix=CHECK-2p2
; RUN: llc -mattr=+sme -force-streaming < %s | FileCheck %s -check-prefixes=CHECK,STREAMING-SVE
-; RUN: llc -mattr=+sme2p2 -force-streaming < %s | FileCheck %s -check-prefix CHECK-2p2
+; RUN: llc -mattr=+sme2p2 -force-streaming < %s | FileCheck %s -check-prefix=CHECK-2p2
+
+; SME2p2 instructions are only valid in streaming mode
+; RUN: llc -mattr=+sve,+sme2p2 < %s | FileCheck %s -check-prefixes=CHECK,SVE
target triple = "aarch64-linux"
|
Hi @sdesmalen-arm, I'm currently working on a PR to fix all the predicates where SVE and SME are used together. Do you mind dropping this PR and leaving it with me? |
Just to add some colour, you'll see from my fixes that your PR is going in the opposite direction. Specially, I think the current implementation of HasSVE2p2_or_SME2p2 is correct in that +sve,+sme2p2 should enable the relevant SVE instructions when in non-streaming mode. |
#145322 - Still a WIP as I'm trying to figure out which tests need updating. |
The following two expressions are not equal:
(hasSVE2p2() || hasSME2p2()) && isSVEorStreamingSVEAvailable()
(hasSVE2p2() || (hasSME2p2() && isStreaming())
If the target has +sve,+sme2p2 and the function is not a streaming function, then the former expression would evaluate to true, thus incorrectly assuming the target has SVE2p2.
This patch also removes redundant
hasSVE() from
isSVEAvailable() && hasSVE()`, which is an NFC change.