From 4503186aa94887ab79dea7850819986ee6bba07d Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Wed, 16 Oct 2024 09:34:31 -0400 Subject: [PATCH] [SPIRV] Register all decls for a variable. Some variable can have multiple decls. In one case, there could be a declaration of a const static member variable that is later defined. All of these decls need to be in astDecls and associated with the same variable. The current code will only add the decl for the defintion. This leads to problems when trying to find the variable for the declaration. Fixes #6787 --- tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 41 +++++++++++++------ tools/clang/lib/SPIRV/DeclResultIdMapper.h | 10 +++++ .../test/CodeGenSPIRV/static_member_var.hlsl | 15 +++++++ 3 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/static_member_var.hlsl diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 3084e27a87..bb947d173a 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -1051,7 +1051,7 @@ DeclResultIdMapper::createFnParam(const ParmVarDecl *param, fnParamInstr->setContainsAliasComponent(isAlias); assert(astDecls[param].instr == nullptr); - astDecls[param].instr = fnParamInstr; + registerVariableForDecl(param, fnParamInstr); if (spirvOptions.debugInfoRich) { // Add DebugLocalVariable information @@ -1100,7 +1100,7 @@ DeclResultIdMapper::createFnVar(const VarDecl *var, bool isAlias = false; (void)getTypeAndCreateCounterForPotentialAliasVar(var, &isAlias); varInstr->setContainsAliasComponent(isAlias); - astDecls[var].instr = varInstr; + registerVariableForDecl(var, varInstr); return varInstr; } @@ -1145,7 +1145,7 @@ DeclResultIdMapper::createFileVar(const VarDecl *var, bool isAlias = false; (void)getTypeAndCreateCounterForPotentialAliasVar(var, &isAlias); varInstr->setContainsAliasComponent(isAlias); - astDecls[var].instr = varInstr; + registerVariableForDecl(var, varInstr); createDebugGlobalVariable(varInstr, type, loc, name); @@ -1267,7 +1267,7 @@ SpirvVariable *DeclResultIdMapper::createExternVar(const VarDecl *var, } } - astDecls[var] = createDeclSpirvInfo(varInstr); + registerVariableForDecl(var, createDeclSpirvInfo(varInstr)); createDebugGlobalVariable(varInstr, type, loc, name); @@ -1305,7 +1305,7 @@ SpirvInstruction *DeclResultIdMapper::createResultId(const VarDecl *var) { } SpirvInstruction *init = theEmitter.doExpr(var->getInit()); - astDecls[var] = createDeclSpirvInfo(init); + registerVariableForDecl(var, createDeclSpirvInfo(init)); return init; } @@ -1324,7 +1324,7 @@ DeclResultIdMapper::createOrUpdateStringVar(const VarDecl *var) { const StringLiteral *stringLiteral = dyn_cast(var->getInit()->IgnoreParenCasts()); SpirvString *init = spvBuilder.getString(stringLiteral->getString()); - astDecls[var] = createDeclSpirvInfo(init); + registerVariableForDecl(var, createDeclSpirvInfo(init)); return init; } @@ -1483,7 +1483,7 @@ SpirvVariable *DeclResultIdMapper::createCTBuffer(const HLSLBufferDecl *decl) { if (isResourceType(varDecl->getType())) continue; - astDecls[varDecl] = createDeclSpirvInfo(bufferVar, index++); + registerVariableForDecl(varDecl, createDeclSpirvInfo(bufferVar, index++)); } // If it does not contains a member with non-resource type, we do not want to // set a dedicated binding number. @@ -1549,7 +1549,7 @@ SpirvVariable *DeclResultIdMapper::createPushConstant(const VarDecl *decl) { } // Register the VarDecl - astDecls[decl] = createDeclSpirvInfo(var); + registerVariableForDecl(decl, createDeclSpirvInfo(var)); // Do not push this variable into resourceVars since it does not need // descriptor set. @@ -1600,7 +1600,7 @@ DeclResultIdMapper::createShaderRecordBuffer(const VarDecl *decl, } // Register the VarDecl - astDecls[decl] = createDeclSpirvInfo(var); + registerVariableForDecl(decl, createDeclSpirvInfo(var)); // Do not push this variable into resourceVars since it does not need // descriptor set. @@ -1638,7 +1638,7 @@ DeclResultIdMapper::createShaderRecordBuffer(const HLSLBufferDecl *decl, if (isResourceType(varDecl->getType())) continue; - astDecls[varDecl] = createDeclSpirvInfo(bufferVar, index++); + registerVariableForDecl(varDecl, createDeclSpirvInfo(bufferVar, index++)); } return bufferVar; } @@ -1688,7 +1688,7 @@ void DeclResultIdMapper::createGlobalsCBuffer(const VarDecl *var) { if (isResourceType(varDecl->getType())) continue; - astDecls[varDecl] = createDeclSpirvInfo(globals, index++); + registerVariableForDecl(varDecl, createDeclSpirvInfo(globals, index++)); } } @@ -1801,7 +1801,7 @@ DeclResultIdMapper::getCounterVarFields(const DeclaratorDecl *decl) { void DeclResultIdMapper::registerSpecConstant(const VarDecl *decl, SpirvInstruction *specConstant) { specConstant->setRValue(); - astDecls[decl] = createDeclSpirvInfo(specConstant); + registerVariableForDecl(decl, createDeclSpirvInfo(specConstant)); } void DeclResultIdMapper::createCounterVar( @@ -4817,7 +4817,7 @@ void DeclResultIdMapper::tryToCreateImplicitConstVar(const ValueDecl *decl) { SpirvInstruction *constVal = spvBuilder.getConstantInt(astContext.UnsignedIntTy, val->getInt()); constVal->setRValue(true); - astDecls[varDecl].instr = constVal; + registerVariableForDecl(varDecl, constVal); } void DeclResultIdMapper::decorateWithIntrinsicAttrs( @@ -4880,6 +4880,21 @@ spv::ExecutionMode DeclResultIdMapper::getInterlockExecutionMode() { spv::ExecutionMode::PixelInterlockOrderedEXT); } +void DeclResultIdMapper::registerVariableForDecl(const VarDecl *var, + SpirvInstruction *varInstr) { + DeclSpirvInfo spirvInfo; + spirvInfo.instr = varInstr; + spirvInfo.indexInCTBuffer = -1; + registerVariableForDecl(var, spirvInfo); +} + +void DeclResultIdMapper::registerVariableForDecl(const VarDecl *var, + DeclSpirvInfo spirvInfo) { + for (const auto *v : var->redecls()) { + astDecls[v] = spirvInfo; + } +} + void DeclResultIdMapper::copyHullOutStageVarsToOutputPatch( SpirvInstruction *hullMainOutputPatch, const ParmVarDecl *outputPatchDecl, QualType outputControlPointType, uint32_t numOutputControlPoints) { diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.h b/tools/clang/lib/SPIRV/DeclResultIdMapper.h index 3d9b8450c8..8cfcd3d2a8 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.h +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.h @@ -981,6 +981,16 @@ class DeclResultIdMapper { /// views. void setInterlockExecutionMode(spv::ExecutionMode mode); + /// \brief Add |varInstr| to |astDecls| for every Decl for the variable |var|. + /// It is possible for a variable to have multiple declarations, and all of + /// them should be associated with the same variable. + void registerVariableForDecl(const VarDecl *var, SpirvInstruction *varInstr); + + /// \brief Add |spirvInfo| to |astDecls| for every Decl for the variable + /// |var|. It is possible for a variable to have multiple declarations, and + /// all of them should be associated with the same variable. + void registerVariableForDecl(const VarDecl *var, DeclSpirvInfo spirvInfo); + private: SpirvBuilder &spvBuilder; SpirvEmitter &theEmitter; diff --git a/tools/clang/test/CodeGenSPIRV/static_member_var.hlsl b/tools/clang/test/CodeGenSPIRV/static_member_var.hlsl new file mode 100644 index 0000000000..015fd52b5d --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/static_member_var.hlsl @@ -0,0 +1,15 @@ +// RUN: %dxc -T ps_6_6 -E PSMain %s -spirv | FileCheck %s + +struct PSInput +{ + static const uint Val; + uint Func() { return Val; } +}; + +static const uint PSInput::Val = 3; + +// CHECK: OpStore %out_var_SV_Target0 %uint_3 +uint PSMain(PSInput input) : SV_Target0 +{ + return input.Func(); +}