Skip to content

[AArch64] Warn when calling a NEON builtin in a streaming function #73672

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

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

SamTebbs33
Copy link
Collaborator

This patch introduces a warning that is emitted when a Neon builtin is called from a streaming function, as that situation is not supported.

Uses work by Kerry McLaughlin.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 28, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-clang

Author: Sam Tebbs (SamTebbs33)

Changes

This patch introduces a warning that is emitted when a Neon builtin is called from a streaming function, as that situation is not supported.

Uses work by Kerry McLaughlin.


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

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+81)
  • (added) clang/test/Sema/aarch64-incompat-sm-builtin-calls.c (+24)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9a7dafa4a298273..e2b7a695322c14b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3148,6 +3148,9 @@ def err_attribute_bad_sve_vector_size : Error<
 def err_attribute_arm_feature_sve_bits_unsupported : Error<
   "%0 is only supported when '-msve-vector-bits=<bits>' is specified with a "
   "value of 128, 256, 512, 1024 or 2048.">;
+def warn_attribute_arm_sm_incompat_builtin : Warning<
+  "builtin call has undefined behaviour when called from a %0 function">,
+  InGroup<DiagGroup<"undefined-arm-streaming">>;
 def err_sve_vector_in_non_sve_target : Error<
   "SVE vector type %0 cannot be used in a target without sve">;
 def err_attribute_riscv_rvv_bits_unsupported : Error<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ae588db02bbe722..07937047a2843b0 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2993,6 +2993,62 @@ static QualType getNeonEltType(NeonTypeFlags Flags, ASTContext &Context,
   llvm_unreachable("Invalid NeonTypeFlag!");
 }
 
+enum ArmStreamingType {
+  ArmNonStreaming,
+  ArmStreaming,
+  ArmStreamingCompatible,
+  ArmLocallyStreaming,
+  ArmStreamingOrSVE2p1
+};
+
+static ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD) {
+  if (FD->hasAttr<ArmLocallyStreamingAttr>())
+    return ArmLocallyStreaming;
+  if (const auto *T = FD->getType()->getAs<FunctionProtoType>()) {
+    if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
+      return ArmStreaming;
+    if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMCompatibleMask)
+      return ArmStreamingCompatible;
+  }
+  return ArmNonStreaming;
+}
+
+static void checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall,
+                                     const FunctionDecl *FD,
+                                     ArmStreamingType BuiltinType) {
+  assert(BuiltinType != ArmLocallyStreaming &&
+         "Unexpected locally_streaming attribute for builtin!");
+
+  ArmStreamingType FnType = getArmStreamingFnType(FD);
+  if (BuiltinType == ArmStreamingOrSVE2p1) {
+    // Check intrinsics that are available in [sve2p1 or sme/sme2].
+    llvm::StringMap<bool> CallerFeatureMap;
+    S.Context.getFunctionFeatureMap(CallerFeatureMap, FD);
+    if (Builtin::evaluateRequiredTargetFeatures("sve2p1", CallerFeatureMap))
+      BuiltinType = ArmStreamingCompatible;
+    else
+      BuiltinType = ArmStreaming;
+  }
+
+  if ((FnType == ArmStreaming || FnType == ArmLocallyStreaming) &&
+      BuiltinType == ArmNonStreaming) {
+    S.Diag(TheCall->getBeginLoc(), diag::warn_attribute_arm_sm_incompat_builtin)
+        << TheCall->getSourceRange() << "streaming or locally streaming";
+  }
+
+  if ((FnType == ArmStreamingCompatible) &&
+      BuiltinType != ArmStreamingCompatible) {
+    S.Diag(TheCall->getBeginLoc(), diag::warn_attribute_arm_sm_incompat_builtin)
+        << TheCall->getSourceRange() << "streaming compatible";
+    return;
+  }
+
+  if (FnType == ArmNonStreaming && BuiltinType == ArmStreaming) {
+    S.Diag(TheCall->getBeginLoc(), diag::warn_attribute_arm_sm_incompat_builtin)
+        << TheCall->getSourceRange() << "non-streaming";
+  }
+}
+
 bool Sema::CheckSVEBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   // Range check SVE intrinsics that take immediate values.
   SmallVector<std::tuple<int,int,int>, 3> ImmChecks;
@@ -3136,6 +3192,31 @@ bool Sema::CheckSVEBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
 
 bool Sema::CheckNeonBuiltinFunctionCall(const TargetInfo &TI,
                                         unsigned BuiltinID, CallExpr *TheCall) {
+  if (const FunctionDecl *FD = getCurFunctionDecl()) {
+    std::optional<ArmStreamingType> BuiltinType;
+
+    bool IsNeon = false;
+    switch (BuiltinID) {
+    default:
+      break;
+#define GET_NEON_BUILTINS
+#define TARGET_BUILTIN(id, x, y, z)                                            \
+  case NEON::BI##id:                                                           \
+    IsNeon = true;                                                             \
+    break;
+#define BUILTIN(id, x, y) TARGET_BUILTIN(id, x, y, "");
+#include "clang/Basic/arm_neon.inc"
+#undef TARGET_BUILTIN
+#undef BUILTIN
+#undef GET_NEON_BUILTINS
+    }
+
+    if (IsNeon) {
+      checkArmStreamingBuiltin(*this, TheCall, FD, ArmNonStreaming);
+      return true;
+    }
+  }
+
   llvm::APSInt Result;
   uint64_t mask = 0;
   unsigned TV = 0;
diff --git a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
new file mode 100644
index 000000000000000..08ed22917da67ca
--- /dev/null
+++ b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
@@ -0,0 +1,24 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1  -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN:   -target-feature +sme -target-feature +sve2 -target-feature +neon -fsyntax-only -verify %s
+
+// REQUIRES: aarch64-registered-target
+
+#include "arm_neon.h"
+#include "arm_sme_draft_spec_subject_to_change.h"
+#include "arm_sve.h"
+
+int16x8_t incompat_neon_sm(int16x8_t splat) __arm_streaming {
+  // expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming or locally streaming function}}
+  return (int16x8_t)__builtin_neon_vqaddq_v((int8x16_t)splat, (int8x16_t)splat, 33);
+}
+
+__arm_locally_streaming int16x8_t incompat_neon_ls(int16x8_t splat) {
+  // expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming or locally streaming function}}
+  return (int16x8_t)__builtin_neon_vqaddq_v((int8x16_t)splat, (int8x16_t)splat, 33);
+}
+
+int16x8_t incompat_neon_smc(int16x8_t splat) __arm_streaming_compatible {
+  // expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming compatible function}}
+  return (int16x8_t)__builtin_neon_vqaddq_v((int8x16_t)splat, (int8x16_t)splat, 33);
+}

This patch introduces a warning that is emitted when a Neon builtin is called from a streaming function, as that situation is not supported.
Copy link
Collaborator

@sdesmalen-arm sdesmalen-arm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

github-actions bot commented Nov 30, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@SamTebbs33 SamTebbs33 merged commit 5234fe3 into llvm:main Nov 30, 2023
@SamTebbs33 SamTebbs33 deleted the sme-attrs2 branch December 6, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants