[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
Xiang Li via cfe-commits
cfe-commits at lists.llvm.org
Tue May 7 08:11:52 PDT 2024
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 <python3kgae at outlook.com>
Date: Tue, 23 Apr 2024 17:49:02 -0400
Subject: [PATCH 1/8] [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 4408d517e70e5..d3d006ed9633f 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 a0bbe5861c572..6bc7813bd43cb 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 38174cf3549f1..81433ee79d48b 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 63e951daec747..bde9617c9820a 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 f4cbece31f181..eac4876ccab49 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 363ae93cb62df..373f2e8f50cdb 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 bb9e37f18d370..fa62cab54e690 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 0000000000000..d3cf798c99575
--- /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 0000000000000..a2c7bb5a1e05c
--- /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 at +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 at +1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}}
+cbuffer Mix2
+{
+ float4 M4;
+ float M5 : packoffset(c1.y);
+ float M6 ;
+}
+
+// expected-error at +1{{attribute 'packoffset' only applies to cbuffer constant}}
+float4 g : packoffset(c0);
+
+cbuffer IllegalOffset
+{
+ // expected-error at +1{{invalid resource class specifier 't2' for packoffset, expected 'c'}}
+ float4 i1 : packoffset(t2);
+ // expected-error at +1{{invalid component 'm' used; expected 'x', 'y', 'z', or 'w'}}
+ float i2 : packoffset(c1.m);
+}
+
+cbuffer Overlap
+{
+ float4 o1 : packoffset(c0);
+ // expected-error at +1{{packoffset overlap between 'o2', 'o1'}}
+ float2 o2 : packoffset(c0.z);
+}
+
+cbuffer CrossReg
+{
+ // expected-error at +1{{packoffset cannot cross register boundary}}
+ float4 c1 : packoffset(c0.y);
+ // expected-error at +1{{packoffset cannot cross register boundary}}
+ float2 c2 : packoffset(c1.w);
+}
+
+struct ST {
+ float s;
+};
+
+cbuffer Aggregate
+{
+ // expected-error at +1{{packoffset cannot cross register boundary}}
+ ST A1 : packoffset(c0.y);
+ // expected-error at +1{{packoffset cannot cross register boundary}}
+ float A2[2] : packoffset(c1.w);
+}
>From c356db64f307bd042d40cbade7f09824c7bf238c Mon Sep 17 00:00:00 2001
From: Xiang Li <python3kgae at outlook.com>
Date: Wed, 24 Apr 2024 09:37:45 -0400
Subject: [PATCH 2/8] 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 6bc7813bd43cb..20e58897f20b2 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 <python3kgae at outlook.com>
Date: Thu, 25 Apr 2024 13:58:18 -0400
Subject: [PATCH 3/8] 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 20e58897f20b2..ac08799faa91f 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 eac4876ccab49..ae9ceaa700af8 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 373f2e8f50cdb..cf7d690f62883 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 fa62cab54e690..b2eb69dd5a145 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 <python3kgae at outlook.com>
Date: Wed, 1 May 2024 12:28:38 -0400
Subject: [PATCH 4/8] 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 bde9617c9820a..dadc91974251c 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 ae9ceaa700af8..ce3b2eef9d4ee 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 cf7d690f62883..d643e5042d7fb 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 b2eb69dd5a145..84859bd74755e 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 d3cf798c99575..d9f96661315d7 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 a2c7bb5a1e05c..34bd51ec06972 100644
--- a/clang/test/SemaHLSL/packoffset-invalid.hlsl
+++ b/clang/test/SemaHLSL/packoffset-invalid.hlsl
@@ -53,3 +53,70 @@ cbuffer Aggregate
// expected-error at +1{{packoffset cannot cross register boundary}}
float A2[2] : packoffset(c1.w);
}
+
+cbuffer Double {
+ // expected-error at +1{{packoffset at 'y' not match alignment 64 required by 'double'}}
+ double d : packoffset(c.y);
+ // expected-error at +1{{packoffset cannot cross register boundary}}
+ double2 d2 : packoffset(c.z);
+ // expected-error at +1{{packoffset cannot cross register boundary}}
+ double3 d3 : packoffset(c.z);
+}
+
+cbuffer ParsingFail {
+// expected-error at +1{{expected identifier}}
+float pf0 : packoffset();
+// expected-error at +1{{expected identifier}}
+float pf1 : packoffset((c0));
+// expected-error at +1{{expected ')'}}
+float pf2 : packoffset(c0, x);
+// expected-error at +1{{invalid component 'X' used}}
+float pf3 : packoffset(c.X);
+// expected-error at +1{{expected '(' after ''}}
+float pf4 : packoffset;
+// expected-error at +1{{expected identifier}}
+float pf5 : packoffset(;
+// expected-error at +1{{expected '(' after '}}
+float pf6 : packoffset);
+// expected-error at +1{{expected '(' after '}}
+float pf7 : packoffset c0.x;
+
+// expected-error at +1{{invalid component 'xy' used}}
+float pf8 : packoffset(c0.xy);
+// expected-error at +1{{invalid component 'rg' used}}
+float pf9 : packoffset(c0.rg);
+// expected-error at +1{{invalid component 'yes' used}}
+float pf10 : packoffset(c0.yes);
+// expected-error at +1{{invalid component 'woo'}}
+float pf11 : packoffset(c0.woo);
+// expected-error at +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 at +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 at +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 <python3kgae at outlook.com>
Date: Wed, 1 May 2024 12:41:53 -0400
Subject: [PATCH 5/8] 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 d643e5042d7fb..19b7bd48b4ae3 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 34bd51ec06972..53ed862c91cae 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 at +1{{attribute 'packoffset' only applies to cbuffer constant}}
+// expected-error at +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 <python3kgae at outlook.com>
Date: Wed, 1 May 2024 17:22:49 -0400
Subject: [PATCH 6/8] 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 d1b6010f10f43..fa9c1df04b6cf 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 ac08799faa91f..2fa694f7d0776 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 <python3kgae at outlook.com>
Date: Mon, 6 May 2024 18:47:03 -0400
Subject: [PATCH 7/8] 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 dadc91974251c..63dfe43bf905f 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 84859bd74755e..6a12c417e2f3a 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 53ed862c91cae..c5983f6fd7e07 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 at +1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}}
+// expected-warning at +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 at +1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}}
+// expected-warning at +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 <python3kgae at outlook.com>
Date: Tue, 7 May 2024 11:11:31 -0400
Subject: [PATCH 8/8] 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 d3d006ed9633f..07baa6edd7674 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 ce3b2eef9d4ee..e9c8d6dca7bf5 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 19b7bd48b4ae3..4864058877317 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 d9f96661315d7..9cfd88eeec330 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);
}
More information about the cfe-commits
mailing list