[clang] [Clang][AArch64] Fix checkArmStreamingBuiltin for 'sve-b16b16' (PR #109420)

via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 20 06:34:01 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Sander de Smalen (sdesmalen-arm)

<details>
<summary>Changes</summary>

The implementation made the assumption that any feature starting with "sve" meant that this was an SVE feature. This is not the case for "sve-b16b16", as this is a feature that applies to both SVE and SME.

This meant that:
```
  __attribute__((target("+sme2,+sve2,+sve-b16b16")))
  svbfloat16_t foo(svbfloat16_t a, svbfloat16_t b, svbfloat16_t c)
                                                      __arm_streaming {
      return svclamp_bf16(a, b, c);
  }
```
would result in an incorrect diagnostic saying that `svclamp_bf16` could only be used in non-streaming functions.

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaARM.cpp (+13-8) 
- (modified) clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c (+6) 


``````````diff
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index efde354860de43..fba1453e5d38fc 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -567,15 +567,18 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall,
   // * When compiling for SVE only, the caller must be in non-streaming mode.
   // * When compiling for both SVE and SME, the caller can be in either mode.
   if (BuiltinType == SemaARM::VerifyRuntimeMode) {
-    auto DisableFeatures = [](llvm::StringMap<bool> &Map, StringRef S) {
-      for (StringRef K : Map.keys())
-        if (K.starts_with(S))
-          Map[K] = false;
-    };
-
     llvm::StringMap<bool> CallerFeatureMapWithoutSVE;
     S.Context.getFunctionFeatureMap(CallerFeatureMapWithoutSVE, FD);
-    DisableFeatures(CallerFeatureMapWithoutSVE, "sve");
+    CallerFeatureMapWithoutSVE["sve"] = false;
+    CallerFeatureMapWithoutSVE["sve2"] = false;
+    CallerFeatureMapWithoutSVE["sve2p1"] = false;
+    // FIXME: This list must be updated with future extensions, because when
+    // an intrinsic is enabled by (sve2p1|sme2p1), disabling just "sve" is
+    // not sufficient, as the feature dependences are not resolved.
+    // At the moment, it should be sufficient to test the 'base' architectural
+    // support for SVE and SME, which must always be provided in the
+    // target guard. e.g. TargetGuard = "sve-b16b16" without "sme" or "sve"
+    // is not sufficient.
 
     // Avoid emitting diagnostics for a function that can never compile.
     if (FnType == SemaARM::ArmStreaming && !CallerFeatureMapWithoutSVE["sme"])
@@ -583,7 +586,9 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall,
 
     llvm::StringMap<bool> CallerFeatureMapWithoutSME;
     S.Context.getFunctionFeatureMap(CallerFeatureMapWithoutSME, FD);
-    DisableFeatures(CallerFeatureMapWithoutSME, "sme");
+    CallerFeatureMapWithoutSME["sme"] = false;
+    CallerFeatureMapWithoutSME["sme2"] = false;
+    CallerFeatureMapWithoutSME["sme2p1"] = false;
 
     // We know the builtin requires either some combination of SVE flags, or
     // some combination of SME flags, but we need to figure out which part
diff --git a/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c b/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
index 45776eb13e4fbc..792d79ee3e600d 100644
--- a/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
+++ b/clang/test/Sema/aarch64-streaming-sme-or-nonstreaming-sve-builtins.c
@@ -38,6 +38,12 @@ svfloat32_t good6(svfloat32_t a, svfloat32_t b, svfloat32_t c) __arm_streaming_c
   return svclamp(a, b, c);
 }
 
+// Test that the +sve-b16b16 is not considered an SVE flag (it applies to both)
+__attribute__((target("+sme2,+sve2,+sve-b16b16")))
+svbfloat16_t good7(svbfloat16_t a, svbfloat16_t b, svbfloat16_t c) __arm_streaming {
+    return svclamp_bf16(a, b, c);
+}
+
 // Without '+sme2', the builtin is only valid in non-streaming mode.
 __attribute__((target("+sve2p1,+sme")))
 svfloat32_t bad1(svfloat32_t a, svfloat32_t b, svfloat32_t c) __arm_streaming {

``````````

</details>


https://github.com/llvm/llvm-project/pull/109420


More information about the cfe-commits mailing list