Skip to content

Commit

Permalink
Disable NRVO for entry functions and patch constant functions (micros…
Browse files Browse the repository at this point in the history 
…oft#4466)

Disable NRVO for entry functions and patch constant functions.

For entry functions (and patch constant functions) each write to an output argument creates a new call to dx.storeOutput (or dx.storePatchConstant). With RVO enabled, every write to the return variable becomes an output instruction, which could be excessive. To avoid this, disable NRVO for any entry functions and patch constant functions.
  • Loading branch information
@adam-yang
adam-yang authored May 19, 2022
1 parent 2e49e68 commit 24909be
Show file tree
Hide file tree
Showing 7 changed files with 309 additions and 76 deletions.
1 change: 1 addition & 0 deletions tools/clang/include/clang/AST/HlslTypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ bool IsUserDefinedRecordType(clang::QualType type);
bool DoesTypeDefineOverloadedOperator(clang::QualType typeWithOperator,
clang::OverloadedOperatorKind opc,
clang::QualType paramType);
bool IsPatchConstantFunctionDecl(const clang::FunctionDecl *FD);

/// <summary>Adds a function declaration to the specified class record.</summary>
/// <param name="context">ASTContext that owns declarations.</param>
Expand Down
2 changes: 2 additions & 0 deletions tools/clang/include/clang/Sema/SemaHLSL.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ clang::OverloadingResult GetBestViableFunction(
clang::OverloadCandidateSet& set,
clang::OverloadCandidateSet::iterator& Best);

bool ShouldSkipNRVO(clang::Sema &sema, clang::QualType returnType, clang::VarDecl *VD, clang::FunctionDecl *FD);

/// <summary>Processes an attribute for a declaration.</summary>
/// <param name="S">Sema with context.</param>
/// <param name="D">Annotated declaration.</param>
Expand Down
49 changes: 1 addition & 48 deletions tools/clang/lib/AST/ASTContextHLSL.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1205,53 +1205,6 @@ UnusualAnnotation* hlsl::UnusualAnnotation::CopyToASTContext(ASTContext& Context
return (UnusualAnnotation*)result;
}

static bool HasTessFactorSemantic(const ValueDecl *decl) {
for (const UnusualAnnotation *it : decl->getUnusualAnnotations()) {
if (it->getKind() == UnusualAnnotation::UA_SemanticDecl) {
const SemanticDecl *sd = cast<SemanticDecl>(it);
const Semantic *pSemantic = Semantic::GetByName(sd->SemanticName);
if (pSemantic && pSemantic->GetKind() == Semantic::Kind::TessFactor)
return true;
}
}
return false;
}

static bool HasTessFactorSemanticRecurse(const ValueDecl *decl, QualType Ty) {
if (Ty->isBuiltinType() || hlsl::IsHLSLVecMatType(Ty))
return false;

if (const RecordType *RT = Ty->getAsStructureType()) {
RecordDecl *RD = RT->getDecl();
for (FieldDecl *fieldDecl : RD->fields()) {
if (HasTessFactorSemanticRecurse(fieldDecl, fieldDecl->getType()))
return true;
}
return false;
}

if (Ty->getAsArrayTypeUnsafe())
return HasTessFactorSemantic(decl);

return false;
}

bool ASTContext::IsPatchConstantFunctionDecl(const FunctionDecl *FD) const {
// This checks whether the function is structurally capable of being a patch
// constant function, not whether it is in fact the patch constant function
// for the entry point of a compiled hull shader (which may not have been
// seen yet). So the answer is conservative.
if (!FD->getReturnType()->isVoidType()) {
// Try to find TessFactor in return type.
if (HasTessFactorSemanticRecurse(FD, FD->getReturnType()))
return true;
}
// Try to find TessFactor in out param.
for (const ParmVarDecl *param : FD->params()) {
if (param->hasAttr<HLSLOutAttr>()) {
if (HasTessFactorSemanticRecurse(param, param->getType()))
return true;
}
}
return false;
return hlsl::IsPatchConstantFunctionDecl(FD);
}
55 changes: 55 additions & 0 deletions tools/clang/lib/AST/HlslTypes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
///////////////////////////////////////////////////////////////////////////////

#include "dxc/Support/Global.h"
#include "dxc/DXIL/DxilSemantic.h"
#include "clang/AST/CanonicalType.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/HlslTypes.h"
Expand Down Expand Up @@ -600,6 +601,60 @@ bool IsUserDefinedRecordType(clang::QualType type) {
return false;
}

static bool HasTessFactorSemantic(const ValueDecl *decl) {
for (const UnusualAnnotation *it : decl->getUnusualAnnotations()) {
if (it->getKind() == UnusualAnnotation::UA_SemanticDecl) {
const SemanticDecl *sd = cast<SemanticDecl>(it);
StringRef semanticName;
unsigned int index = 0;
Semantic::DecomposeNameAndIndex(sd->SemanticName, &semanticName, &index);
const hlsl::Semantic *pSemantic = hlsl::Semantic::GetByName(semanticName);
if (pSemantic && pSemantic->GetKind() == hlsl::Semantic::Kind::TessFactor)
return true;
}
}
return false;
}

static bool HasTessFactorSemanticRecurse(const ValueDecl *decl, QualType Ty) {
if (Ty->isBuiltinType() || hlsl::IsHLSLVecMatType(Ty))
return false;

if (const RecordType *RT = Ty->getAsStructureType()) {
RecordDecl *RD = RT->getDecl();
for (FieldDecl *fieldDecl : RD->fields()) {
if (HasTessFactorSemanticRecurse(fieldDecl, fieldDecl->getType()))
return true;
}
return false;
}

if (Ty->getAsArrayTypeUnsafe())
return HasTessFactorSemantic(decl);

return false;
}

bool IsPatchConstantFunctionDecl(const clang::FunctionDecl *FD) {
// This checks whether the function is structurally capable of being a patch
// constant function, not whether it is in fact the patch constant function
// for the entry point of a compiled hull shader (which may not have been
// seen yet). So the answer is conservative.
if (!FD->getReturnType()->isVoidType()) {
// Try to find TessFactor in return type.
if (HasTessFactorSemanticRecurse(FD, FD->getReturnType()))
return true;
}
// Try to find TessFactor in out param.
for (const ParmVarDecl *param : FD->params()) {
if (param->hasAttr<HLSLOutAttr>()) {
if (HasTessFactorSemanticRecurse(param, param->getType()))
return true;
}
}
return false;
}

bool DoesTypeDefineOverloadedOperator(clang::QualType typeWithOperator,
clang::OverloadedOperatorKind opc,
clang::QualType paramType) {
Expand Down
58 changes: 58 additions & 0 deletions tools/clang/lib/Sema/SemaHLSL.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11069,6 +11069,64 @@ unsigned hlsl::CaculateInitListArraySizeForHLSL(
}
}

// NRVO unsafe for a variety of cases in HLSL
// - vectors/matrix with bool component types
// - attributes not captured to QualType, such as precise and globallycoherent
bool hlsl::ShouldSkipNRVO(clang::Sema& sema, clang::QualType returnType, clang::VarDecl *VD, clang::FunctionDecl *FD) {
// exclude vectors/matrix (not treated as record type)
// NRVO breaks on bool component type due to diff between
// i32 memory and i1 register representation
if (hlsl::IsHLSLVecMatType(returnType))
return true;
QualType ArrayEltTy = returnType;
while (const clang::ArrayType *AT =
sema.getASTContext().getAsArrayType(ArrayEltTy)) {
ArrayEltTy = AT->getElementType();
}
// exclude resource for globallycoherent.
if (hlsl::IsHLSLResourceType(ArrayEltTy))
return true;
// exclude precise.
if (VD->hasAttr<HLSLPreciseAttr>()) {
return true;
}
if (FD) {
// propagate precise the the VD.
if (FD->hasAttr<HLSLPreciseAttr>()) {
VD->addAttr(FD->getAttr<HLSLPreciseAttr>());
return true;
}

// Don't do NRVO if this is an entry function or a patch contsant function.
// With NVRO, writing to the return variable directly writes to the output
// argument instead of to an alloca which gets copied to the output arg in one
// spot. This causes many extra dx.storeOutput's to be emitted.
//
// Check if this is an entry function the easy way if we're a library
if (const HLSLShaderAttr *Attr = FD->getAttr<HLSLShaderAttr>()) {
return true;
}
// Check if it's an entry function the hard way
if (!FD->getDeclContext()->isNamespace() && FD->isGlobal()) {
// Check if this is an entry function by comparing name
if (FD->getName() == sema.getLangOpts().HLSLEntryFunction) {
return true;
}

// See if it's the patch constant function
if (sema.getLangOpts().HLSLProfile.size() &&
(sema.getLangOpts().HLSLProfile[0] == 'h' /*For 'hs'*/ ||
sema.getLangOpts().HLSLProfile[0] == 'l' /*For 'lib'*/))
{
if (hlsl::IsPatchConstantFunctionDecl(FD))
return true;
}
}
}

return false;
}

bool hlsl::IsConversionToLessOrEqualElements(
_In_ clang::Sema* self,
const clang::ExprResult& sourceExpr,
Expand Down
30 changes: 2 additions & 28 deletions tools/clang/lib/Sema/SemaStmt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2702,34 +2702,8 @@ VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType,
return nullptr;

// HLSL Change Begins: NRVO unsafe for a variety of cases in HLSL
// - vectors/matrix with bool component types
// - attributes not captured to QualType, such as precise and globallycoherent
if (getLangOpts().HLSL) {
// exclude vectors/matrix (not treated as record type)
// NRVO breaks on bool component type due to diff between
// i32 memory and i1 register representation
if (hlsl::IsHLSLVecMatType(ReturnType))
return nullptr;
QualType ArrayEltTy = ReturnType;
while (const clang::ArrayType *AT =
Context.getAsArrayType(ArrayEltTy)) {
ArrayEltTy = AT->getElementType();
}
// exclude resource for globallycoherent.
if (hlsl::IsHLSLResourceType(ArrayEltTy))
return nullptr;
// exclude precise.
if (VD->hasAttr<HLSLPreciseAttr>()) {
return nullptr;
}
// propagate precise the the VD.
if (const FunctionDecl *FD = getCurFunctionDecl()) {
if (FD->hasAttr<HLSLPreciseAttr>()) {
VD->addAttr(FD->getAttr<HLSLPreciseAttr>());
return nullptr;
}
}
}
if (getLangOpts().HLSL && hlsl::ShouldSkipNRVO(*this, ReturnType, VD, getCurFunctionDecl()))
return nullptr;
// HLSL Change Ends

if (isCopyElisionCandidate(ReturnType, VD, AllowFunctionParameter))
Expand Down
Loading

0 comments on commit 24909be

Please sign in to comment.