[clang] [AArch64] Warn when calling a NEON builtin in a streaming function (PR #73672)

Sam Tebbs via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 30 05:34:17 PST 2023


https://github.com/SamTebbs33 updated https://github.com/llvm/llvm-project/pull/73672

>From ba3d2c36ee3268b24864466d429a30fec92a69e3 Mon Sep 17 00:00:00 2001
From: Samuel Tebbs <samuel.tebbs at arm.com>
Date: Tue, 28 Nov 2023 16:22:32 +0000
Subject: [PATCH 1/4] [AArch64] Warn when calling a NEON builtin in a streaming
 function

This patch introduces a warning that is emitted when a Neon builtin is called from a streaming function, as that situation is not supported.
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  3 +
 clang/lib/Sema/SemaChecking.cpp               | 81 +++++++++++++++++++
 .../Sema/aarch64-incompat-sm-builtin-calls.c  | 24 ++++++
 3 files changed, 108 insertions(+)
 create mode 100644 clang/test/Sema/aarch64-incompat-sm-builtin-calls.c

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ed9bd929c6c4816..6dfb2d7195203a3 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 9dfff132cd88db3..b0da86a5b227def 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;
@@ -3148,6 +3204,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 at +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 at +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 at +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);
+}

>From faec17ef4e04e03ff68d2b449a776bce6f699b4c Mon Sep 17 00:00:00 2001
From: Samuel Tebbs <samuel.tebbs at arm.com>
Date: Wed, 29 Nov 2023 11:37:12 +0000
Subject: [PATCH 2/4] fixup! remove unneeded parts and simplify switch
 statement

---
 clang/lib/Sema/SemaChecking.cpp               | 33 +++----------------
 .../Sema/aarch64-incompat-sm-builtin-calls.c  |  2 --
 2 files changed, 5 insertions(+), 30 deletions(-)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index b0da86a5b227def..a6593a1685188eb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2997,8 +2997,7 @@ enum ArmStreamingType {
   ArmNonStreaming,
   ArmStreaming,
   ArmStreamingCompatible,
-  ArmLocallyStreaming,
-  ArmStreamingOrSVE2p1
+  ArmLocallyStreaming
 };
 
 static ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD) {
@@ -3020,15 +3019,6 @@ static void checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall,
          "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) {
@@ -3042,11 +3032,6 @@ static void checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall,
         << 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) {
@@ -3205,28 +3190,20 @@ 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, "");
+#define TARGET_BUILTIN(id, ...) case NEON::BI##id:
+#define BUILTIN(id, ...) case NEON::BI##id:
 #include "clang/Basic/arm_neon.inc"
+      checkArmStreamingBuiltin(*this, TheCall, FD, ArmNonStreaming);
+      break;
 #undef TARGET_BUILTIN
 #undef BUILTIN
 #undef GET_NEON_BUILTINS
     }
-
-    if (IsNeon) {
-      checkArmStreamingBuiltin(*this, TheCall, FD, ArmNonStreaming);
-      return true;
-    }
   }
 
   llvm::APSInt Result;
diff --git a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
index 08ed22917da67ca..d6f0f7d885bb6bf 100644
--- a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
+++ b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
@@ -5,8 +5,6 @@
 // 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 at +1 {{builtin call has undefined behaviour when called from a streaming or locally streaming function}}

>From c5c17a44bde23c488141514383b364a434f0e920 Mon Sep 17 00:00:00 2001
From: Samuel Tebbs <samuel.tebbs at arm.com>
Date: Thu, 30 Nov 2023 11:12:26 +0000
Subject: [PATCH 3/4] fixup! remove ArmLocallyStreaming from enum

---
 clang/lib/Sema/SemaChecking.cpp                    | 14 +++++---------
 .../test/Sema/aarch64-incompat-sm-builtin-calls.c  |  4 ++--
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a6593a1685188eb..b8f3ffa913e5667 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2996,13 +2996,12 @@ static QualType getNeonEltType(NeonTypeFlags Flags, ASTContext &Context,
 enum ArmStreamingType {
   ArmNonStreaming,
   ArmStreaming,
-  ArmStreamingCompatible,
-  ArmLocallyStreaming
+  ArmStreamingCompatible
 };
 
 static ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD) {
   if (FD->hasAttr<ArmLocallyStreamingAttr>())
-    return ArmLocallyStreaming;
+    return ArmStreaming;
   if (const auto *T = FD->getType()->getAs<FunctionProtoType>()) {
     if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
       return ArmStreaming;
@@ -3015,18 +3014,15 @@ static ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD) {
 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 ((FnType == ArmStreaming || FnType == ArmLocallyStreaming) &&
+  if (FnType == ArmStreaming &&
       BuiltinType == ArmNonStreaming) {
     S.Diag(TheCall->getBeginLoc(), diag::warn_attribute_arm_sm_incompat_builtin)
-        << TheCall->getSourceRange() << "streaming or locally streaming";
+        << TheCall->getSourceRange() << "streaming";
   }
 
-  if ((FnType == ArmStreamingCompatible) &&
+  if (FnType == ArmStreamingCompatible &&
       BuiltinType != ArmStreamingCompatible) {
     S.Diag(TheCall->getBeginLoc(), diag::warn_attribute_arm_sm_incompat_builtin)
         << TheCall->getSourceRange() << "streaming compatible";
diff --git a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
index d6f0f7d885bb6bf..e77e09c4435188d 100644
--- a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
+++ b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
@@ -7,12 +7,12 @@
 #include "arm_neon.h"
 
 int16x8_t incompat_neon_sm(int16x8_t splat) __arm_streaming {
-  // expected-warning at +1 {{builtin call has undefined behaviour when called from a streaming or locally streaming function}}
+  // expected-warning at +1 {{builtin call has undefined behaviour when called from a 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 at +1 {{builtin call has undefined behaviour when called from a streaming or locally streaming function}}
+  // expected-warning at +1 {{builtin call has undefined behaviour when called from a streaming function}}
   return (int16x8_t)__builtin_neon_vqaddq_v((int8x16_t)splat, (int8x16_t)splat, 33);
 }
 

>From ab2be0ee420501e6293a6b4516cc7e38164cc3c0 Mon Sep 17 00:00:00 2001
From: Samuel Tebbs <samuel.tebbs at arm.com>
Date: Thu, 30 Nov 2023 13:19:06 +0000
Subject: [PATCH 4/4] fixup! formatting

---
 clang/lib/Sema/SemaChecking.cpp | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index b8f3ffa913e5667..77c8334f3ca25d3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2993,11 +2993,7 @@ static QualType getNeonEltType(NeonTypeFlags Flags, ASTContext &Context,
   llvm_unreachable("Invalid NeonTypeFlag!");
 }
 
-enum ArmStreamingType {
-  ArmNonStreaming,
-  ArmStreaming,
-  ArmStreamingCompatible
-};
+enum ArmStreamingType { ArmNonStreaming, ArmStreaming, ArmStreamingCompatible };
 
 static ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD) {
   if (FD->hasAttr<ArmLocallyStreamingAttr>())
@@ -3016,8 +3012,7 @@ static void checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall,
                                      ArmStreamingType BuiltinType) {
   ArmStreamingType FnType = getArmStreamingFnType(FD);
 
-  if (FnType == ArmStreaming &&
-      BuiltinType == ArmNonStreaming) {
+  if (FnType == ArmStreaming && BuiltinType == ArmNonStreaming) {
     S.Diag(TheCall->getBeginLoc(), diag::warn_attribute_arm_sm_incompat_builtin)
         << TheCall->getSourceRange() << "streaming";
   }



More information about the cfe-commits mailing list