[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
https://github.com/hekota updated
https://github.com/llvm/llvm-project/pull/128991
>From 9faff902639aece87b72ed5235d71b8b68533074 Mon Sep 17 00:00:00 2001
From: Helena Kotas
Date: Wed, 26 Feb 2025 17:39:16 -0800
Subject: [PATCH 1/8] Add resource binding attribute on $Globals numeric
constants
---
clang/lib/Sema/SemaHLSL.cpp | 10 +--
.../test/AST/HLSL/resource_binding_attr.hlsl | 67 ---
2 files changed, 47 insertions(+), 30 deletions(-)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 283a9801fc707..ffc3ac1b65854 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1446,18 +1446,20 @@ static bool DiagnoseLocalRegisterBinding(Sema &S,
SourceLocation &ArgLoc,
Ty = Ty->getArrayElementTypeNoTypeQual();
// Basic types
- if (Ty->isArithmeticType()) {
+ if (Ty->isArithmeticType() || Ty->isVectorType()) {
bool DeclaredInCOrTBuffer = isa(D->getDeclContext());
if (SpecifiedSpace && !DeclaredInCOrTBuffer)
S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
-if (!DeclaredInCOrTBuffer &&
-(Ty->isIntegralType(S.getASTContext()) || Ty->isFloatingType())) {
- // Default Globals
+if (!DeclaredInCOrTBuffer && (Ty->isIntegralType(S.getASTContext()) ||
+ Ty->isFloatingType() || Ty->isVectorType()))
{
+ // Register annotation on default constant buffer declaration ($Globals)
if (RegType == RegisterType::CBuffer)
S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b);
else if (RegType != RegisterType::C)
S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum;
+ else
+return true;
} else {
if (RegType == RegisterType::C)
S.Diag(ArgLoc, diag::warn_hlsl_register_type_c_packoffset);
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl
b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index 6fac903f75e18..7ba4f0f60e83c 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -1,41 +1,56 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o
- %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library
-finclude-default-header -ast-dump -o - %s | FileCheck %s
-// CHECK:HLSLBufferDecl 0x[[CB:[0-9a-f]+]] {{.*}} line:8:9 cbuffer CB
-// CHECK-NEXT:HLSLResourceClassAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit CBuffer
-// CHECK-NEXT:HLSLResourceAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit CBuffer
-// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "b3" "space2"
-// CHECK-NEXT:VarDecl 0x[[A:[0-9a-f]+]] {{.*}} col:9 used a 'hlsl_constant
float'
+// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 5]]:9 cbuffer CB
+// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK-NEXT: HLSLResourceAttr {{.*}} Implicit CBuffer
+// CHECK-NEXT: HLSLResourceBindingAttr {{.*}} "b3" "space2"
+// CHECK-NEXT: VarDecl {{.*}} used a 'hlsl_constant float'
cbuffer CB : register(b3, space2) {
float a;
}
-// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:17:9 tbuffer TB
-// CHECK-NEXT:HLSLResourceClassAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit SRV
-// CHECK-NEXT:HLSLResourceAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit TBuffer
-// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "t2" "space1"
-// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'hlsl_constant
float'
+// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 5]]:9 tbuffer TB
+// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit SRV
+// CHECK-NEXT: HLSLResourceAttr {{.*}} Implicit TBuffer
+// CHECK-NEXT: HLSLResourceBindingAttr {{.*}} "t2" "space1"
+// CHECK-NEXT: VarDecl {{.*}} used b 'hlsl_constant float'
tbuffer TB : register(t2, space1) {
float b;
}
-float foo() {
-// CHECK: BinaryOperator 0x{{[0-9a-f]+}} 'float' '+'
-// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}} 'float'
-// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}} 'hlsl_constant float'
lvalue Var 0x[[A]] 'a' 'hlsl_constant float'
-// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}} 'float'
-// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}} 'hlsl_constant float'
lvalue Var 0x[[B]] 'b' 'hlsl_constant float'
+export float foo() {
return a + b;
}
-// CHECK: VarDecl 0x{{[0-9a-f]+}} <{{.*}}> col:17 UAV
'RWBuffer':'hlsl::RWBuffer' callinit
-// CHECK-NEXT:-CXXConstructExpr 0x{{[0-9a-f]+}}
'RWBuffer':'hlsl::RWBuffer' 'void ()'
-// CHECK-NEXT:-HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "u3" "space0"
+// CHECK: VarDecl {{.*}} UAV 'RWBuffer':'hlsl::RWBuffer'
+// CHECK: HLSLResourceBindingAttr {{.*}} "u3" "space0"
RWBuffer UAV : register(u3);
-// CHECK: -VarDecl 0x{{[0-9a-f]+}} <{{.*}}> col:17 UAV1
'RWBuffer':'hlsl::RWBuffer' callinit
-// CHECK-NEXT:-CXXConstructExpr 0x{{[0-9a-f]+}}
'RWBuffer':'hlsl::RWBuffer' 'void ()'
-// CHECK-NEXT:-HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "u2" "space0"
-// CHECK-NEXT:-VarDecl 0x{{[0-9a-f]+}} col:38 UAV2
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
https://github.com/hekota edited https://github.com/llvm/llvm-project/pull/128991 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
+ LayoutElements.push_back(FieldType);
+}
+ }
- if (!layoutField(FD, EndOffset, Layout, LayoutElements,
- Packoffsets ? (*Packoffsets)[Index] : -1))
+ // process delayed layouts
+ if (!DelayLayoutFields.empty()) {
+for (auto I : DelayLayoutFields) {
+ const FieldDecl *FD = I.first;
+ unsigned IndexInLayoutElements = I.second;
hekota wrote:
Yeah, I don't think we use const much unless the compiler tells us :).
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,45 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
+// vector with the user-specified layout offsets.
+// The buffer offsets can be specified 2 ways:
+// 1. declarations in cbuffer {} block can have a packoffset annotation
+//(translates to HLSLPackOffsetAttr)
+// 2. default constant buffer declarations at global scope can have
+//register(c#) annotations (translates to HLSLResourceBindingAttr with
+//RegisterType::C)
+// It is not guaranteed that all declarations in a buffer have an annotation.
+// For those where it is not specified a -1 value is added to the Layout
+// vector. In the final layout these declarations will be placed at the end
+// of the HLSL buffer after all of the elements with specified offset.
static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
- SmallVector &Layout) {
+ SmallVector &Layout) {
assert(Layout.empty() && "expected empty vector for layout");
assert(BufDecl->hasValidPackoffset());
- for (Decl *D : BufDecl->decls()) {
+ for (Decl *D : BufDecl->buffer_decls()) {
hekota wrote:
It depends. I got feedback in the past to not use `auto` if the type is not
immediately obvious. For example with `dyn_cast` the type name is on the same
line, so it is ok to use `auto` there. In the end I think it is more of a
personal preference.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,45 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
+// vector with the user-specified layout offsets.
+// The buffer offsets can be specified 2 ways:
+// 1. declarations in cbuffer {} block can have a packoffset annotation
+//(translates to HLSLPackOffsetAttr)
+// 2. default constant buffer declarations at global scope can have
+//register(c#) annotations (translates to HLSLResourceBindingAttr with
+//RegisterType::C)
+// It is not guaranteed that all declarations in a buffer have an annotation.
+// For those where it is not specified a -1 value is added to the Layout
+// vector. In the final layout these declarations will be placed at the end
+// of the HLSL buffer after all of the elements with specified offset.
static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
- SmallVector &Layout) {
+ SmallVector &Layout) {
assert(Layout.empty() && "expected empty vector for layout");
assert(BufDecl->hasValidPackoffset());
- for (Decl *D : BufDecl->decls()) {
+ for (Decl *D : BufDecl->buffer_decls()) {
if (isa(D) || isa(D)) {
continue;
}
VarDecl *VD = dyn_cast(D);
if (!VD || VD->getType().getAddressSpace() != LangAS::hlsl_constant)
continue;
-assert(VD->hasAttr() &&
- "expected packoffset attribute on every declaration");
-size_t Offset = VD->getAttr()->getOffsetInBytes();
+size_t Offset = -1;
hekota wrote:
Good catch! I'll update the type to `int32_t` to match the SmallVector template
type.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
+ LayoutElements.push_back(FieldType);
+}
+ }
- if (!layoutField(FD, EndOffset, Layout, LayoutElements,
- Packoffsets ? (*Packoffsets)[Index] : -1))
+ // process delayed layouts
+ if (!DelayLayoutFields.empty()) {
+for (auto I : DelayLayoutFields) {
+ const FieldDecl *FD = I.first;
+ unsigned IndexInLayoutElements = I.second;
alsepkow wrote:
a few other variables in this block could also be const.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
+ LayoutElements.push_back(FieldType);
+}
+ }
- if (!layoutField(FD, EndOffset, Layout, LayoutElements,
- Packoffsets ? (*Packoffsets)[Index] : -1))
+ // process delayed layouts
+ if (!DelayLayoutFields.empty()) {
+for (auto I : DelayLayoutFields) {
+ const FieldDecl *FD = I.first;
+ unsigned IndexInLayoutElements = I.second;
+ // the first item in layout vector is size, so we need to offset the
index
+ // by 1
+ unsigned IndexInLayout = IndexInLayoutElements + 1;
+ assert(Layout[IndexInLayout] == UINT_MAX &&
+ LayoutElements[IndexInLayoutElements] == nullptr);
+
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
return nullptr;
- Index++;
+
+ Layout[IndexInLayout] = (unsigned)FieldOffset;
alsepkow wrote:
Cast no longer needed
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,45 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
alsepkow wrote:
Found it 🥳
[dx-graphics-hlsl-variable-register](https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-variable-register
)
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,45 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
+// vector with the user-specified layout offsets.
+// The buffer offsets can be specified 2 ways:
+// 1. declarations in cbuffer {} block can have a packoffset annotation
+//(translates to HLSLPackOffsetAttr)
+// 2. default constant buffer declarations at global scope can have
+//register(c#) annotations (translates to HLSLResourceBindingAttr with
+//RegisterType::C)
+// It is not guaranteed that all declarations in a buffer have an annotation.
+// For those where it is not specified a -1 value is added to the Layout
+// vector. In the final layout these declarations will be placed at the end
+// of the HLSL buffer after all of the elements with specified offset.
static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
- SmallVector &Layout) {
+ SmallVector &Layout) {
assert(Layout.empty() && "expected empty vector for layout");
assert(BufDecl->hasValidPackoffset());
- for (Decl *D : BufDecl->decls()) {
+ for (Decl *D : BufDecl->buffer_decls()) {
alsepkow wrote:
nit: On formatting, it sounds like the llvm repo prefers 'auto *name' for for
loops?
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,45 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
alsepkow wrote:
Just curious, what is 'register(c#)' annotation? I didn't get any hits trying
to search it online or with copilot.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,45 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
+// vector with the user-specified layout offsets.
+// The buffer offsets can be specified 2 ways:
+// 1. declarations in cbuffer {} block can have a packoffset annotation
+//(translates to HLSLPackOffsetAttr)
+// 2. default constant buffer declarations at global scope can have
+//register(c#) annotations (translates to HLSLResourceBindingAttr with
+//RegisterType::C)
+// It is not guaranteed that all declarations in a buffer have an annotation.
+// For those where it is not specified a -1 value is added to the Layout
+// vector. In the final layout these declarations will be placed at the end
+// of the HLSL buffer after all of the elements with specified offset.
static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
- SmallVector &Layout) {
+ SmallVector &Layout) {
assert(Layout.empty() && "expected empty vector for layout");
assert(BufDecl->hasValidPackoffset());
- for (Decl *D : BufDecl->decls()) {
+ for (Decl *D : BufDecl->buffer_decls()) {
if (isa(D) || isa(D)) {
continue;
}
VarDecl *VD = dyn_cast(D);
if (!VD || VD->getType().getAddressSpace() != LangAS::hlsl_constant)
continue;
-assert(VD->hasAttr() &&
- "expected packoffset attribute on every declaration");
-size_t Offset = VD->getAttr()->getOffsetInBytes();
+size_t Offset = -1;
alsepkow wrote:
size_t is unsigned. You should use something like SIZE_MAX instead. Or 0?
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
alsepkow wrote:
P0 should be a const, I think? I noticed we don't really use that pattern in
DXC but it looks like it is used here.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
alsepkow wrote:
Left over cast from a previous revision? Layout contains unsigned ints.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
+ LayoutElements.push_back(FieldType);
+}
+ }
- if (!layoutField(FD, EndOffset, Layout, LayoutElements,
- Packoffsets ? (*Packoffsets)[Index] : -1))
+ // process delayed layouts
+ if (!DelayLayoutFields.empty()) {
+for (auto I : DelayLayoutFields) {
+ const FieldDecl *FD = I.first;
+ unsigned IndexInLayoutElements = I.second;
alsepkow wrote:
const?
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -58,9 +60,15 @@ namespace CodeGen {
// classes) and calls layoutField to converts each field to its corresponding
// LLVM type and to calculate its HLSL constant buffer layout. Any embedded
// structs (or arrays of structs) are converted to target layout types as well.
+//
+// When Packoffsets are specified the elements will be placed based on the
+// user-specified offsets. Not all elements must have a packoffset/register(c#)
+// annotation though. For those that don't, the Packoffsets array will constain
alsepkow wrote:
nit: typo, 'contain'
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
https://github.com/V-FEXrt approved this pull request. https://github.com/llvm/llvm-project/pull/128991 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -174,21 +176,51 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
return cast(QT.getTypePtr());
}
+// Iterates over all declarations in the HLSL buffer and based on the
+// packoffset or register(c#) annotations it fills outs the Layout
+// vector with the user-specified layout offsets.
+// The buffer offsets can be specified 2 ways:
+// 1. declarations in cbuffer {} block can have a packoffset annotation
+//(translates to HLSLPackOffsetAttr)
+// 2. default constant buffer declarations at global scope can have
+//register(c#) annotations (translates to HLSLResourceBindingAttr with
+//RegisterType::C)
+// It is not guaranteed that all declarations in a buffer have an annotation.
+// For those where it is not specified a -1 value is added to the Layout
+// vector. In the final layout these declarations will be placed at the end
+// of the HLSL buffer after all of the elements with specified offset.
static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
- SmallVector &Layout) {
+ SmallVector &Layout) {
assert(Layout.empty() && "expected empty vector for layout");
assert(BufDecl->hasValidPackoffset());
- for (Decl *D : BufDecl->decls()) {
+ for (Decl *D : BufDecl->buffer_decls()) {
if (isa(D) || isa(D)) {
continue;
}
VarDecl *VD = dyn_cast(D);
if (!VD || VD->getType().getAddressSpace() != LangAS::hlsl_constant)
continue;
-assert(VD->hasAttr() &&
- "expected packoffset attribute on every declaration");
-size_t Offset = VD->getAttr()->getOffsetInBytes();
+
+if (!VD->hasAttrs()) {
+ Layout.push_back(-1);
+ continue;
+}
+
+size_t Offset = -1;
+for (auto *Attr : VD->getAttrs()) {
+ if (auto *POA = dyn_cast(Attr)) {
+Offset = POA->getOffsetInBytes();
+break;
+ }
+ auto *RBA = dyn_cast(Attr);
+ if (RBA &&
+ RBA->getRegisterType() == HLSLResourceBindingAttr::RegisterType::C) {
+// size of constant buffer row is 16 bytes
V-FEXrt wrote:
nit: I don't think this comment is providing anything now that there is a
constant for the value
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
+ LayoutElements.push_back(FieldType);
hekota wrote:
Although, at the end of the function the `Layout` and `LayoutElements` are used
separately, one to create the target type and the other for the struct type. So
we need to keep them separated.
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
@@ -89,14 +99,57 @@ llvm::TargetExtType
*HLSLBufferLayoutBuilder::createLayoutType(
RecordTypes.pop_back();
for (const auto *FD : RT->getDecl()->fields()) {
- assert((!Packoffsets || Index < Packoffsets->size()) &&
- "number of elements in layout struct does not "
- "match number of packoffset annotations");
+ unsigned FieldOffset = UINT_MAX;
+ llvm::Type *FieldType = nullptr;
+
+ if (Packoffsets) {
+// have packoffset/register(c#) annotations
+assert(Index < Packoffsets->size() &&
+ "number of elements in layout struct does not match number of "
+ "packoffset annotations");
+int PO = (*Packoffsets)[Index++];
+if (PO != -1) {
+ if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
+return nullptr;
+} else {
+ // No packoffset/register(cX) annotation on this field;
+ // Delay the layout until after all of the other elements
+ // annotated with packoffsets/register(cX) are processed.
+ DelayLayoutFields.emplace_back(FD, LayoutElements.size());
+ // reserve space for this field in the layout vector and elements
list
+ Layout.push_back(UINT_MAX);
+ LayoutElements.push_back(nullptr);
+ continue;
+}
+ } else {
+if (!layoutField(FD, EndOffset, FieldOffset, FieldType))
+ return nullptr;
+ }
+
+ assert(FieldOffset != UINT_MAX && FieldType != nullptr);
+ Layout.push_back((unsigned)FieldOffset);
+ LayoutElements.push_back(FieldType);
hekota wrote:
Good idea!
https://github.com/llvm/llvm-project/pull/128991
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [HLSL] Implement explicit layout for default constant buffer ($Globals) (PR #128991)
https://github.com/hekota updated
https://github.com/llvm/llvm-project/pull/128991
>From 9faff902639aece87b72ed5235d71b8b68533074 Mon Sep 17 00:00:00 2001
From: Helena Kotas
Date: Wed, 26 Feb 2025 17:39:16 -0800
Subject: [PATCH 1/6] Add resource binding attribute on $Globals numeric
constants
---
clang/lib/Sema/SemaHLSL.cpp | 10 +--
.../test/AST/HLSL/resource_binding_attr.hlsl | 67 ---
2 files changed, 47 insertions(+), 30 deletions(-)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 283a9801fc707..ffc3ac1b65854 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1446,18 +1446,20 @@ static bool DiagnoseLocalRegisterBinding(Sema &S,
SourceLocation &ArgLoc,
Ty = Ty->getArrayElementTypeNoTypeQual();
// Basic types
- if (Ty->isArithmeticType()) {
+ if (Ty->isArithmeticType() || Ty->isVectorType()) {
bool DeclaredInCOrTBuffer = isa(D->getDeclContext());
if (SpecifiedSpace && !DeclaredInCOrTBuffer)
S.Diag(ArgLoc, diag::err_hlsl_space_on_global_constant);
-if (!DeclaredInCOrTBuffer &&
-(Ty->isIntegralType(S.getASTContext()) || Ty->isFloatingType())) {
- // Default Globals
+if (!DeclaredInCOrTBuffer && (Ty->isIntegralType(S.getASTContext()) ||
+ Ty->isFloatingType() || Ty->isVectorType()))
{
+ // Register annotation on default constant buffer declaration ($Globals)
if (RegType == RegisterType::CBuffer)
S.Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_b);
else if (RegType != RegisterType::C)
S.Diag(ArgLoc, diag::err_hlsl_binding_type_mismatch) << RegTypeNum;
+ else
+return true;
} else {
if (RegType == RegisterType::C)
S.Diag(ArgLoc, diag::warn_hlsl_register_type_c_packoffset);
diff --git a/clang/test/AST/HLSL/resource_binding_attr.hlsl
b/clang/test/AST/HLSL/resource_binding_attr.hlsl
index 6fac903f75e18..7ba4f0f60e83c 100644
--- a/clang/test/AST/HLSL/resource_binding_attr.hlsl
+++ b/clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -1,41 +1,56 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o
- %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library
-finclude-default-header -ast-dump -o - %s | FileCheck %s
-// CHECK:HLSLBufferDecl 0x[[CB:[0-9a-f]+]] {{.*}} line:8:9 cbuffer CB
-// CHECK-NEXT:HLSLResourceClassAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit CBuffer
-// CHECK-NEXT:HLSLResourceAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit CBuffer
-// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "b3" "space2"
-// CHECK-NEXT:VarDecl 0x[[A:[0-9a-f]+]] {{.*}} col:9 used a 'hlsl_constant
float'
+// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 5]]:9 cbuffer CB
+// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK-NEXT: HLSLResourceAttr {{.*}} Implicit CBuffer
+// CHECK-NEXT: HLSLResourceBindingAttr {{.*}} "b3" "space2"
+// CHECK-NEXT: VarDecl {{.*}} used a 'hlsl_constant float'
cbuffer CB : register(b3, space2) {
float a;
}
-// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:17:9 tbuffer TB
-// CHECK-NEXT:HLSLResourceClassAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit SRV
-// CHECK-NEXT:HLSLResourceAttr 0x[[CB:[0-9a-f]+]] {{.*}} Implicit TBuffer
-// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "t2" "space1"
-// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'hlsl_constant
float'
+// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 5]]:9 tbuffer TB
+// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit SRV
+// CHECK-NEXT: HLSLResourceAttr {{.*}} Implicit TBuffer
+// CHECK-NEXT: HLSLResourceBindingAttr {{.*}} "t2" "space1"
+// CHECK-NEXT: VarDecl {{.*}} used b 'hlsl_constant float'
tbuffer TB : register(t2, space1) {
float b;
}
-float foo() {
-// CHECK: BinaryOperator 0x{{[0-9a-f]+}} 'float' '+'
-// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}} 'float'
-// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}} 'hlsl_constant float'
lvalue Var 0x[[A]] 'a' 'hlsl_constant float'
-// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}} 'float'
-// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}} 'hlsl_constant float'
lvalue Var 0x[[B]] 'b' 'hlsl_constant float'
+export float foo() {
return a + b;
}
-// CHECK: VarDecl 0x{{[0-9a-f]+}} <{{.*}}> col:17 UAV
'RWBuffer':'hlsl::RWBuffer' callinit
-// CHECK-NEXT:-CXXConstructExpr 0x{{[0-9a-f]+}}
'RWBuffer':'hlsl::RWBuffer' 'void ()'
-// CHECK-NEXT:-HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "u3" "space0"
+// CHECK: VarDecl {{.*}} UAV 'RWBuffer':'hlsl::RWBuffer'
+// CHECK: HLSLResourceBindingAttr {{.*}} "u3" "space0"
RWBuffer UAV : register(u3);
-// CHECK: -VarDecl 0x{{[0-9a-f]+}} <{{.*}}> col:17 UAV1
'RWBuffer':'hlsl::RWBuffer' callinit
-// CHECK-NEXT:-CXXConstructExpr 0x{{[0-9a-f]+}}
'RWBuffer':'hlsl::RWBuffer' 'void ()'
-// CHECK-NEXT:-HLSLResourceBindingAttr 0x{{[0-9a-f]+}} "u2" "space0"
-// CHECK-NEXT:-VarDecl 0x{{[0-9a-f]+}} col:38 UAV2
