[Clang][Sema] Reject unsupported opencl address space attributes in SYCL and HLSL #152528
+71
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…YCL and HLSL Raise an error instead of executing unreachable when an unsupported named address space attribute is used in SYCL or HLSL. Arguably some of these could be accepted, but in the interest of simplicity reject them for now instead of crashing. Fixes: llvm#152460
Maetveis
added
clang:frontend
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Mészáros Gergely (Maetveis) ChangesRaise an error instead of executing unreachable when an unsupported named address space attribute is used in SYCL or HLSL. Fixes: #152460 Full diff: https://github.com/llvm/llvm-project/pull/152528.diff 6 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0e9fcaa5fac6a..0b71ad32d6a86 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -161,6 +161,8 @@ Bug Fixes in This Version
targets that treat ``_Float16``/``__fp16`` as native scalar types. Previously
the warning was silently lost because the operands differed only by an implicit
cast chain. (#GH149967).
+- Fix a crash when using opencl address space attributes in HLSL or SYCL device code.
+ (#GH152460).
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index cf23594201143..41a9d26673e7e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4660,7 +4660,7 @@ def err_attribute_regparm_wrong_platform : Error<
def err_attribute_regparm_invalid_number : Error<
"'regparm' parameter must be between 0 and %0 inclusive">;
def err_attribute_not_supported_in_lang : Error<
- "%0 attribute is not supported in %select{C|C++|Objective-C}1">;
+ "%0 attribute is not supported in %select{C|C++|HLSL|Objective-C|SYCL when compiling for the device}1">;
def err_attribute_not_supported_on_arch
: Error<"%0 attribute is not supported on '%1'">;
def warn_gcc_ignores_type_attr : Warning<
diff --git a/clang/lib/Sema/AttributeLangSupport.h b/clang/lib/Sema/AttributeLangSupport.h
new file mode 100644
index 0000000000000..601cd6729d154
--- /dev/null
+++ b/clang/lib/Sema/AttributeLangSupport.h
@@ -0,0 +1,30 @@
+//===- AttributeLangSupport.h --------------------------------- -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the declaration of AttributeLangSupport, which is used in
+/// diagnostics to indicate the language in which an attribute is (not) supported.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SEMA_ATTRIBUTE_LANG_SUPPORT_H
+#define LLVM_CLANG_SEMA_ATTRIBUTE_LANG_SUPPORT_H
+
+
+// NOTE: The order should match the order of the %select in err_attribute_not_supported_in_lang
+// in DiagnosticSemaKinds.td
+namespace clang::AttributeLangSupport {
+enum LANG {
+ C,
+ Cpp,
+ HLSL,
+ ObjC,
+ SYCLDevice,
+};
+} // end namespace clang::AttributeLangSupport
+
+#endif // LLVM_CLANG_SEMA_ATTRIBUTE_LANG_SUPPORT_H
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f5f18b06c2fff..145d03e4324c3 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "AttributeLangSupport.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
@@ -73,14 +74,6 @@
using namespace clang;
using namespace sema;
-namespace AttributeLangSupport {
- enum LANG {
- C,
- Cpp,
- ObjC
- };
-} // end namespace AttributeLangSupport
-
static unsigned getNumAttributeArgs(const ParsedAttr &AL) {
// FIXME: Include the type in the argument list.
return AL.getNumArgs() + AL.hasParsedType();
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 1289bede3f192..6f4d7a086d40c 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "AttributeLangSupport.h"
#include "TypeLocBuilder.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
@@ -6573,8 +6574,16 @@ static void HandleAddressSpaceTypeAttribute(QualType &Type,
if (S.getLangOpts().HLSL)
ASIdx = Attr.asHLSLLangAS();
- if (ASIdx == LangAS::Default)
- llvm_unreachable("Invalid address space");
+ if (ASIdx == LangAS::Default) {
+ assert(S.getLangOpts().SYCLIsDevice || S.getLangOpts().HLSL);
+ S.Diag(Attr.getLoc(), diag::err_attribute_not_supported_in_lang)
+ << Attr
+ << (S.getLangOpts().SYCLIsDevice ? AttributeLangSupport::SYCLDevice
+ : AttributeLangSupport::HLSL);
+ Attr.setInvalid();
+ return;
+ }
+
if (DiagnoseMultipleAddrSpaceAttributes(S, Type.getAddressSpace(), ASIdx,
Attr.getLoc())) {
diff --git a/clang/test/Sema/named_addrspace_lang.c b/clang/test/Sema/named_addrspace_lang.c
new file mode 100644
index 0000000000000..f5a343d8d051a
--- /dev/null
+++ b/clang/test/Sema/named_addrspace_lang.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -x c++ -fsycl-is-host -verify %s
+// RUN: %clang_cc1 -fsyntax-only -x c++ -fsycl-is-device -verify=sycl %s
+// RUN: %clang_cc1 -fsyntax-only -x hlsl -triple dxil-pc-shadermodel6.3-library -verify=hlsl %s
+
+// hlsl-error@#constant{{'opencl_constant' attribute is not supported in HLSL}}
+// hlsl-error@#generic{{'opencl_generic' attribute is not supported in HLSL}}
+// hlsl-error@#global{{'opencl_global' attribute is not supported in HLSL}}
+// hlsl-error@#global_host{{'opencl_global_host' attribute is not supported in HLSL}}
+// hlsl-error@#global_device{{'opencl_global_device' attribute is not supported in HLSL}}
+// hlsl-error@#local{{'opencl_local' attribute is not supported in HLSL}}
+// hlsl-error@#private{{'opencl_private' attribute is not supported in HLSL}}
+
+// sycl-error@#constant{{'opencl_constant' attribute is not supported in SYCL when compiling for the device}}
+// sycl-error@#generic{{'opencl_generic' attribute is not supported in SYCL when compiling for the device}}
+
+int __attribute__((opencl_constant)) glob = 1; // #constant
+int __attribute__((opencl_generic)) gen; // #generic
+int __attribute__((opencl_global)) c; // #global
+int __attribute__((opencl_global_host)) h; // #global_host
+int __attribute__((opencl_global_device)) d; // #global_device
+int __attribute__((opencl_local)) l; // #local
+int __attribute__((opencl_private)) p; // #private
+
+// expected-no-diagnostics
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
bader
reviewed
Aug 7, 2025
There was a problem hiding this comment.
This change make sense to me, but I'd like either @Fznamznon or @tahonermann to take a look as well.
My understanding is that the opencl_generic attribute in OpenCL has the semantics of the Default address space in SYCL offload mode (except for interaction with the constant address space).
Using the opencl_generic attribute in SYCL offload mode seems useless.
Fznamznon
reviewed
Aug 8, 2025
| // hlsl-error@#local{{'opencl_local' attribute is not supported in HLSL}} | ||
| // hlsl-error@#private{{'opencl_private' attribute is not supported in HLSL}} | ||
|
|
||
| // sycl-error@#constant{{'opencl_constant' attribute is not supported in SYCL when compiling for the device}} |
There was a problem hiding this comment.
I wonder, why only SYCL device? IMO address space attributes don't make sense for host compilation at all.
There was a problem hiding this comment.
Because I just kept the current behavior, turning llvm_unreachable into a the "attribute not supported" error.
Mechanically it's only on device because of this piece of code SemaType.cpp:6572, which was added by @bader in D89909, he might remember still why it's only device :).
There was a problem hiding this comment.
TBH I'm not entirely sure why __attribute__((opencl_*)) is supported for C++/Obj-C/CUDA etc (why isn't this guarded by getLangOpts().Opencl || getLangOpts().OpenCLCPlusPlus) either, but I also didn't want to change that.
Co-authored-by: Alexey Bader <[email protected]>
Co-authored-by: Alexey Bader <[email protected]>
|
@farzonl @llvm-beanz, can you check this PR for HLSL? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Raise an error instead of executing unreachable when an unsupported named address space attribute is used in SYCL or HLSL.
Arguably some of these could be accepted, but in the interest of simplicity reject them for now instead of crashing.
Fixes: #152460