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 <jbati...@microsoft.com> Date: Fri, 21 Jun 2024 11:25:22 -0700 Subject: [PATCH 1/4] 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@+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 <jbati...@microsoft.com> Date: Fri, 21 Jun 2024 12:37:21 -0700 Subject: [PATCH 2/4] 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@+1{{attribute 'SV_DispatchThreadID' only applies to parameter}} >From 4b6e23eb6e558340744ba8b18011d1ce839741bb Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Fri, 21 Jun 2024 13:35:20 -0700 Subject: [PATCH 3/4] 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@+1 {{use of undeclared identifier 'SV_DispatchThreadID'}} +// expected-warning@+1 {{'SV_DispatchThreadID' attribute only applies to parameters and non-static data members}} static uint X : SV_DispatchThreadID; -// expected-error@+1 {{use of undeclared identifier 'SV_DispatchThreadID'}} +// expected-error@+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@+1 {{expected expression}} + // expected-warning@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}} RWBuffer<float> U : register(u3); }; >From 34d4b4d05159ff61f9c535730c98d8dc1f5e8aec Mon Sep 17 00:00:00 2001 From: Joshua Batista <jbati...@microsoft.com> Date: Fri, 21 Jun 2024 13:44:46 -0700 Subject: [PATCH 4/4] remove redundant HLSL mode check --- clang/include/clang/Parse/Parser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index dc154c93a3b44..6cb223a14014e 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3030,7 +3030,7 @@ 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); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits