-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[HLSL] Add static methods for resource initialization #155866
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
Conversation
Reorder the arguments of handle initialization builtins to match the order of the llvm intrinsics, and also to match the arguments on the static create methods for resources (coming soon).
✅ With the latest revision this PR passed the C/C++ code formatter. |
…or from handle Adds static methods `__createFromBinding` and `__createFromImplicitBinding` to resource classes. These methods will be used for resource initialization instead of the resource constructors that take binding information. Also adds a private resource constructor that takes an initialized resource handle. This constructor will be called from the static create methods.
e5cfb97
to
dcbbda4
Compare
…create-0-reorder-builtin-args
…' of https://github.com/llvm/llvm-project into res-create-1-add-methods
…create-1-add-methods
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Helena Kotas (hekota) ChangesAdds static methods Also adds a private resource constructor that takes an initialized resource handle which is used by the static create methods. Depends on #155861 Part 1 of #154221 Patch is 27.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/155866.diff 6 Files Affected:
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
index 7830cdd18c6cd..a4d75155d8511 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
@@ -19,6 +19,7 @@
#include "clang/AST/Expr.h"
#include "clang/AST/Type.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaHLSL.h"
@@ -110,8 +111,11 @@ struct BuiltinTypeMethodBuilder {
CXXMethodDecl *Method;
bool IsConst;
bool IsCtor;
+ AccessSpecifier Access;
+ StorageClass SC;
llvm::SmallVector<Param> Params;
llvm::SmallVector<Stmt *> StmtsList;
+ llvm::SmallVector<VarDecl *> LocalVars;
// Argument placeholders, inspired by std::placeholder. These are the indices
// of arguments to forward to `callBuiltin` and other method builder methods.
@@ -120,7 +124,16 @@ struct BuiltinTypeMethodBuilder {
// LastStmt - refers to the last statement in the method body; referencing
// LastStmt will remove the statement from the method body since
// it will be linked from the new expression being constructed.
- enum class PlaceHolder { _0, _1, _2, _3, _4, Handle = 128, LastStmt };
+ enum class PlaceHolder {
+ _0,
+ _1,
+ _2,
+ _3,
+ _4,
+ LocalVar_0 = 64,
+ Handle = 128,
+ LastStmt
+ };
Expr *convertPlaceholder(PlaceHolder PH);
Expr *convertPlaceholder(Expr *E) { return E; }
@@ -130,13 +143,17 @@ struct BuiltinTypeMethodBuilder {
BuiltinTypeMethodBuilder(BuiltinTypeDeclBuilder &DB, DeclarationName &Name,
QualType ReturnTy, bool IsConst = false,
- bool IsCtor = false)
+ bool IsCtor = false,
+ AccessSpecifier Access = AS_public,
+ StorageClass SC = SC_None)
: DeclBuilder(DB), Name(Name), ReturnTy(ReturnTy), Method(nullptr),
- IsConst(IsConst), IsCtor(IsCtor) {}
+ IsConst(IsConst), IsCtor(IsCtor), Access(Access), SC(SC) {}
BuiltinTypeMethodBuilder(BuiltinTypeDeclBuilder &DB, StringRef NameStr,
QualType ReturnTy, bool IsConst = false,
- bool IsCtor = false);
+ bool IsCtor = false,
+ AccessSpecifier Access = AS_public,
+ StorageClass SC = SC_None);
BuiltinTypeMethodBuilder(const BuiltinTypeMethodBuilder &Other) = delete;
~BuiltinTypeMethodBuilder() { finalize(); }
@@ -147,12 +164,19 @@ struct BuiltinTypeMethodBuilder {
BuiltinTypeMethodBuilder &addParam(StringRef Name, QualType Ty,
HLSLParamModifierAttr::Spelling Modifier =
HLSLParamModifierAttr::Keyword_in);
+ BuiltinTypeMethodBuilder &createLocalVar(StringRef Name, QualType Ty);
template <typename... Ts>
BuiltinTypeMethodBuilder &callBuiltin(StringRef BuiltinName,
QualType ReturnType, Ts... ArgSpecs);
template <typename TLHS, typename TRHS>
BuiltinTypeMethodBuilder &assign(TLHS LHS, TRHS RHS);
template <typename T> BuiltinTypeMethodBuilder &dereference(T Ptr);
+ template <typename T>
+ BuiltinTypeMethodBuilder &getResourceHandle(T ResourceRecord);
+ template <typename TResource, typename TValue>
+ BuiltinTypeMethodBuilder &setHandleFieldOnResource(TResource ResourceRecord,
+ TValue HandleValue);
+ template <typename T> BuiltinTypeMethodBuilder &returnValue(T ReturnValue);
BuiltinTypeDeclBuilder &finalize();
Expr *getResourceHandleExpr();
@@ -328,6 +352,17 @@ Expr *BuiltinTypeMethodBuilder::convertPlaceholder(PlaceHolder PH) {
}
ASTContext &AST = DeclBuilder.SemaRef.getASTContext();
+ if (PH >= PlaceHolder::LocalVar_0) {
+ unsigned Index = static_cast<unsigned>(PH) -
+ static_cast<unsigned>(PlaceHolder::LocalVar_0);
+ assert(Index < LocalVars.size() && "local var index out of range");
+ VarDecl *VD = LocalVars[Index];
+ return DeclRefExpr::Create(
+ AST, NestedNameSpecifierLoc(), SourceLocation(), VD, false,
+ DeclarationNameInfo(VD->getDeclName(), SourceLocation()), VD->getType(),
+ VK_LValue);
+ }
+
ParmVarDecl *ParamDecl = Method->getParamDecl(static_cast<unsigned>(PH));
return DeclRefExpr::Create(
AST, NestedNameSpecifierLoc(), SourceLocation(), ParamDecl, false,
@@ -335,12 +370,11 @@ Expr *BuiltinTypeMethodBuilder::convertPlaceholder(PlaceHolder PH) {
ParamDecl->getType(), VK_PRValue);
}
-BuiltinTypeMethodBuilder::BuiltinTypeMethodBuilder(BuiltinTypeDeclBuilder &DB,
- StringRef NameStr,
- QualType ReturnTy,
- bool IsConst, bool IsCtor)
+BuiltinTypeMethodBuilder::BuiltinTypeMethodBuilder(
+ BuiltinTypeDeclBuilder &DB, StringRef NameStr, QualType ReturnTy,
+ bool IsConst, bool IsCtor, AccessSpecifier Access, StorageClass SC)
: DeclBuilder(DB), ReturnTy(ReturnTy), Method(nullptr), IsConst(IsConst),
- IsCtor(IsCtor) {
+ IsCtor(IsCtor), Access(Access), SC(SC) {
assert((!NameStr.empty() || IsCtor) && "method needs a name");
assert(((IsCtor && !IsConst) || !IsCtor) && "constructor cannot be const");
@@ -390,10 +424,9 @@ void BuiltinTypeMethodBuilder::createDecl() {
ExplicitSpecifier(), false, true, false,
ConstexprSpecKind::Unspecified);
else
- Method =
- CXXMethodDecl::Create(AST, DeclBuilder.Record, SourceLocation(),
- NameInfo, FuncTy, TSInfo, SC_None, false, false,
- ConstexprSpecKind::Unspecified, SourceLocation());
+ Method = CXXMethodDecl::Create(
+ AST, DeclBuilder.Record, SourceLocation(), NameInfo, FuncTy, TSInfo, SC,
+ false, false, ConstexprSpecKind::Unspecified, SourceLocation());
// create params & set them to the function prototype
SmallVector<ParmVarDecl *> ParmDecls;
@@ -431,15 +464,31 @@ Expr *BuiltinTypeMethodBuilder::getResourceHandleExpr() {
OK_Ordinary);
}
+BuiltinTypeMethodBuilder &
+BuiltinTypeMethodBuilder::createLocalVar(StringRef Name, QualType Ty) {
+ ensureCompleteDecl();
+
+ ASTContext &AST = DeclBuilder.SemaRef.getASTContext();
+ VarDecl *VD =
+ VarDecl::Create(AST, Method, SourceLocation(), SourceLocation(),
+ &AST.Idents.get(Name, tok::TokenKind::identifier), Ty,
+ AST.getTrivialTypeSourceInfo(Ty), SC_None);
+ LocalVars.push_back(VD);
+ DeclStmt *DS = new (AST)
+ clang::DeclStmt(DeclGroupRef(VD), SourceLocation(), SourceLocation());
+ StmtsList.push_back(DS);
+ return *this;
+}
+
template <typename... Ts>
BuiltinTypeMethodBuilder &
BuiltinTypeMethodBuilder::callBuiltin(StringRef BuiltinName,
QualType ReturnType, Ts... ArgSpecs) {
+ ensureCompleteDecl();
+
std::array<Expr *, sizeof...(ArgSpecs)> Args{
convertPlaceholder(std::forward<Ts>(ArgSpecs))...};
- ensureCompleteDecl();
-
ASTContext &AST = DeclBuilder.SemaRef.getASTContext();
FunctionDecl *FD = lookupBuiltinFunction(DeclBuilder.SemaRef, BuiltinName);
DeclRefExpr *DRE = DeclRefExpr::Create(
@@ -483,6 +532,55 @@ BuiltinTypeMethodBuilder &BuiltinTypeMethodBuilder::dereference(T Ptr) {
return *this;
}
+template <typename T>
+BuiltinTypeMethodBuilder &
+BuiltinTypeMethodBuilder::getResourceHandle(T ResourceRecord) {
+ ensureCompleteDecl();
+
+ Expr *ResourceExpr = convertPlaceholder(ResourceRecord);
+
+ ASTContext &AST = DeclBuilder.SemaRef.getASTContext();
+ FieldDecl *HandleField = DeclBuilder.getResourceHandleField();
+ MemberExpr *HandleExpr = MemberExpr::CreateImplicit(
+ AST, ResourceExpr, false, HandleField, HandleField->getType(), VK_LValue,
+ OK_Ordinary);
+ StmtsList.push_back(HandleExpr);
+ return *this;
+}
+
+template <typename TResource, typename TValue>
+BuiltinTypeMethodBuilder &
+BuiltinTypeMethodBuilder::setHandleFieldOnResource(TResource ResourceRecord,
+ TValue HandleValue) {
+ ensureCompleteDecl();
+
+ Expr *ResourceExpr = convertPlaceholder(ResourceRecord);
+ Expr *HandleValueExpr = convertPlaceholder(HandleValue);
+
+ ASTContext &AST = DeclBuilder.SemaRef.getASTContext();
+ FieldDecl *HandleField = DeclBuilder.getResourceHandleField();
+ MemberExpr *HandleMemberExpr = MemberExpr::CreateImplicit(
+ AST, ResourceExpr, false, HandleField, HandleField->getType(), VK_LValue,
+ OK_Ordinary);
+ Stmt *AssignStmt = BinaryOperator::Create(
+ DeclBuilder.SemaRef.getASTContext(), HandleMemberExpr, HandleValueExpr,
+ BO_Assign, HandleMemberExpr->getType(), ExprValueKind::VK_PRValue,
+ ExprObjectKind::OK_Ordinary, SourceLocation(), FPOptionsOverride());
+ StmtsList.push_back(AssignStmt);
+ return *this;
+}
+
+template <typename T>
+BuiltinTypeMethodBuilder &BuiltinTypeMethodBuilder::returnValue(T ReturnValue) {
+ ensureCompleteDecl();
+
+ Expr *ReturnValueExpr = convertPlaceholder(ReturnValue);
+ ASTContext &AST = DeclBuilder.SemaRef.getASTContext();
+ StmtsList.push_back(
+ ReturnStmt::Create(AST, SourceLocation(), ReturnValueExpr, nullptr));
+ return *this;
+}
+
BuiltinTypeDeclBuilder &BuiltinTypeMethodBuilder::finalize() {
assert(!DeclBuilder.Record->isCompleteDefinition() &&
"record is already complete");
@@ -510,7 +608,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeMethodBuilder::finalize() {
Method->setBody(CompoundStmt::Create(AST, StmtsList, FPOptionsOverride(),
SourceLocation(), SourceLocation()));
Method->setLexicalDeclContext(DeclBuilder.Record);
- Method->setAccess(AccessSpecifier::AS_public);
+ Method->setAccess(Access);
Method->addAttr(AlwaysInlineAttr::CreateImplicit(
AST, SourceRange(), AlwaysInlineAttr::CXX11_clang_always_inline));
DeclBuilder.Record->addDecl(Method);
@@ -676,6 +774,58 @@ BuiltinTypeDeclBuilder::addHandleConstructorFromImplicitBinding() {
.finalize();
}
+BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromBinding() {
+ if (Record->isCompleteDefinition())
+ return *this;
+
+ using PH = BuiltinTypeMethodBuilder::PlaceHolder;
+ ASTContext &AST = SemaRef.getASTContext();
+ QualType HandleType = getResourceHandleField()->getType();
+ QualType RecordType = AST.getTypeDeclType(cast<TypeDecl>(Record));
+
+ return BuiltinTypeMethodBuilder(*this, "__createFromBinding", RecordType,
+ false, false, AS_public, SC_Static)
+ .addParam("registerNo", AST.UnsignedIntTy)
+ .addParam("spaceNo", AST.UnsignedIntTy)
+ .addParam("range", AST.IntTy)
+ .addParam("index", AST.UnsignedIntTy)
+ .addParam("name", AST.getPointerType(AST.CharTy.withConst()))
+ .createLocalVar("tmp", RecordType)
+ .getResourceHandle(PH::LocalVar_0)
+ .callBuiltin("__builtin_hlsl_resource_handlefrombinding", HandleType,
+ PH::LastStmt, PH::_0, PH::_1, PH::_2, PH::_3, PH::_4)
+ .setHandleFieldOnResource(PH::LocalVar_0, PH::LastStmt)
+ .returnValue(PH::LocalVar_0)
+ .finalize();
+}
+
+BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromImplicitBinding() {
+ if (Record->isCompleteDefinition())
+ return *this;
+
+ using PH = BuiltinTypeMethodBuilder::PlaceHolder;
+ ASTContext &AST = SemaRef.getASTContext();
+ QualType HandleType = getResourceHandleField()->getType();
+ QualType RecordType = AST.getTypeDeclType(cast<TypeDecl>(Record));
+
+ return BuiltinTypeMethodBuilder(*this, "__createFromImplicitBinding",
+ RecordType, false, false, AS_public,
+ SC_Static)
+ .addParam("orderId", AST.UnsignedIntTy)
+ .addParam("spaceNo", AST.UnsignedIntTy)
+ .addParam("range", AST.IntTy)
+ .addParam("index", AST.UnsignedIntTy)
+ .addParam("name", AST.getPointerType(AST.CharTy.withConst()))
+ .createLocalVar("tmp", RecordType)
+ .getResourceHandle(PH::LocalVar_0)
+ .callBuiltin("__builtin_hlsl_resource_handlefromimplicitbinding",
+ HandleType, PH::LastStmt, PH::_0, PH::_1, PH::_2, PH::_3,
+ PH::_4)
+ .setHandleFieldOnResource(PH::LocalVar_0, PH::LastStmt)
+ .returnValue(PH::LocalVar_0)
+ .finalize();
+}
+
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addArraySubscriptOperators() {
ASTContext &AST = Record->getASTContext();
DeclarationName Subscript =
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
index 098b72692bd3a..ba860a9080cea 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
@@ -81,6 +81,10 @@ class BuiltinTypeDeclBuilder {
BuiltinTypeDeclBuilder &addHandleConstructorFromBinding();
BuiltinTypeDeclBuilder &addHandleConstructorFromImplicitBinding();
+ // Static create methods
+ BuiltinTypeDeclBuilder &addCreateFromBinding();
+ BuiltinTypeDeclBuilder &addCreateFromImplicitBinding();
+
// Builtin types methods
BuiltinTypeDeclBuilder &addLoadMethods();
BuiltinTypeDeclBuilder &addIncrementCounterMethod();
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 726581d131623..a5d51ca7d35be 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -132,6 +132,8 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
return BuiltinTypeDeclBuilder(S, Decl)
.addHandleMember(RC, IsROV, RawBuffer)
.addDefaultHandleConstructor()
+ .addCreateFromBinding()
+ .addCreateFromImplicitBinding()
.addHandleConstructorFromBinding()
.addHandleConstructorFromImplicitBinding();
}
diff --git a/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl b/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl
index 90794eb69ef46..1fef4901d7404 100644
--- a/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl
+++ b/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl
@@ -56,6 +56,62 @@ RESOURCE Buffer;
// CHECK-NEXT: CXXThisExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue implicit this
// CHECK-NEXT: AlwaysInlineAttr
+// Static __createFromBinding method
+
+// CHECK: CXXMethodDecl {{.*}} __createFromBinding 'hlsl::[[RESOURCE]] (unsigned int, unsigned int, int, unsigned int, const char *)' static
+// CHECK-NEXT: ParmVarDecl {{.*}} registerNo 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} spaceNo 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} range 'int'
+// CHECK-NEXT: ParmVarDecl {{.*}} index 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} name 'const char *'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl {{.*}} tmp 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: BinaryOperator {{.*}} '__hlsl_resource_t {{.*}}]]' '='
+// CHECK-NEXT: MemberExpr {{.*}} '__hlsl_resource_t {{.*}}' lvalue .__handle
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: CallExpr {{.*}} '__hlsl_resource_t {{.*}}'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(...) noexcept' <BuiltinFnToFnPtr>
+// CHECK-NEXT: DeclRefExpr {{.*}} '<builtin fn type>' Function {{.*}} '__builtin_hlsl_resource_handlefrombinding' 'void (...) noexcept'
+// CHECK-NEXT: MemberExpr {{.*}} '__hlsl_resource_t {{.*}}' lvalue .__handle
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'registerNo' 'unsigned int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'spaceNo' 'unsigned int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int' ParmVar {{.*}} 'range' 'int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'index' 'unsigned int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'const char *' ParmVar {{.*}} 'name' 'const char *'
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
+
+// Static __createFromImplicitBinding method
+
+// CHECK: CXXMethodDecl {{.*}} __createFromImplicitBinding 'hlsl::[[RESOURCE]] (unsigned int, unsigned int, int, unsigned int, const char *)' static
+// CHECK-NEXT: ParmVarDecl {{.*}} orderId 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} spaceNo 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} range 'int'
+// CHECK-NEXT: ParmVarDecl {{.*}} index 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} name 'const char *'
+// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-NEXT: DeclStmt {{.*}}
+// CHECK-NEXT: VarDecl {{.*}} tmp 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: BinaryOperator {{.*}} '__hlsl_resource_t {{.*}}]]' '='
+// CHECK-NEXT: MemberExpr {{.*}} '__hlsl_resource_t {{.*}}' lvalue .__handle
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: CallExpr {{.*}} '__hlsl_resource_t {{.*}}'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(...) noexcept' <BuiltinFnToFnPtr>
+// CHECK-NEXT: DeclRefExpr {{.*}} '<builtin fn type>' Function {{.*}} '__builtin_hlsl_resource_handlefromimplicitbinding' 'void (...) noexcept'
+// CHECK-NEXT: MemberExpr {{.*}} '__hlsl_resource_t {{.*}}' lvalue .__handle
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'orderId' 'unsigned int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'spaceNo' 'unsigned int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int' ParmVar {{.*}} 'range' 'int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'index' 'unsigned int'
+// CHECK-NEXT: DeclRefExpr {{.*}} 'const char *' ParmVar {{.*}} 'name' 'const char *'
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]'
+// CHECK-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
+
// Constructor from binding
// CHECK: CXXConstructorDecl {{.*}} [[RESOURCE]] 'void (unsigned int, unsigned int, int, unsigned int, const char *)' inline
@@ -104,5 +160,5 @@ RESOURCE Buffer;
// CHECK-NEXT: DeclRefExpr {{.*}} 'const char *' ParmVar {{.*}} 'name' 'const char *'
// CHECK-NEXT: AlwaysInlineAttr
-// CHECK-NOSUBSCRIPT-NOT: CXXMethodDecl {{.*}} operator[] 'const element_type &(unsigned int) const'
-// CHECK-NOSUBSCRIPT-NOT: CXXMethodDecl {{.*}} operator[] 'element_type &(unsigned int)'
+// CHECK-NOSUBSCRIPT-NOT: CXXMethodDecl {{.*}} operator[] 'const char8_t &(unsigned int) const'
+// CHECK-NOSUBSCRIPT-NOT: CXXMethodDecl {{.*}} operator[] 'char8_t &(unsigned int)'
diff --git a/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl b/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
index e028936e397ac..798af6232b124 100644
--- a/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
+++ b/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
@@ -103,6 +103,62 @@ RESOURCE<float> Buffer;
// CHECK-NEXT: CXXThisExpr {{.*}} 'hlsl::[[RESOURCE]]<element_type>' lvalue implicit this
// CHECK-NEXT: AlwaysInlineAttr
+// Static __createFromBinding method
+
+// CHECK: CXXMethodDecl {{.*}} __createFromBinding 'hlsl::[[RESOURCE]]<element_type> (unsigned int, unsigned int, int, unsigned int, const char *)' static
+// CHECK-NEXT: ParmVarDecl {{.*}} registerNo 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} spaceNo 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} range 'int'
+// CHECK-NEXT: ParmVarDecl {{.*}} index 'unsigned int'
+// CHECK-NEXT: ParmVarDecl {{.*}} name 'const char *'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: DeclStmt
+// CHECK-NEXT: VarDecl {{.*}} tmp 'hlsl::[[RESOURCE]]<element_type>'
+// CHECK-NEXT: BinaryOperator {{.*}} '__hlsl_resource_t {{.*}}]]' '='
+// CHECK-NEXT: MemberExpr {{.*}} '__hlsl_resource_t {{.*}}' lvalue .__handle
+// CHECK-NEXT: DeclRefExpr {{.*}} 'hlsl::[[RESOURCE]]<element_type>' lvalue Var {{.*}} 'tmp' 'hlsl::[[RESOURCE]]<element_type>'
+// CHECK-NEXT: CallExpr {{.*}} '__hlsl_re...
[truncated]
|
…create-1-add-methods
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.
Josh/Alex : LGTUS outside of some nits/general overview questions.
&AST.Idents.get(Name, tok::TokenKind::identifier), Ty, | ||
AST.getTrivialTypeSourceInfo(Ty), SC_None); | ||
LocalVars.push_back(VD); | ||
DeclStmt *DS = new (AST) |
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.
Who owns the lifetime of DS? Do we have a general pattern? I'm used to smart pointers galore.
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.
ASTContext owns all allocations of AST nodes and will take care of freeing the memory.
BuiltinTypeMethodBuilder & | ||
BuiltinTypeMethodBuilder::callBuiltin(StringRef BuiltinName, | ||
QualType ReturnType, Ts... ArgSpecs) { | ||
ensureCompleteDecl(); |
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.
nit: no need to move this?
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 move was intentional. We need a complete method declaration (=all parameters created) before convertPlaceHolder
can refer to them.
.finalize(); | ||
} | ||
|
||
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromBinding() { |
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.
nit: addCreateFromBinding and addCreateFromImplicitBinding are super similar. Do we have a general preference of having explicit functions vs an input argument for this? Just the call to BuiltinTypeMethodBuilder constructor and the first addparam are different.
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.
We do want these methods to be separate with different names since Sema and CodeGen will search for them by name. There are also going to be methods for initializing resources from dynamic binding, or resources with separate binding for counter - the complete list of create methods will be more varied.
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 think Alex was trying to say that the implementations of the builder functions are really similar. That is, would it be worth it to abstract out a helper along the lines of
BuiltinTypeDeclBuilder &addStaticResoureConstructor(StringRef Name, StringRef Builtin, ArrayRef<std::pair<StringRef, QualType>> Params);
I think it might clean this up a bit, but I'm also okay with waiting for some of the other initialization functions to come in before settling on the best abstraction here. Up to you.
BuiltinTypeMethodBuilder & | ||
BuiltinTypeMethodBuilder::callBuiltin(StringRef BuiltinName, | ||
QualType ReturnType, Ts... ArgSpecs) { | ||
ensureCompleteDecl(); |
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.
Was this moved up intentionally?
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.
Yes, this move was intentional. We need a complete method declaration (=all parameters created) before convertPlaceHolder
can refer to them.
…create-1-add-methods
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.
LGTM
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.
LGTM. Minor comments on naming conventions and a potential code clean up
.finalize(); | ||
} | ||
|
||
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCreateFromBinding() { |
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 think Alex was trying to say that the implementations of the builder functions are really similar. That is, would it be worth it to abstract out a helper along the lines of
BuiltinTypeDeclBuilder &addStaticResoureConstructor(StringRef Name, StringRef Builtin, ArrayRef<std::pair<StringRef, QualType>> Params);
I think it might clean this up a bit, but I'm also okay with waiting for some of the other initialization functions to come in before settling on the best abstraction here. Up to you.
…llvm-project into res-create-1-add-methods
Adds static methods
__createFromBinding
and__createFromImplicitBinding
to resource classes. These methods will be used for resource initialization instead of resource constructors that take binding information.Updated proposal: llvm/wg-hlsl#336
Depends on #155861
Part 1 of #154221