Skip to content

[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

Closed
wants to merge 1 commit into from

Conversation

sdesmalen-arm
Copy link
Collaborator

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.

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.
@llvmbot
Copy link
Member

llvmbot commented Jun 23, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Sander de Smalen (sdesmalen-arm)

Changes

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.


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

3 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.td (+7-6)
  • (modified) llvm/test/CodeGen/AArch64/sve2p2-intrinsics.ll (+1)
  • (modified) llvm/test/CodeGen/AArch64/zeroing-forms-ext.ll (+5-2)
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"
 

@paulwalker-arm
Copy link
Collaborator

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?

@paulwalker-arm
Copy link
Collaborator

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.

@paulwalker-arm
Copy link
Collaborator

#145322 - Still a WIP as I'm trying to figure out which tests need updating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants