[clang] c50ef30 - [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (#96346)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 28 01:53:57 PDT 2024
Author: Joshua Batista
Date: 2024-06-28T01:53:54-07:00
New Revision: c50ef30cce32b8e864b90a4ca27c68882d46a19c
URL: https://github.com/llvm/llvm-project/commit/c50ef30cce32b8e864b90a4ca27c68882d46a19c
DIFF: https://github.com/llvm/llvm-project/commit/c50ef30cce32b8e864b90a4ca27c68882d46a19c.diff
LOG: [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (#96346)
`MaybeParseHLSLAnnotations` should be run on Field Decls instead of just
assuming that any colon after a field decl is a bitfield. In the case
that HLSL is the language, the code after the colon may be an
annotation. This PR gives the parser a chance to parse the subsequent
text as if it was an HLSL annotation.
The burden of parsing is now on the HLSL parser, but the actual work
needs to be done in handling every case of an hlsl annotation on a field
decl. SV_DispatchThreadID was straightforward enough to implement in
this PR, and tests have been added that the annotation appears as an
attribute in the AST.
Previously, the `hlsl_annotations_on_struct_members.hlsl` 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 parsed when parsing struct
decls.
This test not only 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, but also
validates that the parser does parse the annotation and adds an
attribute to the field decl in the AST.
Fixes https://github.com/llvm/llvm-project/issues/57889
Added:
clang/test/ParserHLSL/bitfields.hlsl
clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
Modified:
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseHLSL.cpp
clang/lib/Sema/SemaHLSL.cpp
clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
clang/test/SemaHLSL/resource_binding_attr_error.hlsl
Removed:
################################################################################
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 39b17f7f9bd7c..6880fa4bb0b03 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);
}
}
@@ -3029,12 +3030,13 @@ class Parser : public CodeCompletionHandler {
void MaybeParseHLSLAnnotations(ParsedAttributes &Attrs,
SourceLocation *EndLoc = nullptr) {
assert(getLangOpts().HLSL && "MaybeParseHLSLAnnotations is for HLSL only");
- if (getLangOpts().HLSL && Tok.is(tok::colon))
+ if (Tok.is(tok::colon))
ParseHLSLAnnotations(Attrs, EndLoc);
}
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 5e3ee5f0579aa..2fc43a7e7926b 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2681,6 +2681,10 @@ bool Parser::ParseCXXMemberDeclaratorBeforeInitializer(
else
DeclaratorInfo.SetIdentifier(nullptr, Tok.getLocation());
+ if (getLangOpts().HLSL)
+ MaybeParseHLSLAnnotations(DeclaratorInfo, nullptr,
+ /*CouldBeBitField*/ true);
+
if (!DeclaratorInfo.isFunctionDeclarator() && TryConsumeToken(tok::colon)) {
assert(DeclaratorInfo.isPastIdentifier() &&
"don't know where identifier would go yet?");
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/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index c39a24043d664..eebe17a5b4bf7 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -356,15 +356,7 @@ static bool isLegalTypeForHLSLSV_DispatchThreadID(QualType T) {
return true;
}
-void SemaHLSL::handleSV_DispatchThreadIDAttr(Decl *D, const ParsedAttr &AL) {
- // FIXME: support semantic on field.
- // See https://github.com/llvm/llvm-project/issues/57889.
- if (isa<FieldDecl>(D)) {
- Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node)
- << AL << "parameter";
- return;
- }
-
+void SemaHLSL::handleSV_DispatchThreadIDAttr(Decl *D, const ParsedAttr &AL) {
auto *VD = cast<ValueDecl>(D);
if (!isLegalTypeForHLSLSV_DispatchThreadID(VD->getType())) {
Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_type)
diff --git a/clang/test/ParserHLSL/bitfields.hlsl b/clang/test/ParserHLSL/bitfields.hlsl
new file mode 100644
index 0000000000000..307d1143a068e
--- /dev/null
+++ b/clang/test/ParserHLSL/bitfields.hlsl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -ast-dump -x hlsl -o - %s | FileCheck %s
+
+
+struct MyBitFields {
+ // CHECK: FieldDecl 0x{{[0-9a-f]+}} <line:9:3, col:25> col:16 referenced field1 'unsigned int'
+ // CHECK:-ConstantExpr 0x{{[0-9a-f]+}} <col:25> 'int'
+ // CHECK:-value: Int 3
+ // CHECK:-IntegerLiteral 0x{{[0-9a-f]+}} <col:25> 'int' 3
+ unsigned int field1 : 3; // 3 bits for field1
+
+ // CHECK:FieldDecl 0x{{[0-9a-f]+}} <line:15:3, col:25> col:16 referenced field2 'unsigned int'
+ // CHECK:-ConstantExpr 0x{{[0-9a-f]+}} <col:25> 'int'
+ // CHECK:-value: Int 4
+ // CHECK:-IntegerLiteral 0x{{[0-9a-f]+}} <col:25> 'int' 4
+ unsigned int field2 : 4; // 4 bits for field2
+
+ // CHECK:FieldDecl 0x{{[0-9a-f]+}} <line:21:3, col:16> col:7 field3 'int'
+ // CHECK:-ConstantExpr 0x{{[0-9a-f]+}} <col:16> 'int'
+ // CHECK:-value: Int 5
+ // CHECK:-IntegerLiteral 0x{{[0-9a-f]+}} <col:16> 'int' 5
+ 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
new file mode 100644
index 0000000000000..2eebc920388b5
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_annotations_on_struct_members.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// tests that hlsl annotations are properly parsed when applied on field decls,
+// and that the annotation gets properly placed on the AST.
+
+struct Eg9{
+ // CHECK: CXXRecordDecl 0x{{[0-9a-f]+}} <col:1, col:8> col:8 implicit struct Eg9
+ // CHECK: FieldDecl 0x{{[0-9a-f]+}} <line:10:3, col:16> col:16 referenced a 'unsigned int'
+ // CHECK: -HLSLSV_DispatchThreadIDAttr 0x{{[0-9a-f]+}} <col:20>
+ unsigned int a : SV_DispatchThreadID;
+};
+Eg9 e9;
+
+
+RWBuffer<int> In : register(u1);
+
+
+[numthreads(1,1,1)]
+void main() {
+ In[0] = e9.a;
+}
diff --git a/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl b/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
index bf8027d029882..bc3cf8bc51daf 100644
--- a/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
+++ b/clang/test/SemaHLSL/Semantics/invalid_entry_parameter.hlsl
@@ -23,8 +23,7 @@ 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'}}
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