diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index eb7a608f798b8..9a5d38af5a207 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13431,6 +13431,10 @@ def err_hlsl_incomplete_resource_array_in_function_param: Error< "incomplete resource array in a function parameter">; def err_hlsl_assign_to_global_resource: Error< "assignment to global resource variable %0 is not allowed">; +def err_hlsl_assigning_local_resource_is_not_unique + : Error<"assignment of %0 to local resource %1 is not to the same unique " + "global " + "resource">; def err_hlsl_push_constant_unique : Error<"cannot have more than one push constant block">; diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 99d8ed137b0c2..eb037c6341999 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -106,6 +106,34 @@ class ResourceBindings { llvm::DenseMap DeclToBindingListIndex; }; +// ResourceAssigns tracks how local resources are assigned to ensure that at +// any given resource access, it is statically determinable which **unique** +// global resource they are accessing. +class LocalResourceAssigns { +public: + struct Assign { + const Expr *AssignExpr; + const Scope *AssignScope; + const DeclBindingInfo *Info; + + // A new scope is not created for an else/else if it does not precede a + // compound stmt as per CXX specification. The CurMSManglingNumber at the + // time of parsing is used to differentiate the scope in this case. + uint32_t MSMangleNum; + bool Invalidated; + + Assign(const Expr *AssignExpr, const Scope *AssignScope, + const DeclBindingInfo *Info); + }; + + void trackAssign(const ValueDecl *VD, const Scope *AssignScope, + const Expr *AssignExpr, const DeclBindingInfo *Info); + std::optional getAssign(const ValueDecl *VD); + +private: + llvm::DenseMap Assignments; +}; + class SemaHLSL : public SemaBase { public: SemaHLSL(Sema &S); @@ -132,8 +160,17 @@ class SemaHLSL : public SemaBase { bool ActOnUninitializedVarDecl(VarDecl *D); void ActOnEndOfTranslationUnit(TranslationUnitDecl *TU); void CheckEntryPoint(FunctionDecl *FD); + + // Return an info if the expr has common global binding info; returns + // std::nullopt if the expr refers to non-unique global bindings, returns + // nullptr if it isn't refer to any global binding, otherwise it returns + // a reference to the global binding info. + std::optional getGlobalBinding(Expr *E); + + // Return true if everything is ok; returns false if there was an error. bool CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr, Expr *RHSExpr, SourceLocation Loc); + bool CheckResourceSubscriptExpr(CXXOperatorCallExpr *Op); QualType handleVectorBinOpConversion(ExprResult &LHS, ExprResult &RHS, QualType LHSType, QualType RHSType, @@ -229,6 +266,15 @@ class SemaHLSL : public SemaBase { // List of all resource bindings ResourceBindings Bindings; + // Mapping of all local resource assignments + LocalResourceAssigns Assigns; + + // Map of local resource variables to their used global resource. + // + // The binding can be a nullptr, in which case, the variable has yet to be + // initialized or assigned to. + llvm::DenseMap + LocalResourceBindings; // Global declaration collected for the $Globals default constant // buffer which will be created at the end of the translation unit. diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 51739c3b49ac9..b15d8803739da 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -34,6 +34,7 @@ #include "clang/AST/TypeLoc.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/OperatorKinds.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/Specifiers.h" @@ -15836,6 +15837,17 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc, LHSExpr = LHS.get(); } + if (getLangOpts().HLSL) { + if (LHSExpr->getType()->isHLSLResourceRecord() || + LHSExpr->getType()->isHLSLResourceRecordArray()) { + if (!HLSL().CheckResourceBinOp(Opc, LHSExpr, RHSExpr, OpLoc)) + return ExprError(); + } else if (auto *Operator = dyn_cast(LHSExpr)) + if (Operator->getOperator() == OO_Subscript) + if (!HLSL().CheckResourceSubscriptExpr(Operator)) + return ExprError(); + } + // Handle pseudo-objects in the RHS. if (const BuiltinType *pty = RHSExpr->getType()->getAsPlaceholderType()) { // An overload in the RHS can potentially be resolved by the type @@ -15860,12 +15872,6 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc, RHSExpr = resolvedRHS.get(); } - if (getLangOpts().HLSL && (LHSExpr->getType()->isHLSLResourceRecord() || - LHSExpr->getType()->isHLSLResourceRecordArray())) { - if (!HLSL().CheckResourceBinOp(Opc, LHSExpr, RHSExpr, OpLoc)) - return ExprError(); - } - if (getLangOpts().CPlusPlus) { // Otherwise, build an overloaded op if either expression is type-dependent // or has an overloadable type. diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index f15b274a65a53..94704427a31b2 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -27,6 +27,7 @@ #include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LLVM.h" +#include "clang/Basic/OperatorKinds.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" #include "clang/Basic/TargetInfo.h" @@ -195,6 +196,79 @@ bool ResourceBindings::hasBindingInfoForDecl(const VarDecl *VD) const { return DeclToBindingListIndex.contains(VD); } +LocalResourceAssigns::Assign::Assign(const Expr *AssignExpr, + const Scope *AssignScope, + const DeclBindingInfo *Info) + : AssignExpr(AssignExpr), AssignScope(AssignScope), Info(Info), + MSMangleNum(AssignScope->getMSCurManglingNumber()), Invalidated(false) {} + +static bool isEquivalentAssignmentScope(LocalResourceAssigns::Assign Cur, + LocalResourceAssigns::Assign Prev) { + const Scope *CurScope = Cur.AssignScope; + const Scope *PrevScope = Prev.AssignScope; + if (CurScope != PrevScope) + return false; + + // If these are not equivalent, then we are at a tailing single stmt else/ + // else if, and should be denoted as different for assignment. + if (Cur.MSMangleNum != Prev.MSMangleNum) + return false; + + // A switch statement only creates a single scope for all single sub-stmts, + // we should not overwrite a previous case. + if (CurScope->getParent()->getFlags() & Scope::SwitchScope) + return false; + + return true; +} + +static bool isAncestorScope(const Scope *Ancestor, const Scope *Cur) { + while (Cur != nullptr) { + if (Ancestor == Cur) + return true; + Cur = Cur->getParent(); + } + return false; +} + +void LocalResourceAssigns::trackAssign(const ValueDecl *VD, + const Scope *AssignScope, + const Expr *AssignExpr, + const DeclBindingInfo *Info) { + Assign Cur{AssignExpr, AssignScope, Info}; + + auto AssignIt = Assignments.find(VD); + if (AssignIt == Assignments.end()) { + // No assign recorded so insert one + Assignments.insert({VD, Cur}); + return; + } + + Assign &Prev = AssignIt->getSecond(); + + if (isEquivalentAssignmentScope(Cur, Prev) || + isAncestorScope(Cur.AssignScope, Prev.AssignScope->getParent())) { + // If the current scope is equivalent or an ancestor of the previous scope, + // we should ignore the previous access and overwrite it + Prev = Cur; + return; + } + + // If it isn't equivalent, mark the assignment as invalid and updated the + // AssignExpr to the current. We should keep the previous scope to prevent + // overwriting an invalid access. + Prev.Invalidated = Cur.Info != Prev.Info; + Prev.AssignExpr = Cur.AssignExpr; +} + +std::optional +LocalResourceAssigns::getAssign(const ValueDecl *VD) { + auto AssignIt = Assignments.find(VD); + if (AssignIt == Assignments.end()) + return std::nullopt; + return AssignIt->second; +} + SemaHLSL::SemaHLSL(Sema &S) : SemaBase(S) {} Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool CBuffer, @@ -4398,7 +4472,35 @@ bool SemaHLSL::ActOnUninitializedVarDecl(VarDecl *VD) { return false; } -// Return true if everything is ok; returns false if there was an error. +std::optional SemaHLSL::getGlobalBinding(Expr *E) { + if (auto *Ternary = dyn_cast(E)) { + auto TrueInfo = getGlobalBinding(Ternary->getTrueExpr()); + auto FalseInfo = getGlobalBinding(Ternary->getFalseExpr()); + if (!TrueInfo || !FalseInfo) + return std::nullopt; + if (*TrueInfo != *FalseInfo) + return std::nullopt; + return TrueInfo; + } + + if (auto *ASE = dyn_cast(E)) + E = ASE->getBase()->IgnoreParenImpCasts(); + + if (DeclRefExpr *DRE = dyn_cast(E->IgnoreParens())) + if (VarDecl *VD = dyn_cast(DRE->getDecl())) { + const Type *Ty = VD->getType()->getUnqualifiedDesugaredType(); + if (Ty->isArrayType()) + Ty = Ty->getArrayElementTypeNoTypeQual(); + if (const auto *AttrResType = + HLSLAttributedResourceType::findHandleTypeOnResource(Ty)) { + ResourceClass RC = AttrResType->getAttrs().ResourceClass; + return Bindings.getDeclBindingInfo(VD, RC); + } + } + + return nullptr; +} + bool SemaHLSL::CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr, Expr *RHSExpr, SourceLocation Loc) { assert((LHSExpr->getType()->isHLSLResourceRecord() || @@ -4416,16 +4518,44 @@ bool SemaHLSL::CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr, if (DeclRefExpr *DRE = dyn_cast(E->IgnoreParens())) { if (VarDecl *VD = dyn_cast(DRE->getDecl())) { if (VD->hasGlobalStorage() && VD->getStorageClass() != SC_Static) { - // assignment to global resource is not allowed + // Assignment to a global resource is not allowed. SemaRef.Diag(Loc, diag::err_hlsl_assign_to_global_resource) << VD; SemaRef.Diag(VD->getLocation(), diag::note_var_declared_here) << VD; return false; } + + if (auto Binding = getGlobalBinding(RHSExpr)) { + Assigns.trackAssign(VD, SemaRef.getCurScope(), RHSExpr, *Binding); + } else { + SemaRef.Diag(Loc, diag::err_hlsl_assigning_local_resource_is_not_unique) + << RHSExpr << VD; + return false; + } } } return true; } +bool SemaHLSL::CheckResourceSubscriptExpr(CXXOperatorCallExpr *Operator) { + assert(Operator->getOperator() == OO_Subscript && "Expected []operator"); + + if (0 < Operator->getNumArgs()) + if (auto *ResourceAccess = dyn_cast(Operator->getArg(0))) + if (ResourceAccess->getType()->isHLSLResourceRecord()) + if (auto *VD = dyn_cast(ResourceAccess->getDecl())) + if (auto Assign = Assigns.getAssign(VD)) + if (Assign->Invalidated) { + SemaRef.Diag( + Assign->AssignExpr->getBeginLoc(), + diag::err_hlsl_assigning_local_resource_is_not_unique) + << Assign->AssignExpr << VD; + SemaRef.Diag(VD->getLocation(), diag::note_var_declared_here) + << VD; + return false; + } + return true; +} + // Walks though the global variable declaration, collects all resource binding // requirements and adds them to Bindings void SemaHLSL::collectResourceBindingsOnVarDecl(VarDecl *VD) { @@ -4797,6 +4927,18 @@ bool SemaHLSL::transformInitList(const InitializedEntity &Entity, } bool SemaHLSL::handleInitialization(VarDecl *VDecl, Expr *&Init) { + // If initializing a local resource, register the binding it is using. + if (VDecl->getType()->isHLSLResourceRecord() && !VDecl->hasGlobalStorage()) { + if (auto Binding = getGlobalBinding(Init)) { + Assigns.trackAssign(VDecl, SemaRef.getCurScope(), Init, *Binding); + } else { + SemaRef.Diag(Init->getBeginLoc(), + diag::err_hlsl_assigning_local_resource_is_not_unique) + << Init << VDecl; + return false; + } + } + const HLSLVkConstantIdAttr *ConstIdAttr = VDecl->getAttr(); if (!ConstIdAttr) diff --git a/clang/test/SemaHLSL/local_resource_binding.hlsl b/clang/test/SemaHLSL/local_resource_binding.hlsl new file mode 100644 index 0000000000000..a32416ad9b7a8 --- /dev/null +++ b/clang/test/SemaHLSL/local_resource_binding.hlsl @@ -0,0 +1,84 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -verify %s + +// expected-no-diagnostics + +RWBuffer In : register(u0); +RWStructuredBuffer Out0 : register(u1); +RWStructuredBuffer Out1 : register(u2); +RWStructuredBuffer OutArr[]; + +cbuffer c { + bool cond; +}; + +void overwrite(int idx) { + RWStructuredBuffer Out = Out0; + Out = Out1; + Out[idx] = In[idx]; +} + +void no_initial_assignment(int idx) { + RWStructuredBuffer Out; + if (cond) { + Out = Out1; + } + Out[idx] = In[idx]; +} + +void same_assignment(int idx) { + RWStructuredBuffer Out = Out1; + if (cond) { + Out = Out1; + } + Out[idx] = In[idx]; +} + +void conditional_initialization_with_index(int idx) { + RWStructuredBuffer Out = cond ? OutArr[0] : OutArr[1]; + Out[idx] = In[idx]; +} + +void conditional_assignment_with_index(int idx) { + RWStructuredBuffer Out; + if (cond) { + Out = OutArr[0]; + } else { + Out = OutArr[1]; + } + Out[idx] = In[idx]; +} + +void reassignment(int idx) { + RWStructuredBuffer Out = Out0; + if (cond) { + Out = Out0; + } + Out[idx] = In[idx]; +} + +void conditional_result_in_same(int idx) { + RWStructuredBuffer Out = cond ? Out0 : Out0; + Out[idx] = In[idx]; +} + +void unused_reassign(int idx) { + RWStructuredBuffer Out = Out0; + if (cond) { + Out = Out1; + } + Out = Out0; + Out[idx] = In[idx]; +} + +void scoped_switch(int idx) { + RWStructuredBuffer Out; + switch (idx) { + case 0: Out = Out0; + case 1: Out = Out0; + default: { + Out = Out1; + Out = Out0; + } + } + Out[idx] = In[idx]; +} diff --git a/clang/test/SemaHLSL/local_resource_binding_errs.hlsl b/clang/test/SemaHLSL/local_resource_binding_errs.hlsl new file mode 100644 index 0000000000000..8918b93162555 --- /dev/null +++ b/clang/test/SemaHLSL/local_resource_binding_errs.hlsl @@ -0,0 +1,74 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -verify %s + +RWBuffer In : register(u0); +RWStructuredBuffer Out0 : register(u1); +RWStructuredBuffer Out1 : register(u2); +RWStructuredBuffer OutArr[]; + +cbuffer c { + bool cond; +}; + +void conditional_initialization(int idx) { + // expected-error@+1 {{assignment of 'cond ? Out0 : Out1' to local resource 'Out' is not to the same unique global resource}} + RWStructuredBuffer Out = cond ? Out0 : Out1; + Out[idx] = In[idx]; +} + +void branched_assignment(int idx) { + RWStructuredBuffer Out = Out0; // expected-note {{variable 'Out' is declared here}} + if (cond) { + // expected-error@+1 {{assignment of 'Out1' to local resource 'Out' is not to the same unique global resource}} + Out = Out1; + } + Out[idx] = In[idx]; +} + +void branched_assignment_with_array(int idx) { + RWStructuredBuffer Out = Out0; // expected-note {{variable 'Out' is declared here}} + if (cond) { + // expected-error@+1 {{assignment of 'OutArr[0]' to local resource 'Out' is not to the same unique global resource}} + Out = OutArr[0]; + } + Out[idx] = In[idx]; +} + +void conditional_assignment(int idx) { + RWStructuredBuffer Out; + // expected-error@+1 {{assignment of 'cond ? Out0 : Out1' to local resource 'Out' is not to the same unique global resource}} + Out = cond ? Out0 : Out1; + Out[idx] = In[idx]; +} + +static RWStructuredBuffer StaticOut; + +void static_conditional_assignment(int idx) { + // expected-error@+1 {{assignment of 'cond ? Out0 : Out1' to local resource 'StaticOut' is not to the same unique global resource}} + StaticOut = cond ? Out0 : Out1; + StaticOut[idx] = In[idx]; +} + +void scoped_else(int idx) { + RWStructuredBuffer Out; // expected-note {{variable 'Out' is declared here}} + if (cond) { + Out = Out0; + } else { + // expected-error@+1 {{assignment of 'Out1' to local resource 'Out' is not to the same unique global resource}} + Out = Out1; + } + Out[idx] = In[idx]; +} + +void scoped_switch(int idx) { + RWStructuredBuffer Out; // expected-note {{variable 'Out' is declared here}} + switch (idx) { + case 0: Out = Out0; + case 1: Out = Out0; + default: { + Out = Out0; + // expected-error@+1 {{assignment of 'Out1' to local resource 'Out' is not to the same unique global resource}} + Out = Out1; + } + } + Out[idx] = In[idx]; +}