[HLSL] cbuffer: Create host layout structs #122820
Conversation
Creates layout struct for `cbuffer` in Sema which will contains only declarations contributing to the constant buffer layout. Anything else will be filtered out, such as static variables decls, struct and function definitions, resources, or empty struct and zero-sized arrays. If the constant buffer includes a struct that contains any of the above undesirable declarations, a new version of this struct should be created with these declarations filtered out as well. The definition of buffer layour struct is added to the HLSLBufferDecl node and is followed by 'cbuffer` resource handle decl referencing the layout struct as its contained type. Fixes llvm#122553
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-hlsl Author: Helena Kotas (hekota) ChangesCreates layout struct for If the constant buffer includes a struct that contains any of the above undesirable declarations, a new version of this struct should be created with these declarations filtered out as well. The definition of buffer layour struct is added to the HLSLBufferDecl node and is followed by 'cbuffer` resource handle decl referencing the layout struct as its contained type. Fixes #122553 Patch is 27.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122820.diff 9 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 9c470f09406378..0a66ed3d499ff2 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4967,6 +4967,7 @@ class HLSLBufferDecl final : public NamedDecl, public DeclContext {
SourceLocation getRBraceLoc() const { return RBraceLoc; }
void setRBraceLoc(SourceLocation L) { RBraceLoc = L; }
bool isCBuffer() const { return IsCBuffer; }
+ const Type *getResourceHandleType() const;
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 31749e46458d6a..a4f5d1a3a71a63 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5693,6 +5693,19 @@ HLSLBufferDecl *HLSLBufferDecl::CreateDeserialized(ASTContext &C,
SourceLocation(), SourceLocation());
}
+const Type *HLSLBufferDecl::getResourceHandleType() const {
+ // Resource handle is the last decl in the HLSLBufferDecl.
+ // If it is not present, it probably means the buffer is empty.
+ if (VarDecl *VD = llvm::dyn_cast_or_null<VarDecl>(LastDecl)) {
+ const Type *Ty = VD->getType().getTypePtr();
+ if (Ty->isHLSLAttributedResourceType()) {
+ assert(VD->getNameAsString() == "__handle");
+ return Ty;
+ }
+ }
+ return nullptr;
+}
+
//===----------------------------------------------------------------------===//
// ImportDecl Implementation
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 5679bd71581795..51e20ad43fcc8d 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -159,10 +159,12 @@ void CGHLSLRuntime::addConstant(VarDecl *D, Buffer &CB) {
CB.Constants.emplace_back(std::make_pair(GV, LowerBound));
}
-void CGHLSLRuntime::addBufferDecls(const DeclContext *DC, Buffer &CB) {
- for (Decl *it : DC->decls()) {
+void CGHLSLRuntime::addBufferDecls(const HLSLBufferDecl *D, Buffer &CB) {
+ for (Decl *it : D->decls()) {
if (auto *ConstDecl = dyn_cast<VarDecl>(it)) {
- addConstant(ConstDecl, CB);
+ if (ConstDecl->getType().getTypePtr() != D->getResourceHandleType()) {
+ addConstant(ConstDecl, CB);
+ }
} else if (isa<CXXRecordDecl, EmptyDecl>(it)) {
// Nothing to do for this declaration.
} else if (isa<FunctionDecl>(it)) {
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 00e110e8e6fa27..870a5bd0ea6b4f 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -169,7 +169,7 @@ class CGHLSLRuntime {
llvm::hlsl::ElementType ET,
BufferResBinding &Binding);
void addConstant(VarDecl *D, Buffer &CB);
- void addBufferDecls(const DeclContext *DC, Buffer &CB);
+ void addBufferDecls(const HLSLBufferDecl *D, Buffer &CB);
llvm::Triple::ArchType getArch();
llvm::SmallVector<Buffer> Buffers;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 65ddee05a21512..c726672c0118e0 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -21,10 +21,12 @@
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Sema/Initialization.h"
+#include "clang/Sema/Lookup.h"
#include "clang/Sema/ParsedAttr.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/Template.h"
@@ -32,11 +34,13 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/DXILABI.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/TargetParser/Triple.h"
#include <iterator>
+#include <string>
#include <utility>
using namespace clang;
@@ -253,12 +257,253 @@ static void validatePackoffset(Sema &S, HLSLBufferDecl *BufDecl) {
}
}
+// Returns true if the array has a zero size = if any of the dimensions is 0
+static bool isZeroSizedArray(const ConstantArrayType *CAT) {
+ while (CAT && !CAT->isZeroSize())
+ CAT = dyn_cast<ConstantArrayType>(
+ CAT->getElementType()->getUnqualifiedDesugaredType());
+ return CAT != nullptr;
+}
+
+// Returns true if the struct can be used inside HLSL Buffer which means
+// that it does not contain intangible types, empty structs, zero-sized arrays,
+// and the same is true for its base or embedded structs.
+bool isStructHLSLBufferCompatible(const CXXRecordDecl *RD) {
+ if (RD->getTypeForDecl()->isHLSLIntangibleType() ||
+ (RD->field_empty() && RD->getNumBases() == 0))
+ return false;
+ // check fields
+ for (const FieldDecl *Field : RD->fields()) {
+ QualType Ty = Field->getType();
+ if (Ty->isRecordType()) {
+ if (!isStructHLSLBufferCompatible(Ty->getAsCXXRecordDecl()))
+ return false;
+ } else if (Ty->isConstantArrayType()) {
+ if (isZeroSizedArray(cast<ConstantArrayType>(Ty)))
+ return false;
+ }
+ }
+ // check bases
+ for (const CXXBaseSpecifier &Base : RD->bases())
+ if (!isStructHLSLBufferCompatible(Base.getType()->getAsCXXRecordDecl()))
+ return false;
+ return true;
+}
+
+static CXXRecordDecl *findRecordDecl(Sema &S, IdentifierInfo *II,
+ DeclContext *DC) {
+ DeclarationNameInfo NameInfo =
+ DeclarationNameInfo(DeclarationName(II), SourceLocation());
+ LookupResult R(S, NameInfo, Sema::LookupOrdinaryName);
+ S.LookupName(R, S.getScopeForContext(DC));
+ if (R.isSingleResult())
+ return R.getAsSingle<CXXRecordDecl>();
+ return nullptr;
+}
+
+// Creates a name for buffer layout struct using the provide name base.
+// If the name must be unique (not previously defined), a suffix is added
+// until a unique name is found.
+static IdentifierInfo *getHostLayoutStructName(Sema &S,
+ IdentifierInfo *NameBaseII,
+ bool MustBeUnique,
+ DeclContext *DC) {
+ ASTContext &AST = S.getASTContext();
+ std::string NameBase;
+ if (NameBaseII) {
+ NameBase = NameBaseII->getName().str();
+ } else {
+ // anonymous struct
+ NameBase = "anon";
+ MustBeUnique = true;
+ }
+
+ std::string Name = "__hostlayout.struct." + NameBase;
+ IdentifierInfo *II = &AST.Idents.get(Name, tok::TokenKind::identifier);
+ if (!MustBeUnique)
+ return II;
+
+ unsigned suffix = 0;
+ while (true) {
+ if (suffix != 0)
+ II = &AST.Idents.get((llvm::Twine(Name) + "." + Twine(suffix)).str(),
+ tok::TokenKind::identifier);
+ if (!findRecordDecl(S, II, DC))
+ return II;
+ // declaration with that name already exists - increment suffix and try
+ // again until unique name is found
+ suffix++;
+ };
+}
+
+// Returns true if the record type is an HLSL resource class
+static bool isResourceRecordType(const Type *Ty) {
+ return HLSLAttributedResourceType::findHandleTypeOnResource(Ty) != nullptr;
+}
+
+static CXXRecordDecl *createHostLayoutStruct(Sema &S, CXXRecordDecl *StructDecl,
+ HLSLBufferDecl *BufDecl);
+
+// Creates a field declaration of given name and type for HLSL buffer layout
+// struct. Returns nullptr if the type cannot be use in HLSL Buffer layout.
+static FieldDecl *createFieldForHostLayoutStruct(Sema &S, const Type *Ty,
+ IdentifierInfo *II,
+ CXXRecordDecl *LayoutStruct,
+ HLSLBufferDecl *BufDecl) {
+ if (Ty->isRecordType()) {
+ if (isResourceRecordType(Ty))
+ return nullptr;
+ CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
+ if (!isStructHLSLBufferCompatible(RD)) {
+ RD = createHostLayoutStruct(S, RD, BufDecl);
+ if (!RD)
+ return nullptr;
+ Ty = RD->getTypeForDecl();
+ }
+ } else if (Ty->isConstantArrayType()) {
+ if (isZeroSizedArray(cast<ConstantArrayType>(Ty)))
+ return nullptr;
+ }
+ QualType QT = QualType(Ty, 0);
+ ASTContext &AST = S.getASTContext();
+ TypeSourceInfo *TSI = AST.getTrivialTypeSourceInfo(QT, SourceLocation());
+ auto *Field = FieldDecl::Create(AST, LayoutStruct, SourceLocation(),
+ SourceLocation(), II, QT, TSI, nullptr, false,
+ InClassInitStyle::ICIS_NoInit);
+ Field->setAccess(AccessSpecifier::AS_private);
+ return Field;
+}
+
+// Creates host layout struct for a struct included in HLSL Buffer.
+// The layout struct will include only fields that are allowed in HLSL buffer.
+// These fields will be filtered out:
+// - resource classes
+// - empty structs
+// - zero-sized arrays
+// Returns nullptr if the resulting layout struct would be empty.
+static CXXRecordDecl *createHostLayoutStruct(Sema &S, CXXRecordDecl *StructDecl,
+ HLSLBufferDecl *BufDecl) {
+ assert(!isStructHLSLBufferCompatible(StructDecl) &&
+ "struct is already HLSL buffer compatible");
+
+ ASTContext &AST = S.getASTContext();
+ DeclContext *DC = StructDecl->getDeclContext();
+ IdentifierInfo *II = getHostLayoutStructName(
+ S, StructDecl->getIdentifier(), false, BufDecl->getDeclContext());
+
+ // reuse existing if the layout struct if it already exists
+ if (CXXRecordDecl *RD = findRecordDecl(S, II, DC))
+ return RD;
+
+ CXXRecordDecl *LS =
+ CXXRecordDecl::Create(AST, TagDecl::TagKind::Class, BufDecl,
+ SourceLocation(), SourceLocation(), II);
+ LS->setImplicit(true);
+ LS->startDefinition();
+
+ // copy base struct, create HLSL Buffer compatible version if needed
+ if (unsigned NumBases = StructDecl->getNumBases()) {
+ assert(NumBases == 1 && "HLSL supports only one base type");
+ CXXBaseSpecifier Base = *StructDecl->bases_begin();
+ CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
+ if (!isStructHLSLBufferCompatible(BaseDecl)) {
+ BaseDecl = createHostLayoutStruct(S, BaseDecl, BufDecl);
+ if (BaseDecl) {
+ TypeSourceInfo *TSI = AST.getTrivialTypeSourceInfo(
+ QualType(BaseDecl->getTypeForDecl(), 0));
+ Base = CXXBaseSpecifier(SourceRange(), false, StructDecl->isClass(),
+ AS_none, TSI, SourceLocation());
+ }
+ }
+ if (BaseDecl) {
+ const CXXBaseSpecifier *BasesArray[1] = {&Base};
+ LS->setBases(BasesArray, 1);
+ }
+ }
+
+ // filter struct fields
+ for (const FieldDecl *FD : StructDecl->fields()) {
+ const Type *Ty = FD->getType()->getUnqualifiedDesugaredType();
+ if (FieldDecl *NewFD = createFieldForHostLayoutStruct(
+ S, Ty, FD->getIdentifier(), LS, BufDecl))
+ LS->addDecl(NewFD);
+ }
+ LS->completeDefinition();
+
+ if (LS->field_empty() && LS->getNumBases() == 0)
+ return nullptr;
+ BufDecl->addDecl(LS);
+ return LS;
+}
+
+// Creates host layout struct for HLSL Buffer. The struct will include only
+// fields of types that are allowed in HLSL buffer and it will filter out:
+// - static variable declarations
+// - resource classes
+// - empty structs
+// - zero-sized arrays
+// - non-variable declarations
+static CXXRecordDecl *createHostLayoutStructForBuffer(Sema &S,
+ HLSLBufferDecl *BufDecl) {
+ ASTContext &AST = S.getASTContext();
+ IdentifierInfo *II = getHostLayoutStructName(S, BufDecl->getIdentifier(),
+ true, BufDecl->getDeclContext());
+
+ CXXRecordDecl *LS =
+ CXXRecordDecl::Create(AST, TagDecl::TagKind::Class, BufDecl,
+ SourceLocation(), SourceLocation(), II);
+ LS->setImplicit(true);
+ LS->startDefinition();
+
+ for (const Decl *D : BufDecl->decls()) {
+ const VarDecl *VD = dyn_cast<VarDecl>(D);
+ if (!VD || VD->getStorageClass() == SC_Static)
+ continue;
+ const Type *Ty = VD->getType()->getUnqualifiedDesugaredType();
+ if (FieldDecl *FD = createFieldForHostLayoutStruct(
+ S, Ty, VD->getIdentifier(), LS, BufDecl))
+ LS->addDecl(FD);
+ }
+ LS->completeDefinition();
+ BufDecl->addDecl(LS);
+ return LS;
+}
+
+// Creates a "__handle" declaration for the HLSL Buffer type
+// with the corresponding HLSL resource type and adds it to the HLSLBufferDecl
+static void createHLSLBufferHandle(Sema &S, HLSLBufferDecl *BufDecl,
+ CXXRecordDecl *LayoutStruct) {
+ ASTContext &AST = S.getASTContext();
+
+ HLSLAttributedResourceType::Attributes ResAttrs(
+ BufDecl->isCBuffer() ? ResourceClass::CBuffer : ResourceClass::SRV, false,
+ false);
+ QualType ResHandleTy = AST.getHLSLAttributedResourceType(
+ AST.HLSLResourceTy, QualType(LayoutStruct->getTypeForDecl(), 0),
+ ResAttrs);
+
+ IdentifierInfo *II = &AST.Idents.get("__handle", tok::TokenKind::identifier);
+ VarDecl *VD = VarDecl::Create(
+ BufDecl->getASTContext(), BufDecl, SourceLocation(), SourceLocation(), II,
+ ResHandleTy, AST.getTrivialTypeSourceInfo(ResHandleTy, SourceLocation()),
+ SC_None);
+ BufDecl->addDecl(VD);
+}
+
+// Handle end of cbuffer/tbuffer declaration
void SemaHLSL::ActOnFinishBuffer(Decl *Dcl, SourceLocation RBrace) {
auto *BufDecl = cast<HLSLBufferDecl>(Dcl);
BufDecl->setRBraceLoc(RBrace);
validatePackoffset(SemaRef, BufDecl);
+ // create buffer layout struct
+ CXXRecordDecl *LayoutStruct =
+ createHostLayoutStructForBuffer(SemaRef, BufDecl);
+
+ // create buffer resource handle
+ createHLSLBufferHandle(SemaRef, BufDecl, LayoutStruct);
+
SemaRef.PopDeclContext();
}
diff --git a/clang/test/AST/HLSL/ast-dump-comment-cbuffe-tbufferr.hlsl b/clang/test/AST/HLSL/ast-dump-comment-cbuffe-tbufferr.hlsl
index e6a2ea7c6d2dc6..5956eef27205d5 100644
--- a/clang/test/AST/HLSL/ast-dump-comment-cbuffe-tbufferr.hlsl
+++ b/clang/test/AST/HLSL/ast-dump-comment-cbuffe-tbufferr.hlsl
@@ -44,7 +44,13 @@ tbuffer B {
// AST-NEXT:`-ParagraphComment {{.*}}<col:4, col:17>
// AST-NEXT:`-TextComment {{.*}}<col:4, col:17> Text=" CBuffer decl."
// AST-NEXT:-VarDecl {{.*}}<line:15:5, col:11> col:11 a 'float'
-// AST-NEXT:`-VarDecl {{.*}}<line:19:5, col:9> col:9 b 'int'
+// AST-NEXT:-VarDecl {{.*}}<line:19:5, col:9> col:9 b 'int'
+// AST-NEXT:CXXRecordDecl 0x[[CB:[0-9a-f]+]] {{.*}} implicit class __hostlayout.struct.A definition
+// AST:FieldDecl 0x[[CB:[0-9a-f]+]] {{.*}} a 'float'
+// AST-NEXT:FieldDecl 0x[[CB:[0-9a-f]+]] {{.*}} b 'int'
+// AST-NEXT:VarDecl 0x[[CB:[0-9a-f]+]] {{.*}} __handle '__hlsl_resource_t
+// AST-SAME{LITERAL}: [[hlsl::resource_class(CBuffer)]] [[hlsl::contained_type(__hostlayout.struct.A)]]'
+
// AST-NEXT:HLSLBufferDecl {{.*}}<line:29:1, line:38:1> line:29:9 tbuffer B
// AST-NEXT:-HLSLResourceClassAttr {{.*}} <<invalid sloc>> Implicit SRV
// AST-NEXT:-HLSLResourceAttr {{.*}} <<invalid sloc>> Implicit TBuffer
@@ -52,4 +58,9 @@ tbuffer B {
// AST-NEXT: `-ParagraphComment {{.*}}<col:4, col:17>
// AST-NEXT: `-TextComment {{.*}}<col:4, col:17> Text=" TBuffer decl."
// AST-NEXT:-VarDecl {{.*}}<line:33:5, col:11> col:11 c 'float'
-// AST-NEXT:`-VarDecl {{.*}} <line:37:5, col:9> col:9 d 'int'
+// AST-NEXT:-VarDecl {{.*}} <line:37:5, col:9> col:9 d 'int'
+// AST-NEXT:CXXRecordDecl 0x[[CB:[0-9a-f]+]] {{.*}} implicit class __hostlayout.struct.B definition
+// AST:FieldDecl 0x[[CB:[0-9a-f]+]] {{.*}} c 'float'
+// AST-NEXT:FieldDecl 0x[[CB:[0-9a-f]+]] {{.*}} d 'int'
+// AST-NEXT:VarDecl 0x[[CB:[0-9a-f]+]] {{.*}} __handle '__hlsl_resource_t
+// AST-SAME{LITERAL}: [[hlsl::resource_class(SRV)]] [[hlsl::contained_type(__hostlayout.struct.B)]]'
diff --git a/clang/test/AST/HLSL/cbuffer.hlsl b/clang/test/AST/HLSL/cbuffer.hlsl
new file mode 100644
index 00000000000000..b47ea930c1b57f
--- /dev/null
+++ b/clang/test/AST/HLSL/cbuffer.hlsl
@@ -0,0 +1,217 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -ast-dump -o - %s | FileCheck %s
+
+struct EmptyStruct {
+};
+
+struct A {
+ float a;
+};
+
+struct B {
+ RWBuffer<float> buf;
+ EmptyStruct es;
+ float ea[0];
+ float a;
+};
+
+struct C {
+ EmptyStruct es;
+};
+
+typedef B BTypedef;
+typedef C CTypedef;
+
+struct D : B {
+ float b;
+};
+
+struct E : EmptyStruct {
+ float c;
+};
+
+struct F : A {
+ int ae[0];
+};
+
+typedef float EmptyArrayTypedef[10][0];
+
+// CHECK: HLSLBufferDecl {{.*}} line:41:9 cbuffer CB
+// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
+cbuffer CB {
+ // CHECK: VarDecl {{.*}} col:9 used a1 'float'
+ float a1;
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class __hostlayout.struct.CB definition
+ // CHECK: FieldDecl {{.*}} a1 'float'
+ // CHECK: VarDecl {{.*}} __handle '__hlsl_resource_t
+ // CHECK-SAME{LITERAL}: [[hlsl::resource_class(CBuffer)]] [[hlsl::contained_type(__hostlayout.struct.CB)]]'
+}
+
+// Check that buffer layout struct does not include resources or empty types
+// CHECK: HLSLBufferDecl {{.*}} line:55:9 cbuffer CB
+// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
+cbuffer CB {
+ // CHECK: VarDecl {{.*}} col:9 used a2 'float'
+ float a2;
+ // CHECK: VarDecl {{.*}} col:19 b2 'RWBuffer<float>':'hlsl::RWBuffer<float>'
+ RWBuffer<float> b2;
+ // CHECK: VarDecl {{.*}} col:15 c2 'EmptyStruct'
+ EmptyStruct c2;
+ // CHECK: VarDecl {{.*}} col:9 d2 'float[0]'
+ float d2[0];
+ // CHECK: VarDecl {{.*}} col:9 e2 'float'
+ float e2;
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class __hostlayout.struct.CB.1 definition
+ // CHECK: FieldDecl {{.*}} a2 'float'
+ // CHECK-NEXT: FieldDecl {{.*}} e2 'float'
+ // CHECK: VarDecl {{.*}} __handle '__hlsl_resource_t
+ // CHECK-SAME{LITERAL}: [[hlsl::resource_class(CBuffer)]] [[hlsl::contained_type(__hostlayout.struct.CB.1)]]'
+}
+
+// Check that layout struct is created for B and the empty struct C is removed
+// CHECK: HLSLBufferDecl {{.*}} line:78:9 cbuffer CB
+// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
+cbuffer CB {
+ // CHECK: VarDecl {{.*}} col:5 used s1 'A'
+ A s1;
+ // CHECK: VarDecl {{.*}} col:5 s2 'B'
+ B s2;
+ // CHECK: VarDecl {{.*}} col:12 s3 'CTypedef':'C
+ CTypedef s3;
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class __hostlayout.struct.B definition
+ // CHECK: FieldDecl {{.*}} a 'float'
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class __hostlayout.struct.CB.2 definition
+ // CHECK: FieldDecl {{.*}} s1 'A'
+ // CHECK: FieldDecl {{.*}} s2 '__hostlayout.struct.B'
+ // CHECK-NEXT: VarDecl {{.*}} __handle '__hlsl_resource_t
+ // CHECK-SAME{LITERAL}: [[hlsl::resource_class(CBuffer)]] [[hlsl::contained_type(__hostlayout.struct.CB.2)]]'
+}
+
+// check that layout struct is created for D because of its base struct
+// CHECK: HLSLBufferDecl {{.*}} line:100:9 cbuffer CB
+// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
+cbuffer CB {
+ // CHECK: VarDecl {{.*}} s4 'D'
+ D s4;
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class __hostlayout.struct.D definition
+ // CHECK: FieldDecl {{.*}} b 'float'
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class __hostlayout.struct.CB.3 definition
+ // CHECK: FieldDecl {{.*}} s4 '__hostlayout.struct.D'
+ // CHECK: VarDecl {{.*}} __handle '__hlsl_resource_t
+ // CHECK-SAME{LITERAL}: [[hlsl::resource_class(CBuffer)]] [[hlsl::contained_type(__hostlayout.struct.CB.3)]]'
+}
+
+// check that layout struct is created for E because because its base struct
+// is empty and should be eliminated, and BTypedef should reuse the previously
+// defined '__hostlayout.struct.B'
+// CHECK: HLSLBufferDecl {{.*}} line:119:9 cbuffer CB
+// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
+// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
+cbuffer CB {
+ // CHECK: VarDecl {{.*}} s5 'E'
+ E s5;
+ // CHECK: VarDecl {{.*}} s6 'BTypedef':'B'
+ BTypedef s6;
+
+ // CHECK: CXXRecordDecl {{.*}} implicit class _...
[truncated]
|
There was a problem hiding this comment.
The one design question I have here is: should we be bothering to create a handle for the cbuffer in the frontend at all?
My thought had been that we would instead emit the cbuffer member declarations as constant address space (2 for DXIL) declarations and have a pass to collect them and replace them with accesses off a handle that we generate in the backend.
Then we just need to generate metadata in the frontend for the buffer's contained type, and which global variables are bound to which cbuffer.
I was kind of hoping we might be able to generate the accesses off a handle during codegen when the global is accessed. I'm not sure if that's possible though. |
I'm concerned that this will be more complex to make work with the other CBV syntaxes. For example, the implicit globals don't have a bufferdecl to attach to, so we'll need an extra implicit decl to attach them to, and the If instead we treat CBV as a storage class with an associated address space (similar to |
Use "__layout." prefix instead of "__hostlayout.struct." to make the text output more concise for both AST and LLVM IR.
hekota
changed the title
clang/lib/Sema/SemaHLSL.cpp
Outdated
| MustBeUnique = true; | ||
| } | ||
|
|
||
| std::string Name = "__layout_" + NameBase; |
There was a problem hiding this comment.
Could we use a prefix/suffix that is unspellable in HLSL syntax? Such as a .host_layout suffix or something?
There was a problem hiding this comment.
That's what I had originally, but having the name spellable enables additional layer of testing with using static asserts:
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_1), "");
The name starts with 2 underscores and that means it is reserved. Noone should be using it by accident.
clang/lib/Sema/SemaHLSL.cpp
Outdated
| LookupResult R(S, NameInfo, Sema::LookupOrdinaryName); | ||
| S.LookupName(R, S.getScopeForContext(DC)); | ||
| if (R.isSingleResult()) | ||
| return R.getAsSingle<CXXRecordDecl>(); |
There was a problem hiding this comment.
Could this be multiple results if the name is defined in multiple accessible scopes (like current and parent scopes)? Or does it only return zero or one result from the specified scope?
There was a problem hiding this comment.
This should only return zero or one result in the specified scope, which for the HLSLBufferDecl is the global scope. I will update the LookupNameKind to LookupTagName though to be more specific that the lookup is for a record/struct.
There was a problem hiding this comment.
Ok, I don't see cases like this tested:
namespace NS1 {
struct Foo { ... };
struct Bar { struct Foo { ... }; ... };
}
struct Foo { ... };
...
cbuffer CB1 {
Foo foo1;
NS1::Foo foo2;
NS1::Bar::Foo foo3;
}
namespace NS2 {
struct Foo { ... };
cbuffer CB2 {
::Foo foo0;
Foo foo1;
NS1::Foo foo2;
NS1::Bar::Foo foo3;
}
}In what decl context will implicit host layout decls be created for each type? Will there be duplicates?
There was a problem hiding this comment.
I see what you mean now. I have added a separate cbuffer AST test with namespaces and changed the implementation to always create the layout struct on the same context as the original struct so that it will be in the same namespace. The lookup is needs to scan just the immediate non-transparent decl context.
clang/lib/Sema/SemaHLSL.cpp
Outdated
| CXXRecordDecl *LayoutStruct, | ||
| HLSLBufferDecl *BufDecl) { | ||
| if (Ty->isRecordType()) { | ||
| if (isResourceRecordType(Ty)) |
There was a problem hiding this comment.
Won't we want to catch more intangible types than just resources here? How do we make sure this is kept up-to-date for all intangible types?
On a related note, how do we ensure this code under createHostLayoutStruct always has matching/consistent logic to the code in requiresImplicitBufferLayoutStructure? Could there be a way to share the key logic components perhaps? For instance, there could be functions that return whether a leaf type is intangible (maybe this is just Type::IsHLSLIntangibleType), or is zero-sized, separate from the logic that recursively traverses a record type looking for these properties on field types. The IsHLSLIntangibleType and zero-sized detection functions are then used by both traversal paths to ensure consistent behavior.
There was a problem hiding this comment.
Oops, replied in wrong thread. Edited to move.
There was a problem hiding this comment.
There is an assert at the top of the createHostLayoutStruct function that will make sure the logic is consistent:
assert(requiresImplicitBufferLayoutStructure(StructDecl) && "struct is already HLSL buffer compatible");
I have also moved the checking into a shared function isInvalidConstantBufferLeafElementType. Currently the only intangible types are resources classes, but I added a check for builtin resource type as well.
|
|
||
| // copy base struct, create HLSL Buffer compatible version if needed | ||
| if (unsigned NumBases = StructDecl->getNumBases()) { | ||
| assert(NumBases == 1 && "HLSL supports only one base type"); |
There was a problem hiding this comment.
HLSL actually supports multiple interface bases (abstract bases), though we don't reliably implement these in DXC.
There was a problem hiding this comment.
I don't think it is accurate to say HLSL supports multiple bases. The interface support in DXC is HLSL 2015-only, we don't do anything sane with it in the language versions that DXC actually supports compiling for.
There was a problem hiding this comment.
The interface support in DXC is HLSL 2015-only
Not entirely true. We don't support use of interface instances (basically like pointers), but we do support interface definitions, inheritance from such, and overriding method implementations.
There is a bug in DXC when calling a method originally defined in an interface that is not inherited first in a multiple-inheritance scenario due to the way the MicrosoftCXXABI generates the code to access this for a method which we inherited by defaulting to that ABI in DXC.
There was a problem hiding this comment.
Ok, so there can be multiple interfaces, but only one base struct, correct? And interfaces have only methods and no data members, so they can be safely ignored for the layout struct.
There was a problem hiding this comment.
And Clang does not support interfaces yet.
There was a problem hiding this comment.
I have filed an issue and will add a FIXME to the code.
There was a problem hiding this comment.
It looks like we might not be supporting interfaces in Clang, so I'll remove the FIXME.
- create common function isInvalidConstantBufferLeafElementType - create layout structs in the same context as the original struct - fix lookup to scan just the first non-transparent context - add test case with namespaces
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Creates layout struct for
cbufferin Sema which will contains only declarations contributing to the constant buffer layout. Anything else will be filtered out, such as static variables decls, struct and function definitions, resources, or empty struct and zero-sized arrays.If the constant buffer includes a struct that contains any of the above undesirable declarations, a new version of this struct should be created with these declarations filtered out as well.
The definition of buffer layout struct will be added to the HLSLBufferDecl AST node as the last node. Any layout structs for embedded structures will be added there as well.
Fixes #122553