-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[HLSL] cbuffer: Create host layout structs #122820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
71ddb5a
b88886b
07ec283
fc939db
0233994
75edd2b
bbdd56b
80fc426
a25ac0a
45ab868
aaf83f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,22 +21,26 @@ | |
#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" | ||
#include "llvm/ADT/STLExtras.h" | ||
#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,229 @@ 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 contains at least one element that prevents it | ||
// from being included inside HLSL Buffer as is, such as an intangible type, | ||
// empty struct, or zero-sized array. If it does, a new implicit layout struct | ||
// needs to be created for HLSL Buffer use that will exclude these unwanted | ||
// declarations (see createHostLayoutStruct function). | ||
static bool requiresImplicitBufferLayoutStructure(const CXXRecordDecl *RD) { | ||
if (RD->getTypeForDecl()->isHLSLIntangibleType() || RD->isEmpty()) | ||
return true; | ||
// check fields | ||
for (const FieldDecl *Field : RD->fields()) { | ||
QualType Ty = Field->getType(); | ||
if (Ty->isRecordType()) { | ||
if (requiresImplicitBufferLayoutStructure(Ty->getAsCXXRecordDecl())) | ||
return true; | ||
} else if (Ty->isConstantArrayType()) { | ||
if (isZeroSizedArray(cast<ConstantArrayType>(Ty))) | ||
return true; | ||
} | ||
} | ||
// check bases | ||
for (const CXXBaseSpecifier &Base : RD->bases()) | ||
if (requiresImplicitBufferLayoutStructure( | ||
Base.getType()->getAsCXXRecordDecl())) | ||
return true; | ||
return false; | ||
} | ||
|
||
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; | ||
hekota marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (NameBaseII) { | ||
NameBase = NameBaseII->getName().str(); | ||
} else { | ||
// anonymous struct | ||
NameBase = "anon"; | ||
MustBeUnique = true; | ||
} | ||
|
||
std::string Name = "__layout_" + NameBase; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we use a prefix/suffix that is unspellable in HLSL syntax? Such as a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's what I had originally, but having the name spellable enables additional layer of testing with using static asserts:
The name starts with 2 underscores and that means it is reserved. Noone should be using it by accident. |
||
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, | ||
hekota marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, replied in wrong thread. Edited to move. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is an assert at the top of the
I have also moved the checking into a shared function |
||
return nullptr; | ||
CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); | ||
if (requiresImplicitBufferLayoutStructure(RD)) { | ||
RD = createHostLayoutStruct(S, RD, BufDecl); | ||
if (!RD) | ||
return nullptr; | ||
Ty = RD->getTypeForDecl(); | ||
} | ||
} else if (Ty->isConstantArrayType()) { | ||
hekota marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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(requiresImplicitBufferLayoutStructure(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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HLSL actually supports multiple There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it is accurate to say HLSL supports multiple bases. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And Clang does not support interfaces yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have filed an issue and will add a FIXME to the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we might not be supporting interfaces in Clang, so I'll remove the FIXME. |
||
CXXBaseSpecifier Base = *StructDecl->bases_begin(); | ||
CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl(); | ||
if (requiresImplicitBufferLayoutStructure(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 | ||
// The layour struct will be added to the HLSLBufferDecl declarations. | ||
void 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); | ||
} | ||
|
||
// 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 | ||
createHostLayoutStructForBuffer(SemaRef, BufDecl); | ||
|
||
SemaRef.PopDeclContext(); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only return zero or one result in the specified scope, which for the
HLSLBufferDecl
is the global scope. I will update theLookupNameKind
toLookupTagName
though to be more specific that the lookup is for a record/struct.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I don't see cases like this tested:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.