https://github.com/python3kgae updated https://github.com/llvm/llvm-project/pull/89836
>From 4d8c72688656fe3b2ce8817087d8cf7352b5876b Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Tue, 23 Apr 2024 17:49:02 -0400 Subject: [PATCH 01/10] [HLSL] Support packoffset attribute in AST Add HLSLPackOffsetAttr to save packoffset in AST. Since we have to parse the attribute manually in ParseHLSLAnnotations, we could create the ParsedAttribute with a integer offset parameter instead of string. This approach avoids parsing the string if the offset is saved as a string in HLSLPackOffsetAttr. --- clang/include/clang/Basic/Attr.td | 7 ++ clang/include/clang/Basic/AttrDocs.td | 18 +++++ .../clang/Basic/DiagnosticParseKinds.td | 2 + .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/lib/Parse/ParseHLSL.cpp | 80 +++++++++++++++++++ clang/lib/Sema/SemaDeclAttr.cpp | 44 ++++++++++ clang/lib/Sema/SemaHLSL.cpp | 48 +++++++++++ clang/test/AST/HLSL/packoffset.hlsl | 16 ++++ clang/test/SemaHLSL/packoffset-invalid.hlsl | 55 +++++++++++++ 9 files changed, 273 insertions(+) create mode 100644 clang/test/AST/HLSL/packoffset.hlsl create mode 100644 clang/test/SemaHLSL/packoffset-invalid.hlsl diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 4408d517e70e..d3d006ed9633 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4372,6 +4372,13 @@ def HLSLResourceBinding: InheritableAttr { let Documentation = [HLSLResourceBindingDocs]; } +def HLSLPackOffset: HLSLAnnotationAttr { + let Spellings = [HLSLAnnotation<"packoffset">]; + let LangOpts = [HLSL]; + let Args = [IntArgument<"Offset">]; + let Documentation = [HLSLPackOffsetDocs]; +} + def HLSLSV_DispatchThreadID: HLSLAnnotationAttr { let Spellings = [HLSLAnnotation<"SV_DispatchThreadID">]; let Subjects = SubjectList<[ParmVar, Field]>; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index a0bbe5861c57..6bc7813bd43c 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7398,6 +7398,24 @@ The full documentation is available here: https://docs.microsoft.com/en-us/windo }]; } +def HLSLPackOffsetDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ +The packoffset attribute is used to change the layout of a cbuffer. +Attribute spelling in HLSL is: ``packoffset(c[Subcomponent[.component]])``. +A subcomponent is a register number, which is an integer. A component is in the form of [.xyzw]. + +Here're packoffset examples with and without component: +.. code-block:: c++ + cbuffer A { + float3 a : packoffset(c0.y); + float4 b : packoffset(c4); + } + +The full documentation is available here: https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-variable-packoffset + }]; +} + def HLSLSV_DispatchThreadIDDocs : Documentation { let Category = DocCatFunction; let Content = [{ diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 38174cf3549f..81433ee79d48 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1745,5 +1745,7 @@ def err_hlsl_separate_attr_arg_and_number : Error<"wrong argument format for hls def ext_hlsl_access_specifiers : ExtWarn< "access specifiers are a clang HLSL extension">, InGroup<HLSLExtension>; +def err_hlsl_unsupported_component : Error<"invalid component '%0' used; expected 'x', 'y', 'z', or 'w'">; +def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%0' for packoffset, expected 'c'">; } // end of Parser diagnostics diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 63e951daec74..bde9617c9820 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12167,6 +12167,9 @@ def err_hlsl_init_priority_unsupported : Error< def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">; def err_hlsl_unsupported_register_number : Error<"register number should be an integer">; def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">; +def err_hlsl_packoffset_mix : Error<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">; +def err_hlsl_packoffset_overlap : Error<"packoffset overlap between %0, %1">; +def err_hlsl_packoffset_cross_reg_boundary : Error<"packoffset cannot cross register boundary">; def err_hlsl_pointers_unsupported : Error< "%select{pointers|references}0 are unsupported in HLSL">; diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp index f4cbece31f18..eac4876ccab4 100644 --- a/clang/lib/Parse/ParseHLSL.cpp +++ b/clang/lib/Parse/ParseHLSL.cpp @@ -183,6 +183,86 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, return; } } break; + case ParsedAttr::AT_HLSLPackOffset: { + // Parse 'packoffset( c[Subcomponent][.component] )'. + // Check '('. + if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen_after)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } + // Check c[Subcomponent] as an identifier. + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } + StringRef OffsetStr = Tok.getIdentifierInfo()->getName(); + SourceLocation OffsetLoc = Tok.getLocation(); + if (OffsetStr[0] != 'c') { + Diag(Tok.getLocation(), diag::err_hlsl_packoffset_invalid_reg) + << OffsetStr; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } + OffsetStr = OffsetStr.substr(1); + unsigned SubComponent = 0; + if (!OffsetStr.empty()) { + // Make sure SubComponent is a number. + if (OffsetStr.getAsInteger(10, SubComponent)) { + Diag(OffsetLoc.getLocWithOffset(1), + diag::err_hlsl_unsupported_register_number); + return; + } + } + unsigned Component = 0; + ConsumeToken(); // consume identifier. + if (Tok.is(tok::period)) { + ConsumeToken(); // consume period. + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } + StringRef ComponentStr = Tok.getIdentifierInfo()->getName(); + SourceLocation SpaceLoc = Tok.getLocation(); + ConsumeToken(); // consume identifier. + // Make sure Component is a single character. + if (ComponentStr.size() != 1) { + Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } else { + switch (ComponentStr[0]) { + case 'x': + Component = 0; + break; + case 'y': + Component = 1; + break; + case 'z': + Component = 2; + break; + case 'w': + Component = 3; + break; + default: + Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } + } + } + unsigned Offset = SubComponent * 4 + Component; + ASTContext &Ctx = Actions.getASTContext(); + ArgExprs.push_back(IntegerLiteral::Create( + Ctx, llvm::APInt(Ctx.getTypeSize(Ctx.getSizeType()), Offset), + Ctx.getSizeType(), + SourceLocation())); // Dummy location for integer literal. + if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; + } + } break; case ParsedAttr::UnknownAttribute: Diag(Loc, diag::err_unknown_hlsl_semantic) << II; return; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 363ae93cb62d..373f2e8f50cd 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7314,6 +7314,47 @@ static void handleHLSLSV_DispatchThreadIDAttr(Sema &S, Decl *D, D->addAttr(::new (S.Context) HLSLSV_DispatchThreadIDAttr(S.Context, AL)); } +static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + if (!isa<VarDecl>(D)) { + S.Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node) + << AL << "cbuffer constant"; + return; + } + auto *BufDecl = dyn_cast<HLSLBufferDecl>(D->getDeclContext()); + if (!BufDecl) { + S.Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node) + << AL << "cbuffer constant"; + return; + } + + uint32_t Offset; + if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Offset)) + return; + + QualType T = cast<VarDecl>(D)->getType().getCanonicalType(); + // Check if T is an array or struct type. + // TODO: mark matrix type as aggregate type. + bool IsAggregateTy = (T->isArrayType() || T->isStructureType()); + + unsigned ComponentNum = Offset & 0x3; + // Check ComponentNum is valid for T. + if (IsAggregateTy) { + if (ComponentNum != 0) { + S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); + return; + } + } else { + unsigned size = S.getASTContext().getTypeSize(T); + // Make sure ComponentNum + sizeof(T) <= 4. + if ((ComponentNum * 32 + size) > 128) { + S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); + return; + } + } + + D->addAttr(::new (S.Context) HLSLPackOffsetAttr(S.Context, AL, Offset)); +} + static void handleHLSLShaderAttr(Sema &S, Decl *D, const ParsedAttr &AL) { StringRef Str; SourceLocation ArgLoc; @@ -9735,6 +9776,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_HLSLSV_DispatchThreadID: handleHLSLSV_DispatchThreadIDAttr(S, D, AL); break; + case ParsedAttr::AT_HLSLPackOffset: + handleHLSLPackOffsetAttr(S, D, AL); + break; case ParsedAttr::AT_HLSLShader: handleHLSLShaderAttr(S, D, AL); break; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index bb9e37f18d37..fa62cab54e69 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -42,6 +42,54 @@ Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool CBuffer, void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) { auto *BufDecl = cast<HLSLBufferDecl>(Dcl); BufDecl->setRBraceLoc(RBrace); + + // Validate packoffset. + llvm::SmallVector<std::pair<VarDecl *, HLSLPackOffsetAttr *>> PackOffsetVec; + bool HasPackOffset = false; + bool HasNonPackOffset = false; + for (auto *Field : BufDecl->decls()) { + VarDecl *Var = dyn_cast<VarDecl>(Field); + if (!Var) + continue; + if (Field->hasAttr<HLSLPackOffsetAttr>()) { + PackOffsetVec.emplace_back(Var, Field->getAttr<HLSLPackOffsetAttr>()); + HasPackOffset = true; + } else { + HasNonPackOffset = true; + } + } + + if (HasPackOffset && HasNonPackOffset) { + Diag(BufDecl->getLocation(), diag::err_hlsl_packoffset_mix); + } else if (HasPackOffset) { + ASTContext &Context = getASTContext(); + // Make sure no overlap in packoffset. + llvm::SmallDenseMap<VarDecl *, std::pair<unsigned, unsigned>> + PackOffsetRanges; + for (auto &Pair : PackOffsetVec) { + VarDecl *Var = Pair.first; + HLSLPackOffsetAttr *Attr = Pair.second; + unsigned Size = Context.getTypeSize(Var->getType()); + unsigned Begin = Attr->getOffset() * 32; + unsigned End = Begin + Size; + for (auto &Range : PackOffsetRanges) { + VarDecl *OtherVar = Range.first; + unsigned OtherBegin = Range.second.first; + unsigned OtherEnd = Range.second.second; + if (Begin < OtherEnd && OtherBegin < Begin) { + Diag(Var->getLocation(), diag::err_hlsl_packoffset_overlap) + << Var << OtherVar; + break; + } else if (OtherBegin < End && Begin < OtherBegin) { + Diag(Var->getLocation(), diag::err_hlsl_packoffset_overlap) + << Var << OtherVar; + break; + } + } + PackOffsetRanges[Var] = std::make_pair(Begin, End); + } + } + SemaRef.PopDeclContext(); } diff --git a/clang/test/AST/HLSL/packoffset.hlsl b/clang/test/AST/HLSL/packoffset.hlsl new file mode 100644 index 000000000000..d3cf798c9957 --- /dev/null +++ b/clang/test/AST/HLSL/packoffset.hlsl @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.3-library -S -finclude-default-header -ast-dump -x hlsl %s | FileCheck %s + + +// CHECK: HLSLBufferDecl {{.*}} cbuffer A +cbuffer A +{ + // CHECK-NEXT: VarDecl {{.*}} C1 'float4' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 + float4 C1 : packoffset(c); + // CHECK-NEXT: VarDecl {{.*}} col:11 C2 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 4 + float C2 : packoffset(c1); + // CHECK-NEXT: VarDecl {{.*}} col:11 C3 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 5 + float C3 : packoffset(c1.y); +} diff --git a/clang/test/SemaHLSL/packoffset-invalid.hlsl b/clang/test/SemaHLSL/packoffset-invalid.hlsl new file mode 100644 index 000000000000..a2c7bb5a1e05 --- /dev/null +++ b/clang/test/SemaHLSL/packoffset-invalid.hlsl @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.3-library -verify %s + +// expected-error@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} +cbuffer Mix +{ + float4 M1 : packoffset(c0); + float M2; + float M3 : packoffset(c1.y); +} + +// expected-error@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} +cbuffer Mix2 +{ + float4 M4; + float M5 : packoffset(c1.y); + float M6 ; +} + +// expected-error@+1{{attribute 'packoffset' only applies to cbuffer constant}} +float4 g : packoffset(c0); + +cbuffer IllegalOffset +{ + // expected-error@+1{{invalid resource class specifier 't2' for packoffset, expected 'c'}} + float4 i1 : packoffset(t2); + // expected-error@+1{{invalid component 'm' used; expected 'x', 'y', 'z', or 'w'}} + float i2 : packoffset(c1.m); +} + +cbuffer Overlap +{ + float4 o1 : packoffset(c0); + // expected-error@+1{{packoffset overlap between 'o2', 'o1'}} + float2 o2 : packoffset(c0.z); +} + +cbuffer CrossReg +{ + // expected-error@+1{{packoffset cannot cross register boundary}} + float4 c1 : packoffset(c0.y); + // expected-error@+1{{packoffset cannot cross register boundary}} + float2 c2 : packoffset(c1.w); +} + +struct ST { + float s; +}; + +cbuffer Aggregate +{ + // expected-error@+1{{packoffset cannot cross register boundary}} + ST A1 : packoffset(c0.y); + // expected-error@+1{{packoffset cannot cross register boundary}} + float A2[2] : packoffset(c1.w); +} >From c356db64f307bd042d40cbade7f09824c7bf238c Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Wed, 24 Apr 2024 09:37:45 -0400 Subject: [PATCH 02/10] Fix doc format. --- clang/include/clang/Basic/AttrDocs.td | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 6bc7813bd43c..20e58897f20b 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7406,7 +7406,9 @@ Attribute spelling in HLSL is: ``packoffset(c[Subcomponent[.component]])``. A subcomponent is a register number, which is an integer. A component is in the form of [.xyzw]. Here're packoffset examples with and without component: + .. code-block:: c++ + cbuffer A { float3 a : packoffset(c0.y); float4 b : packoffset(c4); >From 2e406f5e644dcdd04259551079953e6339060330 Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Thu, 25 Apr 2024 13:58:18 -0400 Subject: [PATCH 03/10] Update per comment. --- clang/include/clang/Basic/AttrDocs.td | 2 +- clang/lib/Parse/ParseHLSL.cpp | 38 +++++++++++++-------------- clang/lib/Sema/SemaDeclAttr.cpp | 8 +----- clang/lib/Sema/SemaHLSL.cpp | 34 +++++++++++------------- 4 files changed, 36 insertions(+), 46 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 20e58897f20b..ac08799faa91 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7402,7 +7402,7 @@ def HLSLPackOffsetDocs : Documentation { let Category = DocCatFunction; let Content = [{ The packoffset attribute is used to change the layout of a cbuffer. -Attribute spelling in HLSL is: ``packoffset(c[Subcomponent[.component]])``. +Attribute spelling in HLSL is: ``packoffset( c[Subcomponent][.component] )``. A subcomponent is a register number, which is an integer. A component is in the form of [.xyzw]. Here're packoffset examples with and without component: diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp index eac4876ccab4..ae9ceaa700af 100644 --- a/clang/lib/Parse/ParseHLSL.cpp +++ b/clang/lib/Parse/ParseHLSL.cpp @@ -211,6 +211,7 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, if (OffsetStr.getAsInteger(10, SubComponent)) { Diag(OffsetLoc.getLocWithOffset(1), diag::err_hlsl_unsupported_register_number); + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; } } @@ -231,25 +232,24 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; - } else { - switch (ComponentStr[0]) { - case 'x': - Component = 0; - break; - case 'y': - Component = 1; - break; - case 'z': - Component = 2; - break; - case 'w': - Component = 3; - break; - default: - Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return; - } + } + switch (ComponentStr[0]) { + case 'x': + Component = 0; + break; + case 'y': + Component = 1; + break; + case 'z': + Component = 2; + break; + case 'w': + Component = 3; + break; + default: + Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; } } unsigned Offset = SubComponent * 4 + Component; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 373f2e8f50cd..cf7d690f6288 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7315,13 +7315,7 @@ static void handleHLSLSV_DispatchThreadIDAttr(Sema &S, Decl *D, } static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (!isa<VarDecl>(D)) { - S.Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node) - << AL << "cbuffer constant"; - return; - } - auto *BufDecl = dyn_cast<HLSLBufferDecl>(D->getDeclContext()); - if (!BufDecl) { + if (!isa<VarDecl>(D) || !isa<HLSLBufferDecl>(D->getDeclContext())) { S.Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node) << AL << "cbuffer constant"; return; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index fa62cab54e69..b2eb69dd5a14 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -64,29 +64,25 @@ void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) { } else if (HasPackOffset) { ASTContext &Context = getASTContext(); // Make sure no overlap in packoffset. - llvm::SmallDenseMap<VarDecl *, std::pair<unsigned, unsigned>> - PackOffsetRanges; - for (auto &Pair : PackOffsetVec) { - VarDecl *Var = Pair.first; - HLSLPackOffsetAttr *Attr = Pair.second; + // Sort PackOffsetVec by offset. + std::sort(PackOffsetVec.begin(), PackOffsetVec.end(), + [](const std::pair<VarDecl *, HLSLPackOffsetAttr *> &LHS, + const std::pair<VarDecl *, HLSLPackOffsetAttr *> &RHS) { + return LHS.second->getOffset() < RHS.second->getOffset(); + }); + + for (unsigned i = 0; i < PackOffsetVec.size() - 1; i++) { + VarDecl *Var = PackOffsetVec[i].first; + HLSLPackOffsetAttr *Attr = PackOffsetVec[i].second; unsigned Size = Context.getTypeSize(Var->getType()); unsigned Begin = Attr->getOffset() * 32; unsigned End = Begin + Size; - for (auto &Range : PackOffsetRanges) { - VarDecl *OtherVar = Range.first; - unsigned OtherBegin = Range.second.first; - unsigned OtherEnd = Range.second.second; - if (Begin < OtherEnd && OtherBegin < Begin) { - Diag(Var->getLocation(), diag::err_hlsl_packoffset_overlap) - << Var << OtherVar; - break; - } else if (OtherBegin < End && Begin < OtherBegin) { - Diag(Var->getLocation(), diag::err_hlsl_packoffset_overlap) - << Var << OtherVar; - break; - } + unsigned NextBegin = PackOffsetVec[i + 1].second->getOffset() * 32; + if (End > NextBegin) { + VarDecl *NextVar = PackOffsetVec[i + 1].first; + Diag(NextVar->getLocation(), diag::err_hlsl_packoffset_overlap) + << NextVar << Var; } - PackOffsetRanges[Var] = std::make_pair(Begin, End); } } >From 228aca7a682256471690de48bf63b59324ec8ceb Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Wed, 1 May 2024 12:28:38 -0400 Subject: [PATCH 04/10] Check double alignment. Support rgba as component. Add calculateLegacyCbufferSize to get correct size when check overlap. --- .../clang/Basic/DiagnosticSemaKinds.td | 1 + clang/lib/Parse/ParseHLSL.cpp | 7 +- clang/lib/Sema/SemaDeclAttr.cpp | 29 ++++-- clang/lib/Sema/SemaHLSL.cpp | 37 ++++++- clang/test/AST/HLSL/packoffset.hlsl | 96 +++++++++++++++++-- clang/test/SemaHLSL/packoffset-invalid.hlsl | 67 +++++++++++++ 6 files changed, 219 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index bde9617c9820..dadc91974251 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12170,6 +12170,7 @@ def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected def err_hlsl_packoffset_mix : Error<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">; def err_hlsl_packoffset_overlap : Error<"packoffset overlap between %0, %1">; def err_hlsl_packoffset_cross_reg_boundary : Error<"packoffset cannot cross register boundary">; +def err_hlsl_packoffset_alignment_mismatch : Error<"packoffset at 'y' not match alignment %0 required by %1">; def err_hlsl_pointers_unsupported : Error< "%select{pointers|references}0 are unsupported in HLSL">; diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp index ae9ceaa700af..ce3b2eef9d4e 100644 --- a/clang/lib/Parse/ParseHLSL.cpp +++ b/clang/lib/Parse/ParseHLSL.cpp @@ -235,15 +235,19 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, } switch (ComponentStr[0]) { case 'x': + case 'r': Component = 0; break; case 'y': + case 'g': Component = 1; break; case 'z': + case 'b': Component = 2; break; case 'w': + case 'a': Component = 3; break; default: @@ -256,8 +260,7 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, ASTContext &Ctx = Actions.getASTContext(); ArgExprs.push_back(IntegerLiteral::Create( Ctx, llvm::APInt(Ctx.getTypeSize(Ctx.getSizeType()), Offset), - Ctx.getSizeType(), - SourceLocation())); // Dummy location for integer literal. + Ctx.getSizeType(), OffsetLoc)); if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index cf7d690f6288..d643e5042d7f 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7332,17 +7332,28 @@ static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { unsigned ComponentNum = Offset & 0x3; // Check ComponentNum is valid for T. - if (IsAggregateTy) { - if (ComponentNum != 0) { - S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); - return; - } - } else { - unsigned size = S.getASTContext().getTypeSize(T); - // Make sure ComponentNum + sizeof(T) <= 4. - if ((ComponentNum * 32 + size) > 128) { + if (ComponentNum) { + unsigned Size = S.getASTContext().getTypeSize(T); + if (IsAggregateTy || Size > 128) { S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); return; + } else { + // Make sure ComponentNum + sizeof(T) <= 4. + if ((ComponentNum * 32 + Size) > 128) { + S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); + return; + } + QualType EltTy = T; + if (const auto *VT = T->getAs<VectorType>()) + EltTy = VT->getElementType(); + unsigned Align = S.getASTContext().getTypeAlign(EltTy); + if (Align > 32 && ComponentNum == 1) { + // NOTE: ComponentNum 3 will hit err_hlsl_packoffset_cross_reg_boundary. + // So we only need to check ComponentNum 1 here. + S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_alignment_mismatch) + << Align << EltTy; + return; + } } } diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index b2eb69dd5a14..84859bd74755 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -39,6 +39,41 @@ Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool CBuffer, return Result; } +// Calculate the size of a legacy cbuffer type based on +// https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-packing-rules +static unsigned calculateLegacyCbufferSize(const ASTContext &Context, + QualType T) { + unsigned Size = 0; + constexpr unsigned CBufferAlign = 128; + if (const RecordType *RT = T->getAs<RecordType>()) { + const RecordDecl *RD = RT->getDecl(); + for (const FieldDecl *Field : RD->fields()) { + QualType Ty = Field->getType(); + unsigned FieldSize = calculateLegacyCbufferSize(Context, Ty); + unsigned FieldAlign = 32; + if (Ty->isAggregateType()) + FieldAlign = CBufferAlign; + Size = llvm::alignTo(Size, FieldAlign); + Size += FieldSize; + } + } else if (const ConstantArrayType *AT = Context.getAsConstantArrayType(T)) { + if (unsigned ElementCount = AT->getSize().getZExtValue()) { + unsigned ElementSize = + calculateLegacyCbufferSize(Context, AT->getElementType()); + unsigned AlignedElementSize = llvm::alignTo(ElementSize, CBufferAlign); + Size = AlignedElementSize * (ElementCount - 1) + ElementSize; + } + } else if (const VectorType *VT = T->getAs<VectorType>()) { + unsigned ElementCount = VT->getNumElements(); + unsigned ElementSize = + calculateLegacyCbufferSize(Context, VT->getElementType()); + Size = ElementSize * ElementCount; + } else { + Size = Context.getTypeSize(T); + } + return Size; +} + void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) { auto *BufDecl = cast<HLSLBufferDecl>(Dcl); BufDecl->setRBraceLoc(RBrace); @@ -74,7 +109,7 @@ void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) { for (unsigned i = 0; i < PackOffsetVec.size() - 1; i++) { VarDecl *Var = PackOffsetVec[i].first; HLSLPackOffsetAttr *Attr = PackOffsetVec[i].second; - unsigned Size = Context.getTypeSize(Var->getType()); + unsigned Size = calculateLegacyCbufferSize(Context, Var->getType()); unsigned Begin = Attr->getOffset() * 32; unsigned End = Begin + Size; unsigned NextBegin = PackOffsetVec[i + 1].second->getOffset() * 32; diff --git a/clang/test/AST/HLSL/packoffset.hlsl b/clang/test/AST/HLSL/packoffset.hlsl index d3cf798c9957..d9f96661315d 100644 --- a/clang/test/AST/HLSL/packoffset.hlsl +++ b/clang/test/AST/HLSL/packoffset.hlsl @@ -4,13 +4,97 @@ // CHECK: HLSLBufferDecl {{.*}} cbuffer A cbuffer A { - // CHECK-NEXT: VarDecl {{.*}} C1 'float4' + // CHECK-NEXT: VarDecl {{.*}} A1 'float4' // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 - float4 C1 : packoffset(c); - // CHECK-NEXT: VarDecl {{.*}} col:11 C2 'float' + float4 A1 : packoffset(c); + // CHECK-NEXT: VarDecl {{.*}} col:11 A2 'float' // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 4 - float C2 : packoffset(c1); - // CHECK-NEXT: VarDecl {{.*}} col:11 C3 'float' + float A2 : packoffset(c1); + // CHECK-NEXT: VarDecl {{.*}} col:11 A3 'float' // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 5 - float C3 : packoffset(c1.y); + float A3 : packoffset(c1.y); +} + +// CHECK: HLSLBufferDecl {{.*}} cbuffer B +cbuffer B +{ + // CHECK: VarDecl {{.*}} B0 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + float B0 : packoffset(c0.g); + // CHECK-NEXT: VarDecl {{.*}} B1 'double' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 + double B1 : packoffset(c0.b); + // CHECK-NEXT: VarDecl {{.*}} B2 'half' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 + half B2 : packoffset(c0.r); +} + +// CHECK: HLSLBufferDecl {{.*}} cbuffer C +cbuffer C +{ + // CHECK: VarDecl {{.*}} C0 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + float C0 : packoffset(c0.y); + // CHECK-NEXT: VarDecl {{.*}} C1 'float2' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 + float2 C1 : packoffset(c0.z); + // CHECK-NEXT: VarDecl {{.*}} C2 'half' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 + half C2 : packoffset(c0.x); +} + + +// CHECK: HLSLBufferDecl {{.*}} cbuffer D +cbuffer D +{ + // CHECK: VarDecl {{.*}} D0 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + float D0 : packoffset(c0.y); + // CHECK-NEXT: VarDecl {{.*}} D1 'float[2]' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 4 + float D1[2] : packoffset(c1.x); + // CHECK-NEXT: VarDecl {{.*}} D2 'half3' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 9 + half3 D2 : packoffset(c2.y); + // CHECK-NEXT: VarDecl {{.*}} D3 'double' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 + double D3 : packoffset(c0.z); +} + +struct ST { + float a; + float2 b; + half c; +}; + +// CHECK: HLSLBufferDecl {{.*}} cbuffer S +cbuffer S { + // CHECK: VarDecl {{.*}} S0 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + float S0 : packoffset(c0.y); + // CHECK: VarDecl {{.*}} S1 'ST' + // CHECK: HLSLPackOffsetAttr {{.*}} 4 + ST S1 : packoffset(c1); + // CHECK: VarDecl {{.*}} S2 'double2' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 8 + double2 S2 : packoffset(c2); +} + +struct ST2 { + float s0; + ST s1; + half s2; +}; + +// CHECK: HLSLBufferDecl {{.*}} cbuffer S2 +cbuffer S2 { + // CHECK: VarDecl {{.*}} S20 'float' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 3 + float S20 : packoffset(c0.a); + // CHECK: VarDecl {{.*}} S21 'ST2' + // CHECK: HLSLPackOffsetAttr {{.*}} 4 + ST2 S21 : packoffset(c1); + // CHECK: VarDecl {{.*}} S22 'half' + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 13 + half S22 : packoffset(c3.y); } diff --git a/clang/test/SemaHLSL/packoffset-invalid.hlsl b/clang/test/SemaHLSL/packoffset-invalid.hlsl index a2c7bb5a1e05..34bd51ec0697 100644 --- a/clang/test/SemaHLSL/packoffset-invalid.hlsl +++ b/clang/test/SemaHLSL/packoffset-invalid.hlsl @@ -53,3 +53,70 @@ cbuffer Aggregate // expected-error@+1{{packoffset cannot cross register boundary}} float A2[2] : packoffset(c1.w); } + +cbuffer Double { + // expected-error@+1{{packoffset at 'y' not match alignment 64 required by 'double'}} + double d : packoffset(c.y); + // expected-error@+1{{packoffset cannot cross register boundary}} + double2 d2 : packoffset(c.z); + // expected-error@+1{{packoffset cannot cross register boundary}} + double3 d3 : packoffset(c.z); +} + +cbuffer ParsingFail { +// expected-error@+1{{expected identifier}} +float pf0 : packoffset(); +// expected-error@+1{{expected identifier}} +float pf1 : packoffset((c0)); +// expected-error@+1{{expected ')'}} +float pf2 : packoffset(c0, x); +// expected-error@+1{{invalid component 'X' used}} +float pf3 : packoffset(c.X); +// expected-error@+1{{expected '(' after ''}} +float pf4 : packoffset; +// expected-error@+1{{expected identifier}} +float pf5 : packoffset(; +// expected-error@+1{{expected '(' after '}} +float pf6 : packoffset); +// expected-error@+1{{expected '(' after '}} +float pf7 : packoffset c0.x; + +// expected-error@+1{{invalid component 'xy' used}} +float pf8 : packoffset(c0.xy); +// expected-error@+1{{invalid component 'rg' used}} +float pf9 : packoffset(c0.rg); +// expected-error@+1{{invalid component 'yes' used}} +float pf10 : packoffset(c0.yes); +// expected-error@+1{{invalid component 'woo'}} +float pf11 : packoffset(c0.woo); +// expected-error@+1{{invalid component 'xr' used}} +float pf12 : packoffset(c0.xr); +} + +struct ST2 { + float a; + float2 b; +}; + +cbuffer S { + float S0 : packoffset(c0.y); + ST2 S1[2] : packoffset(c1); + // expected-error@+1{{packoffset overlap between 'S2', 'S1'}} + half2 S2 : packoffset(c1.w); + half2 S3 : packoffset(c2.w); +} + +struct ST23 { + float s0; + ST2 s1; +}; + +cbuffer S2 { + float S20 : packoffset(c0.y); + ST2 S21 : packoffset(c1); + half2 S22 : packoffset(c2.w); + double S23[2] : packoffset(c3); + // expected-error@+1{{packoffset overlap between 'S24', 'S23'}} + float S24 : packoffset(c3.z); + float S25 : packoffset(c4.z); +} >From 72df629bde95909bdc9529edba58bffa0013fd0d Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Wed, 1 May 2024 12:41:53 -0400 Subject: [PATCH 05/10] Update error message. --- clang/lib/Sema/SemaDeclAttr.cpp | 2 +- clang/test/SemaHLSL/packoffset-invalid.hlsl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index d643e5042d7f..19b7bd48b4ae 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7317,7 +7317,7 @@ static void handleHLSLSV_DispatchThreadIDAttr(Sema &S, Decl *D, static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (!isa<VarDecl>(D) || !isa<HLSLBufferDecl>(D->getDeclContext())) { S.Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node) - << AL << "cbuffer constant"; + << AL << "shader constant in a constant buffer"; return; } diff --git a/clang/test/SemaHLSL/packoffset-invalid.hlsl b/clang/test/SemaHLSL/packoffset-invalid.hlsl index 34bd51ec0697..53ed862c91ca 100644 --- a/clang/test/SemaHLSL/packoffset-invalid.hlsl +++ b/clang/test/SemaHLSL/packoffset-invalid.hlsl @@ -16,7 +16,7 @@ cbuffer Mix2 float M6 ; } -// expected-error@+1{{attribute 'packoffset' only applies to cbuffer constant}} +// expected-error@+1{{attribute 'packoffset' only applies to shader constant in a constant buffer}} float4 g : packoffset(c0); cbuffer IllegalOffset >From 1e1a8cef6d33e0bd68172027e37e50079e606aee Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Wed, 1 May 2024 17:22:49 -0400 Subject: [PATCH 06/10] Add the difference for mix packoffset and non-packoffset to the difference doc. --- clang/docs/HLSL/ExpectedDifferences.rst | 15 +++++++++++++++ clang/include/clang/Basic/AttrDocs.td | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst index d1b6010f10f4..fa9c1df04b6c 100644 --- a/clang/docs/HLSL/ExpectedDifferences.rst +++ b/clang/docs/HLSL/ExpectedDifferences.rst @@ -108,3 +108,18 @@ behavior between Clang and DXC. Some examples include: diagnostic notifying the user of the conversion rather than silently altering precision relative to the other overloads (as FXC does) or generating code that will fail validation (as DXC does). + +Mix packoffset and non-packoffset +================================= + +DXC allows mixing packoffset and non-packoffset elements in the same cbuffer, +accompanied by a warning. + +However, both Clang and FXC do not permit this and instead report an error. + +.. code-block:: hlsl + + cbuffer CB { + float4 A; + float4 B : packoffset(c0); + } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index ac08799faa91..2fa694f7d077 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7405,7 +7405,7 @@ The packoffset attribute is used to change the layout of a cbuffer. Attribute spelling in HLSL is: ``packoffset( c[Subcomponent][.component] )``. A subcomponent is a register number, which is an integer. A component is in the form of [.xyzw]. -Here're packoffset examples with and without component: +Examples: .. code-block:: c++ >From 932e7486c197318de1b14974b063cbbb0cfea3c1 Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Mon, 6 May 2024 18:47:03 -0400 Subject: [PATCH 07/10] Change to error on mix into warning. --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaHLSL.cpp | 7 ++++--- clang/test/SemaHLSL/packoffset-invalid.hlsl | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index dadc91974251..63dfe43bf905 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12167,7 +12167,7 @@ def err_hlsl_init_priority_unsupported : Error< def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">; def err_hlsl_unsupported_register_number : Error<"register number should be an integer">; def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">; -def err_hlsl_packoffset_mix : Error<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">; +def warn_hlsl_packoffset_mix : Warning<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">; def err_hlsl_packoffset_overlap : Error<"packoffset overlap between %0, %1">; def err_hlsl_packoffset_cross_reg_boundary : Error<"packoffset cannot cross register boundary">; def err_hlsl_packoffset_alignment_mismatch : Error<"packoffset at 'y' not match alignment %0 required by %1">; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 84859bd74755..6a12c417e2f3 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -94,9 +94,10 @@ void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) { } } - if (HasPackOffset && HasNonPackOffset) { - Diag(BufDecl->getLocation(), diag::err_hlsl_packoffset_mix); - } else if (HasPackOffset) { + if (HasPackOffset && HasNonPackOffset) + Diag(BufDecl->getLocation(), diag::warn_hlsl_packoffset_mix); + + if (HasPackOffset) { ASTContext &Context = getASTContext(); // Make sure no overlap in packoffset. // Sort PackOffsetVec by offset. diff --git a/clang/test/SemaHLSL/packoffset-invalid.hlsl b/clang/test/SemaHLSL/packoffset-invalid.hlsl index 53ed862c91ca..c5983f6fd7e0 100644 --- a/clang/test/SemaHLSL/packoffset-invalid.hlsl +++ b/clang/test/SemaHLSL/packoffset-invalid.hlsl @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.3-library -verify %s -// expected-error@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} +// expected-warning@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} cbuffer Mix { float4 M1 : packoffset(c0); @@ -8,7 +8,7 @@ cbuffer Mix float M3 : packoffset(c1.y); } -// expected-error@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} +// expected-warning@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} cbuffer Mix2 { float4 M4; >From db23da00c3832f1285b2bdd2f12eb9dcff14e023 Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Tue, 7 May 2024 11:11:31 -0400 Subject: [PATCH 08/10] Switch to save 2 int args. --- clang/include/clang/Basic/Attr.td | 7 ++++++- clang/lib/Parse/ParseHLSL.cpp | 21 +++++++++++-------- clang/lib/Sema/SemaDeclAttr.cpp | 25 ++++++++++++---------- clang/test/AST/HLSL/packoffset.hlsl | 32 ++++++++++++++--------------- 4 files changed, 49 insertions(+), 36 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index d3d006ed9633..07baa6edd767 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4375,8 +4375,13 @@ def HLSLResourceBinding: InheritableAttr { def HLSLPackOffset: HLSLAnnotationAttr { let Spellings = [HLSLAnnotation<"packoffset">]; let LangOpts = [HLSL]; - let Args = [IntArgument<"Offset">]; + let Args = [IntArgument<"Subcomponent">, IntArgument<"Component">]; let Documentation = [HLSLPackOffsetDocs]; + let AdditionalMembers = [{ + unsigned getOffset() { + return subcomponent * 4 + component; + } + }]; } def HLSLSV_DispatchThreadID: HLSLAnnotationAttr { diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp index ce3b2eef9d4e..e9c8d6dca7bf 100644 --- a/clang/lib/Parse/ParseHLSL.cpp +++ b/clang/lib/Parse/ParseHLSL.cpp @@ -197,7 +197,7 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, return; } StringRef OffsetStr = Tok.getIdentifierInfo()->getName(); - SourceLocation OffsetLoc = Tok.getLocation(); + SourceLocation SubComponentLoc = Tok.getLocation(); if (OffsetStr[0] != 'c') { Diag(Tok.getLocation(), diag::err_hlsl_packoffset_invalid_reg) << OffsetStr; @@ -209,7 +209,7 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, if (!OffsetStr.empty()) { // Make sure SubComponent is a number. if (OffsetStr.getAsInteger(10, SubComponent)) { - Diag(OffsetLoc.getLocWithOffset(1), + Diag(SubComponentLoc.getLocWithOffset(1), diag::err_hlsl_unsupported_register_number); SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; @@ -217,6 +217,7 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, } unsigned Component = 0; ConsumeToken(); // consume identifier. + SourceLocation ComponentLoc; if (Tok.is(tok::period)) { ConsumeToken(); // consume period. if (!Tok.is(tok::identifier)) { @@ -225,11 +226,12 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, return; } StringRef ComponentStr = Tok.getIdentifierInfo()->getName(); - SourceLocation SpaceLoc = Tok.getLocation(); + ComponentLoc = Tok.getLocation(); ConsumeToken(); // consume identifier. // Make sure Component is a single character. if (ComponentStr.size() != 1) { - Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; + Diag(ComponentLoc, diag::err_hlsl_unsupported_component) + << ComponentStr; SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; } @@ -251,16 +253,19 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs, Component = 3; break; default: - Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; + Diag(ComponentLoc, diag::err_hlsl_unsupported_component) + << ComponentStr; SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; } } - unsigned Offset = SubComponent * 4 + Component; ASTContext &Ctx = Actions.getASTContext(); + QualType SizeTy = Ctx.getSizeType(); + uint64_t SizeTySize = Ctx.getTypeSize(SizeTy); + ArgExprs.push_back(IntegerLiteral::Create( + Ctx, llvm::APInt(SizeTySize, SubComponent), SizeTy, SubComponentLoc)); ArgExprs.push_back(IntegerLiteral::Create( - Ctx, llvm::APInt(Ctx.getTypeSize(Ctx.getSizeType()), Offset), - Ctx.getSizeType(), OffsetLoc)); + Ctx, llvm::APInt(SizeTySize, Component), SizeTy, ComponentLoc)); if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 19b7bd48b4ae..486405887731 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7321,8 +7321,11 @@ static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { return; } - uint32_t Offset; - if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Offset)) + uint32_t SubComponent; + if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), SubComponent)) + return; + uint32_t Component; + if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(1), Component)) return; QualType T = cast<VarDecl>(D)->getType().getCanonicalType(); @@ -7330,16 +7333,15 @@ static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // TODO: mark matrix type as aggregate type. bool IsAggregateTy = (T->isArrayType() || T->isStructureType()); - unsigned ComponentNum = Offset & 0x3; - // Check ComponentNum is valid for T. - if (ComponentNum) { + // Check Component is valid for T. + if (Component) { unsigned Size = S.getASTContext().getTypeSize(T); if (IsAggregateTy || Size > 128) { S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); return; } else { - // Make sure ComponentNum + sizeof(T) <= 4. - if ((ComponentNum * 32 + Size) > 128) { + // Make sure Component + sizeof(T) <= 4. + if ((Component * 32 + Size) > 128) { S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_cross_reg_boundary); return; } @@ -7347,9 +7349,9 @@ static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (const auto *VT = T->getAs<VectorType>()) EltTy = VT->getElementType(); unsigned Align = S.getASTContext().getTypeAlign(EltTy); - if (Align > 32 && ComponentNum == 1) { - // NOTE: ComponentNum 3 will hit err_hlsl_packoffset_cross_reg_boundary. - // So we only need to check ComponentNum 1 here. + if (Align > 32 && Component == 1) { + // NOTE: Component 3 will hit err_hlsl_packoffset_cross_reg_boundary. + // So we only need to check Component 1 here. S.Diag(AL.getLoc(), diag::err_hlsl_packoffset_alignment_mismatch) << Align << EltTy; return; @@ -7357,7 +7359,8 @@ static void handleHLSLPackOffsetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } } - D->addAttr(::new (S.Context) HLSLPackOffsetAttr(S.Context, AL, Offset)); + D->addAttr(::new (S.Context) + HLSLPackOffsetAttr(S.Context, AL, SubComponent, Component)); } static void handleHLSLShaderAttr(Sema &S, Decl *D, const ParsedAttr &AL) { diff --git a/clang/test/AST/HLSL/packoffset.hlsl b/clang/test/AST/HLSL/packoffset.hlsl index d9f96661315d..9cfd88eeec33 100644 --- a/clang/test/AST/HLSL/packoffset.hlsl +++ b/clang/test/AST/HLSL/packoffset.hlsl @@ -5,13 +5,13 @@ cbuffer A { // CHECK-NEXT: VarDecl {{.*}} A1 'float4' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 0 float4 A1 : packoffset(c); // CHECK-NEXT: VarDecl {{.*}} col:11 A2 'float' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 4 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 0 float A2 : packoffset(c1); // CHECK-NEXT: VarDecl {{.*}} col:11 A3 'float' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 5 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 1 float A3 : packoffset(c1.y); } @@ -19,13 +19,13 @@ cbuffer A cbuffer B { // CHECK: VarDecl {{.*}} B0 'float' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 1 float B0 : packoffset(c0.g); // CHECK-NEXT: VarDecl {{.*}} B1 'double' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 2 double B1 : packoffset(c0.b); // CHECK-NEXT: VarDecl {{.*}} B2 'half' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 0 half B2 : packoffset(c0.r); } @@ -48,16 +48,16 @@ cbuffer C cbuffer D { // CHECK: VarDecl {{.*}} D0 'float' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 1 float D0 : packoffset(c0.y); // CHECK-NEXT: VarDecl {{.*}} D1 'float[2]' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 4 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 0 float D1[2] : packoffset(c1.x); // CHECK-NEXT: VarDecl {{.*}} D2 'half3' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 9 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 1 half3 D2 : packoffset(c2.y); // CHECK-NEXT: VarDecl {{.*}} D3 'double' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 2 double D3 : packoffset(c0.z); } @@ -70,13 +70,13 @@ struct ST { // CHECK: HLSLBufferDecl {{.*}} cbuffer S cbuffer S { // CHECK: VarDecl {{.*}} S0 'float' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 1 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 1 float S0 : packoffset(c0.y); // CHECK: VarDecl {{.*}} S1 'ST' - // CHECK: HLSLPackOffsetAttr {{.*}} 4 + // CHECK: HLSLPackOffsetAttr {{.*}} 1 0 ST S1 : packoffset(c1); // CHECK: VarDecl {{.*}} S2 'double2' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 8 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 2 0 double2 S2 : packoffset(c2); } @@ -89,12 +89,12 @@ struct ST2 { // CHECK: HLSLBufferDecl {{.*}} cbuffer S2 cbuffer S2 { // CHECK: VarDecl {{.*}} S20 'float' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 3 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 3 float S20 : packoffset(c0.a); // CHECK: VarDecl {{.*}} S21 'ST2' - // CHECK: HLSLPackOffsetAttr {{.*}} 4 + // CHECK: HLSLPackOffsetAttr {{.*}} 1 0 ST2 S21 : packoffset(c1); // CHECK: VarDecl {{.*}} S22 'half' - // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 13 + // CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 3 1 half S22 : packoffset(c3.y); } >From 81f1b3984ffde1399cadd831ddfb1e5e0df1afd7 Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Tue, 7 May 2024 13:23:15 -0400 Subject: [PATCH 09/10] Remove the expected difference. --- clang/docs/HLSL/ExpectedDifferences.rst | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst index fa9c1df04b6c..d1b6010f10f4 100644 --- a/clang/docs/HLSL/ExpectedDifferences.rst +++ b/clang/docs/HLSL/ExpectedDifferences.rst @@ -108,18 +108,3 @@ behavior between Clang and DXC. Some examples include: diagnostic notifying the user of the conversion rather than silently altering precision relative to the other overloads (as FXC does) or generating code that will fail validation (as DXC does). - -Mix packoffset and non-packoffset -================================= - -DXC allows mixing packoffset and non-packoffset elements in the same cbuffer, -accompanied by a warning. - -However, both Clang and FXC do not permit this and instead report an error. - -.. code-block:: hlsl - - cbuffer CB { - float4 A; - float4 B : packoffset(c0); - } >From 4b6cc97baf2a6c9e7398806dd4195f0d396a9fa0 Mon Sep 17 00:00:00 2001 From: Xiang Li <python3k...@outlook.com> Date: Tue, 7 May 2024 17:35:43 -0400 Subject: [PATCH 10/10] Add warning group for mix packoffset. --- clang/include/clang/Basic/DiagnosticGroups.td | 3 +++ clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 60f87da2a738..2beb1d45124b 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1507,6 +1507,9 @@ def BranchProtection : DiagGroup<"branch-protection">; // Warnings for HLSL Clang extensions def HLSLExtension : DiagGroup<"hlsl-extensions">; +// Warning for mix packoffset and non-packoffset. +def HLSLMixPackOffset : DiagGroup<"mix-packoffset">; + // Warnings for DXIL validation def DXILValidation : DiagGroup<"dxil-validation">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 63dfe43bf905..dad9ad0cf519 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -12167,7 +12167,8 @@ def err_hlsl_init_priority_unsupported : Error< def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">; def err_hlsl_unsupported_register_number : Error<"register number should be an integer">; def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">; -def warn_hlsl_packoffset_mix : Warning<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">; +def warn_hlsl_packoffset_mix : Warning<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">, + InGroup<HLSLMixPackOffset>; def err_hlsl_packoffset_overlap : Error<"packoffset overlap between %0, %1">; def err_hlsl_packoffset_cross_reg_boundary : Error<"packoffset cannot cross register boundary">; def err_hlsl_packoffset_alignment_mismatch : Error<"packoffset at 'y' not match alignment %0 required by %1">; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits