[clang] [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (PR #96346)
Joshua Batista via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 21 13:36:53 PDT 2024
https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/96346
>From c267be670adf7aac050484dc1b243aa0eff60b5f Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 21 Jun 2024 11:25:22 -0700
Subject: [PATCH 1/3] parse hlsl annotations on struct, add test
---
clang/lib/Parse/ParseDeclCXX.cpp | 3 ++
.../hlsl_annotations_on_struct_members.hlsl | 30 +++++++++++++++++++
2 files changed, 33 insertions(+)
create mode 100644 clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index d02548f6441f9..c4a4657cbd04f 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2646,6 +2646,9 @@ bool Parser::ParseCXXMemberDeclaratorBeforeInitializer(
else
DeclaratorInfo.SetIdentifier(nullptr, Tok.getLocation());
+ if (getLangOpts().HLSL)
+ MaybeParseHLSLAnnotations(DeclaratorInfo);
+
if (!DeclaratorInfo.isFunctionDeclarator() && TryConsumeToken(tok::colon)) {
assert(DeclaratorInfo.isPastIdentifier() &&
"don't know where identifier would go yet?");
diff --git a/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl b/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
new file mode 100644
index 0000000000000..f1e258b0d853c
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify
+
+// previously, this test would result in an error shown below on the line that
+// declares variable a in struct Eg9:
+// error: use of undeclared identifier
+// 'SV_DispatchThreadID'
+// This is because the annotation is parsed as if it was a c++ bit field, and an identifier
+// that represents an integer is expected, but not found.
+
+// This test ensures that hlsl annotations are attempted to be parsed when parsing struct decls.
+// Ideally, we'd validate this behavior by ensuring the annotation is parsed and properly
+// attached as an attribute to the member in the struct in the AST. However, in this case
+// this can't happen presently because there are other issues with annotations on field decls.
+// This test just ensures we make progress by moving the validation error from the realm of
+// C++ and expecting bitfields, to HLSL and a specialized error for the recognized annotation.
+
+struct Eg9{
+// expected-error at +1{{attribute 'SV_DispatchThreadID' only applies to parameter}}
+ int a : SV_DispatchThreadID;
+};
+Eg9 e9;
+
+
+RWBuffer<int> In : register(u1);
+
+
+[numthreads(1,1,1)]
+void main() {
+ In[0] = e9.a;
+}
>From 5c98dd990b938258bd94057220c61e85c1333e85 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 21 Jun 2024 12:37:21 -0700
Subject: [PATCH 2/3] unconsume colon if bitfield detected, add bitfield test
---
clang/include/clang/Parse/Parser.h | 8 +++++---
clang/lib/Parse/ParseDeclCXX.cpp | 3 ++-
clang/lib/Parse/ParseHLSL.cpp | 8 +++++++-
clang/test/ParserHLSL/bitfields.hlsl | 18 ++++++++++++++++++
.../hlsl_annotations_on_struct_members.hlsl | 14 +-------------
5 files changed, 33 insertions(+), 18 deletions(-)
create mode 100644 clang/test/ParserHLSL/bitfields.hlsl
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 95c0655f9a214..dc154c93a3b44 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3017,11 +3017,12 @@ class Parser : public CodeCompletionHandler {
const IdentifierInfo *EnclosingScope = nullptr);
void MaybeParseHLSLAnnotations(Declarator &D,
- SourceLocation *EndLoc = nullptr) {
+ SourceLocation *EndLoc = nullptr,
+ bool CouldBeBitField = false) {
assert(getLangOpts().HLSL && "MaybeParseHLSLAnnotations is for HLSL only");
if (Tok.is(tok::colon)) {
ParsedAttributes Attrs(AttrFactory);
- ParseHLSLAnnotations(Attrs, EndLoc);
+ ParseHLSLAnnotations(Attrs, EndLoc, CouldBeBitField);
D.takeAttributes(Attrs);
}
}
@@ -3034,7 +3035,8 @@ class Parser : public CodeCompletionHandler {
}
void ParseHLSLAnnotations(ParsedAttributes &Attrs,
- SourceLocation *EndLoc = nullptr);
+ SourceLocation *EndLoc = nullptr,
+ bool CouldBeBitField = false);
Decl *ParseHLSLBuffer(SourceLocation &DeclEnd);
void MaybeParseMicrosoftAttributes(ParsedAttributes &Attrs) {
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index c4a4657cbd04f..461dbb2743c76 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2647,7 +2647,8 @@ bool Parser::ParseCXXMemberDeclaratorBeforeInitializer(
DeclaratorInfo.SetIdentifier(nullptr, Tok.getLocation());
if (getLangOpts().HLSL)
- MaybeParseHLSLAnnotations(DeclaratorInfo);
+ MaybeParseHLSLAnnotations(DeclaratorInfo, nullptr,
+ /*CouldBeBitField*/ true);
if (!DeclaratorInfo.isFunctionDeclarator() && TryConsumeToken(tok::colon)) {
assert(DeclaratorInfo.isPastIdentifier() &&
diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp
index 4b72afe9986e5..b36ea4012c26e 100644
--- a/clang/lib/Parse/ParseHLSL.cpp
+++ b/clang/lib/Parse/ParseHLSL.cpp
@@ -119,9 +119,11 @@ static void fixSeparateAttrArgAndNumber(StringRef ArgStr, SourceLocation ArgLoc,
}
void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs,
- SourceLocation *EndLoc) {
+ SourceLocation *EndLoc,
+ bool CouldBeBitField) {
assert(Tok.is(tok::colon) && "Not a HLSL Annotation");
+ Token OldToken = Tok;
ConsumeToken();
IdentifierInfo *II = nullptr;
@@ -131,6 +133,10 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs,
II = Tok.getIdentifierInfo();
if (!II) {
+ if (CouldBeBitField) {
+ UnconsumeToken(OldToken);
+ return;
+ }
Diag(Tok.getLocation(), diag::err_expected_semantic_identifier);
return;
}
diff --git a/clang/test/ParserHLSL/bitfields.hlsl b/clang/test/ParserHLSL/bitfields.hlsl
new file mode 100644
index 0000000000000..abb3109c1d72a
--- /dev/null
+++ b/clang/test/ParserHLSL/bitfields.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify
+
+// expected-no-diagnostics
+
+struct MyBitFields {
+ unsigned int field1 : 3; // 3 bits for field1
+ unsigned int field2 : 4; // 4 bits for field2
+ int field3 : 5; // 5 bits for field3 (signed)
+};
+
+
+
+[numthreads(1,1,1)]
+void main() {
+ MyBitFields m;
+ m.field1 = 4;
+ m.field2 = m.field1*2;
+}
\ No newline at end of file
diff --git a/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl b/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
index f1e258b0d853c..6132ffa0ad476 100644
--- a/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
+++ b/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
@@ -1,18 +1,6 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify
-// previously, this test would result in an error shown below on the line that
-// declares variable a in struct Eg9:
-// error: use of undeclared identifier
-// 'SV_DispatchThreadID'
-// This is because the annotation is parsed as if it was a c++ bit field, and an identifier
-// that represents an integer is expected, but not found.
-
-// This test ensures that hlsl annotations are attempted to be parsed when parsing struct decls.
-// Ideally, we'd validate this behavior by ensuring the annotation is parsed and properly
-// attached as an attribute to the member in the struct in the AST. However, in this case
-// this can't happen presently because there are other issues with annotations on field decls.
-// This test just ensures we make progress by moving the validation error from the realm of
-// C++ and expecting bitfields, to HLSL and a specialized error for the recognized annotation.
+// TODO: update once we handle annotations on struct fields
struct Eg9{
// expected-error at +1{{attribute 'SV_DispatchThreadID' only applies to parameter}}
>From 4b6e23eb6e558340744ba8b18011d1ce839741bb Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 21 Jun 2024 13:35:20 -0700
Subject: [PATCH 3/3] update existing tests
---
clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl | 4 ++--
clang/test/SemaHLSL/resource_binding_attr_error.hlsl | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl b/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
index bf8027d029882..8752e97b53f61 100644
--- a/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
+++ b/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
@@ -23,8 +23,8 @@ void foo() {
}
struct ST2 {
-// expected-error at +1 {{use of undeclared identifier 'SV_DispatchThreadID'}}
+// expected-warning at +1 {{'SV_DispatchThreadID' attribute only applies to parameters and non-static data members}}
static uint X : SV_DispatchThreadID;
-// expected-error at +1 {{use of undeclared identifier 'SV_DispatchThreadID'}}
+// expected-error at +1 {{attribute 'SV_DispatchThreadID' only applies to parameter}}
uint s : SV_DispatchThreadID;
};
diff --git a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
index 95774472aaea0..2f8aa098db701 100644
--- a/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
+++ b/clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -55,6 +55,6 @@ void bar(RWBuffer<float> U : register(u3)) {
struct S {
// FIXME: generate better error when support semantic on struct field.
// See https://github.com/llvm/llvm-project/issues/57889.
- // expected-error at +1 {{expected expression}}
+ // expected-warning at +1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
RWBuffer<float> U : register(u3);
};
More information about the cfe-commits
mailing list