[HLSL] Use static create methods to initialize resources in arrays #157005
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "CGHLSLRuntime.h" | ||
| #include "Address.h" | ||
| #include "CGDebugInfo.h" | ||
| #include "CodeGenFunction.h" | ||
| #include "CodeGenModule.h" | ||
|
|
@@ -39,6 +40,7 @@ | |
| #include "llvm/Support/ErrorHandling.h" | ||
| #include "llvm/Support/FormatVariadic.h" | ||
| #include <cstdint> | ||
| #include <optional> | ||
|
|
||
| using namespace clang; | ||
| using namespace CodeGen; | ||
|
|
@@ -111,52 +113,29 @@ static int getTotalArraySize(ASTContext &AST, const clang::Type *Ty) { | |
| return AST.getConstantArrayElementCount(cast<ConstantArrayType>(Ty)); | ||
| } | ||
|
|
||
| static Value *buildNameForResource(llvm::StringRef BaseName, | ||
| CodeGenModule &CGM) { | ||
| llvm::SmallString<64> GlobalName = {BaseName, ".str"}; | ||
| return CGM.GetAddrOfConstantCString(BaseName.str(), GlobalName.c_str()) | ||
| .getPointer(); | ||
| } | ||
|
|
||
| static CXXMethodDecl *lookupMethod(CXXRecordDecl *Record, StringRef Name, | ||
| StorageClass SC = SC_None) { | ||
| for (auto *Method : Record->methods()) { | ||
| if (Method->getStorageClass() == SC && Method->getName() == Name) | ||
| return Method; | ||
| } | ||
| return nullptr; | ||
| } | ||
|
|
||
| static CXXMethodDecl *lookupResourceInitMethodAndSetupArgs( | ||
| CodeGenModule &CGM, CXXRecordDecl *ResourceDecl, llvm::Value *Range, | ||
| llvm::Value *Index, StringRef Name, HLSLResourceBindingAttr *RBA, | ||
| HLSLVkBindingAttr *VkBinding, CallArgList &Args) { | ||
| assert((VkBinding || RBA) && "at least one a binding attribute expected"); | ||
|
|
||
| ASTContext &AST = CGM.getContext(); | ||
| std::optional<uint32_t> RegisterSlot; | ||
| uint32_t SpaceNo = 0; | ||
| if (VkBinding) { | ||
|
|
@@ -168,44 +147,57 @@ static void createResourceCtorArgs(CodeGenModule &CGM, CXXConstructorDecl *CD, | |
| SpaceNo = RBA->getSpaceNumber(); | ||
| } | ||
|
|
||
| CXXMethodDecl *CreateMethod = nullptr; | ||
| Value *NameStr = buildNameForResource(Name, CGM); | ||
| Value *Space = llvm::ConstantInt::get(CGM.IntTy, SpaceNo); | ||
|
|
||
| if (RegisterSlot.has_value()) { | ||
| // explicit binding | ||
| auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RegisterSlot.value()); | ||
| Args.add(RValue::get(RegSlot), AST.UnsignedIntTy); | ||
| CreateMethod = lookupMethod(ResourceDecl, "__createFromBinding", SC_Static); | ||
| } else { | ||
| // implicit binding | ||
| auto *OrderID = | ||
| llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID()); | ||
| Args.add(RValue::get(OrderID), AST.UnsignedIntTy); | ||
| CreateMethod = | ||
| lookupMethod(ResourceDecl, "__createFromImplicitBinding", SC_Static); | ||
| } | ||
| Args.add(RValue::get(Space), AST.UnsignedIntTy); | ||
| Args.add(RValue::get(Range), AST.IntTy); | ||
| Args.add(RValue::get(Index), AST.UnsignedIntTy); | ||
| Args.add(RValue::get(NameStr), AST.getPointerType(AST.CharTy.withConst())); | ||
|
|
||
| return CreateMethod; | ||
| } | ||
|
|
||
| static void callResourceInitMethod(CodeGenFunction &CGF, | ||
| CXXMethodDecl *CreateMethod, | ||
| CallArgList &Args, Address ReturnAddress) { | ||
| llvm::Constant *CalleeFn = CGF.CGM.GetAddrOfFunction(CreateMethod); | ||
| const FunctionProtoType *Proto = | ||
| CreateMethod->getType()->getAs<FunctionProtoType>(); | ||
| const CGFunctionInfo &FnInfo = | ||
| CGF.CGM.getTypes().arrangeFreeFunctionCall(Args, Proto, false); | ||
| ReturnValueSlot ReturnValue(ReturnAddress, false); | ||
| CGCallee Callee(CGCalleeInfo(Proto), CalleeFn); | ||
| CGF.EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr); | ||
| } | ||
|
|
||
| // Initializes local resource array variable. For multi-dimensional arrays it | ||
| // calls itself recursively to initialize its sub-arrays. The Index used in the | ||
| // resource constructor calls will begin at StartIndex and will be incremented | ||
| // for each array element. The last used resource Index is returned to the | ||
| // caller. If the function returns std::nullopt, it indicates an error. | ||
| static std::optional<llvm::Value *> initializeLocalResourceArray( | ||
| CodeGenFunction &CGF, CXXRecordDecl *ResourceDecl, | ||
| const ConstantArrayType *ArrayTy, AggValueSlot &ValueSlot, | ||
| llvm::Value *Range, llvm::Value *StartIndex, StringRef ResourceName, | ||
| HLSLResourceBindingAttr *RBA, HLSLVkBindingAttr *VkBinding, | ||
| ArrayRef<llvm::Value *> PrevGEPIndices, SourceLocation ArraySubsExprLoc) { | ||
|
|
||
| ASTContext &AST = CGF.getContext(); | ||
| llvm::IntegerType *IntTy = CGF.CGM.IntTy; | ||
| llvm::Value *Index = StartIndex; | ||
| llvm::Value *One = llvm::ConstantInt::get(IntTy, 1); | ||
|
|
@@ -226,16 +218,19 @@ static Value *initializeLocalResourceArray( | |
| Index = CGF.Builder.CreateAdd(Index, One); | ||
| GEPIndices.back() = llvm::ConstantInt::get(IntTy, I); | ||
| } | ||
| std::optional<llvm::Value *> MaybeIndex = initializeLocalResourceArray( | ||
| CGF, ResourceDecl, SubArrayTy, ValueSlot, Range, Index, ResourceName, | ||
| RBA, VkBinding, GEPIndices, ArraySubsExprLoc); | ||
| if (!MaybeIndex) | ||
| return std::nullopt; | ||
| Index = *MaybeIndex; | ||
| } | ||
| return Index; | ||
| } | ||
|
|
||
| // For array of resources, initialize each resource in the array. | ||
| llvm::Type *Ty = CGF.ConvertTypeForMem(ElemType); | ||
| CharUnits ElemSize = AST.getTypeSizeInChars(ElemType); | ||
| CharUnits Align = | ||
| TmpArrayAddr.getAlignment().alignmentOfArrayElement(ElemSize); | ||
|
|
||
|
|
@@ -244,16 +239,21 @@ static Value *initializeLocalResourceArray( | |
| Index = CGF.Builder.CreateAdd(Index, One); | ||
| GEPIndices.back() = llvm::ConstantInt::get(IntTy, I); | ||
| } | ||
| Address ReturnAddress = | ||
| CGF.Builder.CreateGEP(TmpArrayAddr, GEPIndices, Ty, Align); | ||
|
|
||
| CallArgList Args; | ||
| CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( | ||
| CGF.CGM, ResourceDecl, Range, Index, ResourceName, RBA, VkBinding, | ||
| Args); | ||
|
|
||
| if (!CreateMethod) | ||
| // This can happen if someone creates an array of structs that looks like | ||
| // an HLSL resource record array but it does not have the required static | ||
| // create method. No binding will be generated for it. | ||
| return std::nullopt; | ||
|
There was a problem hiding this comment. Do we need to surface an error if this happens? Can this happen? There was a problem hiding this comment. This can happen when someone has an array of fake/custom(?) resource classes - array of structs with a __hlsl_resource_t member - and the struct does not have the create method. There was a problem hiding this comment. Chatted with @hekota offline. Seems like this happens for manually written structs containing handles, and the outcome here is that they just don't get bindings generated. I think not supporting automatically binding handles for people writing their own types against internal compiler details is completely reasonable. |
||
|
|
||
| callResourceInitMethod(CGF, CreateMethod, Args, ReturnAddress); | ||
| } | ||
| return Index; | ||
| } | ||
|
|
@@ -969,11 +969,6 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr( | |
| QualType ResourceTy = | ||
| ResultTy->isArrayType() ? AST.getBaseElementType(ResultTy) : ResultTy; | ||
|
|
||
| // Create a temporary variable for the result, which is either going | ||
| // to be a single resource instance or a local array of resources (we need to | ||
| // return an LValue). | ||
|
|
@@ -986,7 +981,6 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr( | |
| TmpVar, Qualifiers(), AggValueSlot::IsDestructed_t(true), | ||
| AggValueSlot::DoesNotNeedGCBarriers, AggValueSlot::IsAliased_t(false), | ||
| AggValueSlot::DoesNotOverlap); | ||
|
|
||
| // Calculate total array size (= range size). | ||
| llvm::Value *Range = | ||
|
|
@@ -995,27 +989,30 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr( | |
| // If the result of the subscript operation is a single resource, call the | ||
| // constructor. | ||
| if (ResultTy == ResourceTy) { | ||
| CallArgList Args; | ||
| CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( | ||
| CGF.CGM, ResourceTy->getAsCXXRecordDecl(), Range, Index, | ||
| ArrayDecl->getName(), RBA, VkBinding, Args); | ||
|
|
||
| if (!CreateMethod) | ||
| // This can happen if someone creates an array of structs that looks like | ||
| // an HLSL resource record array but it does not have the required static | ||
| // create method. No binding will be generated for it. | ||
| return std::nullopt; | ||
|
There was a problem hiding this comment. Same as above, what happens if this happens? |
||
|
|
||
| callResourceInitMethod(CGF, CreateMethod, Args, ValueSlot.getAddress()); | ||
|
|
||
| } else { | ||
| // The result of the subscript operation is a local resource array which | ||
| // needs to be initialized. | ||
| const ConstantArrayType *ArrayTy = | ||
| cast<ConstantArrayType>(ResultTy.getTypePtr()); | ||
| std::optional<llvm::Value *> EndIndex = initializeLocalResourceArray( | ||
| CGF, ResourceTy->getAsCXXRecordDecl(), ArrayTy, ValueSlot, Range, Index, | ||
| ArrayDecl->getName(), RBA, VkBinding, | ||
| {llvm::ConstantInt::get(CGM.IntTy, 0)}, ArraySubsExpr->getExprLoc()); | ||
| if (!EndIndex) | ||
| return std::nullopt; | ||
| } | ||
| return CGF.MakeAddrLValue(TmpVar, ResultTy, AlignmentSource::Decl); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 should probably do this with
Sema::LookupSingleName.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.
Unfortunately, this is in CodeGen and we do not have access to Sema anymore.
I can use Sema lookup in my previous PR though (#156544), so I will update it at least there.