https://github.com/hekota created
https://github.com/llvm/llvm-project/pull/121989
There will be more changes coming in to `SemaHLSL::ActOnFinishBuffer` so it
would be good to move the packoffset validation out to a separate function.
This PR also unifies the units for cbuffer offset calculations to bytes.
>From 7b031383b6b8f3fb3deedc6a8698c17bd5b5ed3d Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Tue, 7 Jan 2025 12:07:28 -0800
Subject: [PATCH] [HLSL][NFC] Move packoffset validation to separate function
and calculate offsets in bytes
---
clang/include/clang/Basic/Attr.td | 6 +--
clang/lib/Sema/SemaHLSL.cpp | 78 +++++++++++++++++--------------
2 files changed, 47 insertions(+), 37 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index bc5f0662adf8e9..f6ecf046a2c42e 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4721,9 +4721,9 @@ def HLSLPackOffset: HLSLAnnotationAttr {
let Args = [IntArgument<"Subcomponent">, IntArgument<"Component">];
let Documentation = [HLSLPackOffsetDocs];
let AdditionalMembers = [{
- unsigned getOffset() {
- return subcomponent * 4 + component;
- }
+ unsigned getOffsetInBytes() {
+ return subcomponent * 16 + component * 4;
+ }
}];
}
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 600c800029fd05..d7012b496aaf0d 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -164,18 +164,18 @@ Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool
CBuffer,
return Result;
}
-// Calculate the size of a legacy cbuffer type based on
+// Calculate the size of a legacy cbuffer type in bytes 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;
+ constexpr unsigned CBufferAlign = 16;
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;
+ unsigned FieldAlign = 4;
if (Ty->isAggregateType())
FieldAlign = CBufferAlign;
Size = llvm::alignTo(Size, FieldAlign);
@@ -194,17 +194,19 @@ static unsigned calculateLegacyCbufferSize(const
ASTContext &Context,
calculateLegacyCbufferSize(Context, VT->getElementType());
Size = ElementSize * ElementCount;
} else {
- Size = Context.getTypeSize(T);
+ Size = Context.getTypeSize(T) / 8;
}
return Size;
}
-void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) {
- auto *BufDecl = cast<HLSLBufferDecl>(Dcl);
- BufDecl->setRBraceLoc(RBrace);
-
- // Validate packoffset.
+// Validate packoffset:
+// - make sure if packoffset it used on all decls or none
+// - the packoffset ranges must not overlap
+static void validatePackoffset(Sema &S, HLSLBufferDecl *BufDecl) {
llvm::SmallVector<std::pair<VarDecl *, HLSLPackOffsetAttr *>> PackOffsetVec;
+
+ // Make sure the packoffset annotations are either on all declarations
+ // or on none.
bool HasPackOffset = false;
bool HasNonPackOffset = false;
for (auto *Field : BufDecl->decls()) {
@@ -219,33 +221,41 @@ void SemaHLSL::ActOnFinishBuffer(Decl *Dcl,
SourceLocation RBrace) {
}
}
- 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.
- 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 = calculateLegacyCbufferSize(Context, Var->getType());
- unsigned Begin = Attr->getOffset() * 32;
- unsigned End = Begin + Size;
- 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;
- }
+ if (!HasPackOffset)
+ return;
+
+ if (HasNonPackOffset)
+ S.Diag(BufDecl->getLocation(), diag::warn_hlsl_packoffset_mix);
+
+ // Make sure there is no overlap in packoffset - sort PackOffsetVec by offset
+ // and compare adjacent values.
+ ASTContext &Context = S.getASTContext();
+ std::sort(PackOffsetVec.begin(), PackOffsetVec.end(),
+ [](const std::pair<VarDecl *, HLSLPackOffsetAttr *> &LHS,
+ const std::pair<VarDecl *, HLSLPackOffsetAttr *> &RHS) {
+ return LHS.second->getOffsetInBytes() <
+ RHS.second->getOffsetInBytes();
+ });
+ for (unsigned i = 0; i < PackOffsetVec.size() - 1; i++) {
+ VarDecl *Var = PackOffsetVec[i].first;
+ HLSLPackOffsetAttr *Attr = PackOffsetVec[i].second;
+ unsigned Size = calculateLegacyCbufferSize(Context, Var->getType());
+ unsigned Begin = Attr->getOffsetInBytes();
+ unsigned End = Begin + Size;
+ unsigned NextBegin = PackOffsetVec[i + 1].second->getOffsetInBytes();
+ if (End > NextBegin) {
+ VarDecl *NextVar = PackOffsetVec[i + 1].first;
+ S.Diag(NextVar->getLocation(), diag::err_hlsl_packoffset_overlap)
+ << NextVar << Var;
}
}
+}
+
+void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) {
+ auto *BufDecl = cast<HLSLBufferDecl>(Dcl);
+ BufDecl->setRBraceLoc(RBrace);
+
+ validatePackoffset(SemaRef, BufDecl);
SemaRef.PopDeclContext();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
