llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Helena Kotas (hekota) <details> <summary>Changes</summary> Use static methods __createFromBinding and __createFromImplicitBinding to initialize resources in resource arrays. Depends on #<!-- -->156544 Part 3 of #<!-- -->154221 --- Patch is 51.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/157005.diff 9 Files Affected: - (modified) clang/include/clang/Sema/SemaHLSL.h (-5) - (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+74-52) - (modified) clang/lib/Sema/SemaHLSL.cpp (+26-66) - (modified) clang/test/CodeGenHLSL/resources/res-array-global-dyn-index.hlsl (+7-7) - (modified) clang/test/CodeGenHLSL/resources/res-array-global-multi-dim.hlsl (+22-22) - (modified) clang/test/CodeGenHLSL/resources/res-array-global-subarray-many.hlsl (+53-26) - (modified) clang/test/CodeGenHLSL/resources/res-array-global-subarray-one.hlsl (+19-16) - (modified) clang/test/CodeGenHLSL/resources/res-array-global.hlsl (+40-27) - (modified) clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl (+2-2) ``````````diff diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 5cbe1b658f5cd..86265a51fb252 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -241,11 +241,6 @@ class SemaHLSL : public SemaBase { bool initGlobalResourceDecl(VarDecl *VD); bool initGlobalResourceArrayDecl(VarDecl *VD); - void createResourceRecordCtorArgs(const Type *ResourceTy, StringRef VarName, - HLSLResourceBindingAttr *RBA, - HLSLVkBindingAttr *VkBinding, - uint32_t ArrayIndex, - llvm::SmallVectorImpl<Expr *> &Args); }; } // namespace clang diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index d27f3781c69a3..fa365f419cdb1 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "CGHLSLRuntime.h" +#include "Address.h" #include "CGDebugInfo.h" #include "CodeGenFunction.h" #include "CodeGenModule.h" @@ -38,6 +39,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" #include <cstdint> +#include <optional> using namespace clang; using namespace CodeGen; @@ -148,14 +150,22 @@ static Value *buildNameForResource(llvm::StringRef BaseName, .getPointer(); } -static void createResourceCtorArgs(CodeGenModule &CGM, CXXConstructorDecl *CD, - llvm::Value *ThisPtr, llvm::Value *Range, - llvm::Value *Index, StringRef Name, - HLSLResourceBindingAttr *RBA, - HLSLVkBindingAttr *VkBinding, - CallArgList &Args) { +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) { @@ -167,44 +177,57 @@ static void createResourceCtorArgs(CodeGenModule &CGM, CXXConstructorDecl *CD, SpaceNo = RBA->getSpaceNumber(); } - ASTContext &AST = CD->getASTContext(); + CXXMethodDecl *CreateMethod = nullptr; Value *NameStr = buildNameForResource(Name, CGM); Value *Space = llvm::ConstantInt::get(CGM.IntTy, SpaceNo); - Args.add(RValue::get(ThisPtr), CD->getThisType()); if (RegisterSlot.has_value()) { // explicit binding auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RegisterSlot.value()); Args.add(RValue::get(RegSlot), AST.UnsignedIntTy); - Args.add(RValue::get(Space), AST.UnsignedIntTy); - Args.add(RValue::get(Range), AST.IntTy); - Args.add(RValue::get(Index), AST.UnsignedIntTy); - + CreateMethod = lookupMethod(ResourceDecl, "__createFromBinding", SC_Static); } else { // implicit binding - assert(RBA && "missing implicit binding attribute"); auto *OrderID = llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID()); - 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(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. -static Value *initializeLocalResourceArray( - CodeGenFunction &CGF, AggValueSlot &ValueSlot, - const ConstantArrayType *ArrayTy, CXXConstructorDecl *CD, +// 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); @@ -225,16 +248,19 @@ static Value *initializeLocalResourceArray( Index = CGF.Builder.CreateAdd(Index, One); GEPIndices.back() = llvm::ConstantInt::get(IntTy, I); } - Index = initializeLocalResourceArray( - CGF, ValueSlot, SubArrayTy, CD, Range, Index, ResourceName, RBA, - VkBinding, GEPIndices, ArraySubsExprLoc); + 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 = CD->getASTContext().getTypeSizeInChars(ElemType); + CharUnits ElemSize = AST.getTypeSizeInChars(ElemType); CharUnits Align = TmpArrayAddr.getAlignment().alignmentOfArrayElement(ElemSize); @@ -243,16 +269,18 @@ static Value *initializeLocalResourceArray( Index = CGF.Builder.CreateAdd(Index, One); GEPIndices.back() = llvm::ConstantInt::get(IntTy, I); } - Address ThisAddress = + Address ReturnAddress = CGF.Builder.CreateGEP(TmpArrayAddr, GEPIndices, Ty, Align); - llvm::Value *ThisPtr = CGF.getAsNaturalPointerTo(ThisAddress, ElemType); CallArgList Args; - createResourceCtorArgs(CGF.CGM, CD, ThisPtr, Range, Index, ResourceName, - RBA, VkBinding, Args); - CGF.EmitCXXConstructorCall(CD, Ctor_Complete, false, false, ThisAddress, - Args, ValueSlot.mayOverlap(), ArraySubsExprLoc, - ValueSlot.isSanitizerChecked()); + CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( + CGF.CGM, ResourceDecl, Range, Index, ResourceName, RBA, VkBinding, + Args); + + if (!CreateMethod) + return std::nullopt; + + callResourceInitMethod(CGF, CreateMethod, Args, ReturnAddress); } return Index; } @@ -906,11 +934,6 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr( QualType ResourceTy = ResultTy->isArrayType() ? AST.getBaseElementType(ResultTy) : ResultTy; - // Lookup the resource class constructor based on the resource type and - // binding. - CXXConstructorDecl *CD = findResourceConstructorDecl( - AST, ResourceTy, VkBinding || RBA->hasRegisterSlot()); - // 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). @@ -923,7 +946,6 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr( TmpVar, Qualifiers(), AggValueSlot::IsDestructed_t(true), AggValueSlot::DoesNotNeedGCBarriers, AggValueSlot::IsAliased_t(false), AggValueSlot::DoesNotOverlap); - Address TmpVarAddress = ValueSlot.getAddress(); // Calculate total array size (= range size). llvm::Value *Range = @@ -932,27 +954,27 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr( // If the result of the subscript operation is a single resource, call the // constructor. if (ResultTy == ResourceTy) { - QualType ThisType = CD->getThisType()->getPointeeType(); - llvm::Value *ThisPtr = CGF.getAsNaturalPointerTo(TmpVarAddress, ThisType); - - // Assemble the constructor parameters. CallArgList Args; - createResourceCtorArgs(CGM, CD, ThisPtr, Range, Index, ArrayDecl->getName(), - RBA, VkBinding, Args); - // Call the constructor. - CGF.EmitCXXConstructorCall(CD, Ctor_Complete, false, false, TmpVarAddress, - Args, ValueSlot.mayOverlap(), - ArraySubsExpr->getExprLoc(), - ValueSlot.isSanitizerChecked()); + CXXMethodDecl *CreateMethod = lookupResourceInitMethodAndSetupArgs( + CGF.CGM, ResourceTy->getAsCXXRecordDecl(), Range, Index, + ArrayDecl->getName(), RBA, VkBinding, Args); + + if (!CreateMethod) + return std::nullopt; + + 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()); - initializeLocalResourceArray(CGF, ValueSlot, ArrayTy, CD, Range, Index, - ArrayDecl->getName(), RBA, VkBinding, - {llvm::ConstantInt::get(CGM.IntTy, 0)}, - ArraySubsExpr->getExprLoc()); + 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); } diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 775098d06b18c..4e3bccf86ee9c 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -3696,55 +3696,6 @@ void SemaHLSL::ActOnVariableDeclarator(VarDecl *VD) { deduceAddressSpace(VD); } -void SemaHLSL::createResourceRecordCtorArgs( - const Type *ResourceTy, StringRef VarName, HLSLResourceBindingAttr *RBA, - HLSLVkBindingAttr *VkBinding, uint32_t ArrayIndex, - llvm::SmallVectorImpl<Expr *> &Args) { - std::optional<uint32_t> RegisterSlot; - uint32_t SpaceNo = 0; - if (VkBinding) { - RegisterSlot = VkBinding->getBinding(); - SpaceNo = VkBinding->getSet(); - } else if (RBA) { - if (RBA->hasRegisterSlot()) - RegisterSlot = RBA->getSlotNumber(); - SpaceNo = RBA->getSpaceNumber(); - } - - ASTContext &AST = SemaRef.getASTContext(); - uint64_t UIntTySize = AST.getTypeSize(AST.UnsignedIntTy); - uint64_t IntTySize = AST.getTypeSize(AST.IntTy); - IntegerLiteral *RangeSize = IntegerLiteral::Create( - AST, llvm::APInt(IntTySize, 1), AST.IntTy, SourceLocation()); - IntegerLiteral *Index = - IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, ArrayIndex), - AST.UnsignedIntTy, SourceLocation()); - IntegerLiteral *Space = - IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, SpaceNo), - AST.UnsignedIntTy, SourceLocation()); - StringLiteral *Name = StringLiteral::Create( - AST, VarName, StringLiteralKind::Ordinary, false, - AST.getStringLiteralArrayType(AST.CharTy.withConst(), VarName.size()), - SourceLocation()); - - // resource with explicit binding - if (RegisterSlot.has_value()) { - IntegerLiteral *RegSlot = IntegerLiteral::Create( - AST, llvm::APInt(UIntTySize, RegisterSlot.value()), AST.UnsignedIntTy, - SourceLocation()); - Args.append({RegSlot, Space, RangeSize, Index, Name}); - } else { - // resource with implicit binding - uint32_t OrderID = (RBA && RBA->hasImplicitBindingOrderID()) - ? RBA->getImplicitBindingOrderID() - : getNextImplicitBindingOrderID(); - IntegerLiteral *OrderId = - IntegerLiteral::Create(AST, llvm::APInt(UIntTySize, OrderID), - AST.UnsignedIntTy, SourceLocation()); - Args.append({Space, RangeSize, Index, OrderId, Name}); - } -} - bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) { assert(VD->getType()->isHLSLResourceRecord() && "expected resource record type"); @@ -3849,28 +3800,37 @@ bool SemaHLSL::initGlobalResourceArrayDecl(VarDecl *VD) { // Individual resources in a resource array are not initialized here. They // are initialized later on during codegen when the individual resources are - // accessed. Codegen will emit a call to the resource constructor with the - // specified array index. We need to make sure though that the constructor + // accessed. Codegen will emit a call to the resource initialization method + // with the specified array index. We need to make sure though that the method // for the specific resource type is instantiated, so codegen can emit a call // to it when the array element is accessed. - SmallVector<Expr *> Args; - QualType ResElementTy = VD->getASTContext().getBaseElementType(VD->getType()); - createResourceRecordCtorArgs(ResElementTy.getTypePtr(), VD->getName(), - VD->getAttr<HLSLResourceBindingAttr>(), - VD->getAttr<HLSLVkBindingAttr>(), 0, Args); - SourceLocation Loc = VD->getLocation(); - InitializedEntity Entity = - InitializedEntity::InitializeTemporary(ResElementTy); - InitializationKind Kind = InitializationKind::CreateDirect(Loc, Loc, Loc); - InitializationSequence InitSeq(SemaRef, Entity, Kind, Args); - if (InitSeq.Failed()) + // Find correct initialization method based on the resource binding + // information. + ASTContext &AST = SemaRef.getASTContext(); + QualType ResElementTy = AST.getBaseElementType(VD->getType()); + CXXRecordDecl *ResourceDecl = ResElementTy->getAsCXXRecordDecl(); + + HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>(); + HLSLVkBindingAttr *VkBinding = VD->getAttr<HLSLVkBindingAttr>(); + CXXMethodDecl *CreateMethod = nullptr; + + if (VkBinding || (RBA && RBA->hasRegisterSlot())) + // Resource has explicit binding. + CreateMethod = lookupMethod(ResourceDecl, "__createFromBinding", SC_Static); + else + // Resource has implicit binding. + CreateMethod = + lookupMethod(ResourceDecl, "__createFromImplicitBinding", SC_Static); + + if (!CreateMethod) return false; - // This takes care of instantiating and emitting of the constructor that will - // be called from codegen when the array is accessed. - ExprResult OneResInit = InitSeq.Perform(SemaRef, Entity, Kind, Args); - return !OneResInit.isInvalid(); + // Make sure the create method template is instantiated and emitted. + if (!CreateMethod->isDefined() && CreateMethod->isTemplateInstantiation()) + SemaRef.InstantiateFunctionDefinition(VD->getLocation(), CreateMethod, + true); + return true; } // Returns true if the initialization has been handled. diff --git a/clang/test/CodeGenHLSL/resources/res-array-global-dyn-index.hlsl b/clang/test/CodeGenHLSL/resources/res-array-global-dyn-index.hlsl index bbd162e3aad20..f17cf12945e4a 100644 --- a/clang/test/CodeGenHLSL/resources/res-array-global-dyn-index.hlsl +++ b/clang/test/CodeGenHLSL/resources/res-array-global-dyn-index.hlsl @@ -1,17 +1,15 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -finclude-default-header \ -// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s +// RUN: -emit-llvm -disable-llvm-passes -o - %s | llvm-cxxfilt | FileCheck %s // CHECK: @[[BufA:.*]] = private unnamed_addr constant [2 x i8] c"A\00", align 1 RWBuffer<float> A[4][3] : register(u2); RWStructuredBuffer<float> Out; -// Make sure A[GI.x][GI.y] is translated to a RWBuffer<float> constructor call with range 12 and dynamically calculated index +// Make sure A[GI.x][GI.y] is translated to a RWBuffer<float>::__createFromBinding call +// with range 12 and dynamically calculated index -// NOTE: -// Constructor call for explicit binding has "jjij" in the mangled name and the arguments are (register, space, range_size, index, name). - -// CHECK: define internal void @_Z4mainDv3_j(<3 x i32> noundef %GI) +// CHECK: define internal void @main(unsigned int vector[3])(<3 x i32> noundef %GI) // CHECK: %[[GI_alloca:.*]] = alloca <3 x i32>, align 16 // CHECK: %[[Tmp0:.*]] = alloca %"class.hlsl::RWBuffer // CHECK: store <3 x i32> %GI, ptr %[[GI_alloca]] @@ -22,7 +20,9 @@ RWStructuredBuffer<float> Out; // CHECK: %[[GI_x:.*]] = extractelement <3 x i32> %[[GI]], i32 0 // CHECK: %[[Tmp1:.*]] = mul i32 %[[GI_x]], 3 // CHECK: %[[Index:.*]] = add i32 %[[GI_y]], %[[Tmp1]] -// CHECK: call void @_ZN4hlsl8RWBufferIfEC1EjjijPKc(ptr {{.*}} %[[Tmp0]], i32 noundef 2, i32 noundef 0, i32 noundef 12, i32 noundef %[[Index]], ptr noundef @A.str) +// CHECK: call void @hlsl::RWBuffer<float>::__createFromBinding(unsigned int, unsigned int, int, unsigned int, char const*) +// CHECK-SAME: (ptr {{.*}} sret(%"class.hlsl::RWBuffer") align 4 %[[Tmp0]], +// CHECK-SAME: i32 noundef 2, i32 noundef 0, i32 noundef 12, i32 noundef %[[Index]], ptr noundef @A.str) [numthreads(4,1,1)] void main(uint3 GI : SV_GroupThreadID) { Out[0] = A[GI.x][GI.y][0]; diff --git a/clang/test/CodeGenHLSL/resources/res-array-global-multi-dim.hlsl b/clang/test/CodeGenHLSL/resources/res-array-global-multi-dim.hlsl index 36871f9a63b3f..1a05897b9e70b 100644 --- a/clang/test/CodeGenHLSL/resources/res-array-global-multi-dim.hlsl +++ b/clang/test/CodeGenHLSL/resources/res-array-global-multi-dim.hlsl @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -finclude-default-header \ -// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s +// RUN: -emit-llvm -disable-llvm-passes -o - %s | llvm-cxxfilt | FileCheck %s // RUN: %clang_cc1 -finclude-default-header -triple spirv-unknown-vulkan-compute \ -// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s +// RUN: -emit-llvm -disable-llvm-passes -o - %s | llvm-cxxfilt | FileCheck %s // CHECK: @[[BufB:.*]] = private unnamed_addr constant [2 x i8] c"B\00", align 1 // CHECK: @[[BufC:.*]] = private unnamed_addr constant [2 x i8] c"C\00", align 1 @@ -17,30 +17,30 @@ RWStructuredBuffer<float> Out; [numthreads(4,1,1)] void main() { - // CHECK: define internal{{.*}} void @_Z4mainv() + // CHECK: define internal{{.*}} void @main() // CHECK: %[[Tmp0:.*]] = alloca %"class.hlsl::RWBuffer // CHECK: %[[Tmp1:.*]] = alloca %"class.hlsl::RWBuffer // CHECK: %[[Tmp2:.*]] = alloca %"class.hlsl::RWBuffer // CHECK: %[[Tmp3:.*]] = alloca %"class.hlsl::RWBuffer - // NOTE: - // Constructor call for explicit binding has "jjij" in the mangled name and the arguments are (register, space, range_size, index, name). - // For implicit binding the constructor has "ji... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/157005 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
