Skip to content

Commit

Permalink
[SPIRV] Emit DebugFunction/Definition for both wrapper and real funct…
Browse files Browse the repository at this point in the history 
…ions (#6966)

#6758
unfortunately caused a regression for shaders compiled without -fcgl.
This PR extends the original fix and corrects the regression. We now
emit DebugFunction and DebugFunctionDefinition pairs for both the
wrapper and the main functions, and a DebugEntrypoint that points at the
wrapper function. This survives inlining.
  • Loading branch information
@SteveUrquhart
SteveUrquhart authored Nov 19, 2024
1 parent 72b353a commit 891392e
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 90 deletions.
143 changes: 96 additions & 47 deletions tools/clang/lib/SPIRV/SpirvEmitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1442,48 +1442,34 @@ void SpirvEmitter::doFunctionDecl(const FunctionDecl *decl) {
// This will allow the entry-point name to be something like
// myNamespace::myEntrypointFunc.
std::string funcName = getFnName(decl);
std::string debugFuncName = funcName;

std::string srcFuncName = "src." + funcName;
SpirvFunction *func = declIdMapper.getOrRegisterFn(decl);
SpirvFunction *srcFunc = nullptr;

auto loc = decl->getLocStart();
auto range = decl->getSourceRange();
RichDebugInfo *info = nullptr;
SpirvDebugFunction *debugFunction = nullptr;
SpirvDebugFunction *srcDebugFunction = nullptr;
SpirvDebugInstruction *outer_scope = spvContext.getCurrentLexicalScope();
const auto &sm = astContext.getSourceManager();
if (spirvOptions.debugInfoRich && decl->hasBody()) {
const uint32_t line = sm.getPresumedLineNumber(loc);
const uint32_t column = sm.getPresumedColumnNumber(loc);
info = getOrCreateRichDebugInfo(loc);

auto *source = info->source;
// Note that info->scopeStack.back() is a lexical scope of the function
// caller.
auto *parentScope = info->compilationUnit;
// TODO: figure out the proper flag based on the function decl.
// using FlagIsPublic for now.
uint32_t flags = 3u;
// The line number in the source program at which the function scope begins.
auto scopeLine = sm.getPresumedLineNumber(decl->getBody()->getLocStart());
debugFunction = spvBuilder.createDebugFunction(decl, debugFuncName, source,
line, column, parentScope,
"", flags, scopeLine, func);
func->setDebugScope(new (spvContext) SpirvDebugScope(debugFunction));

spvContext.pushDebugLexicalScope(info, debugFunction);
}

bool isEntry = false;
const auto iter = functionInfoMap.find(decl);
if (iter != functionInfoMap.end()) {
const auto &entryInfo = iter->second;
if (entryInfo->isEntryFunction) {
isEntry = true;
funcName = "src." + funcName;
// Create wrapper for the entry function
if (!emitEntryFunctionWrapper(decl, func))
srcFunc = func;

func = emitEntryFunctionWrapper(decl, &info, &debugFunction, srcFunc);
if (func == nullptr)
return;
if (spirvOptions.debugInfoRich && decl->hasBody()) {
// use the HLSL name the debug user will expect to see
srcDebugFunction = emitDebugFunction(decl, srcFunc, &info, funcName);
}

// Generate DebugEntryPoint if function definition
if (spirvOptions.debugInfoVulkan && debugFunction) {
auto *cu = dyn_cast<SpirvDebugCompilationUnit>(outer_scope);
Expand All @@ -1492,15 +1478,30 @@ void SpirvEmitter::doFunctionDecl(const FunctionDecl *decl) {
clang::getGitCommitHash(),
spirvOptions.clOptions);
}
} else {
if (spirvOptions.debugInfoRich && decl->hasBody()) {
debugFunction = emitDebugFunction(decl, func, &info, funcName);
}
}
}

if (spirvOptions.debugInfoRich) {
spvContext.pushDebugLexicalScope(info, debugFunction);
}

const QualType retType =
declIdMapper.getTypeAndCreateCounterForPotentialAliasVar(decl);

spvBuilder.beginFunction(retType, decl->getLocStart(), funcName,
decl->hasAttr<HLSLPreciseAttr>(),
decl->hasAttr<NoInlineAttr>(), func);
if (srcFunc) {
spvBuilder.beginFunction(retType, decl->getLocStart(), srcFuncName,
decl->hasAttr<HLSLPreciseAttr>(),
decl->hasAttr<NoInlineAttr>(), srcFunc);

} else {
spvBuilder.beginFunction(retType, decl->getLocStart(), funcName,
decl->hasAttr<HLSLPreciseAttr>(),
decl->hasAttr<NoInlineAttr>(), func);
}

bool isNonStaticMemberFn = false;
if (const auto *memberFn = dyn_cast<CXXMethodDecl>(decl)) {
Expand Down Expand Up @@ -1551,7 +1552,9 @@ void SpirvEmitter::doFunctionDecl(const FunctionDecl *decl) {

// Add DebugFunctionDefinition if we are emitting
// NonSemantic.Shader.DebugInfo.100 debug info
if (spirvOptions.debugInfoVulkan && debugFunction)
if (spirvOptions.debugInfoVulkan && srcDebugFunction)
spvBuilder.createDebugFunctionDef(srcDebugFunction, srcFunc);
else if (spirvOptions.debugInfoVulkan && debugFunction)
spvBuilder.createDebugFunctionDef(debugFunction, func);

// Process all statments in the body.
Expand Down Expand Up @@ -13331,11 +13334,17 @@ bool SpirvEmitter::processTessellationShaderAttributes(
}

bool SpirvEmitter::emitEntryFunctionWrapperForRayTracing(
const FunctionDecl *decl, SpirvFunction *entryFuncInstr) {
const FunctionDecl *decl, SpirvDebugFunction *debugFunction,
SpirvFunction *entryFuncInstr) {
// The entry basic block.
auto *entryLabel = spvBuilder.createBasicBlock();
spvBuilder.setInsertPoint(entryLabel);

// Add DebugFunctionDefinition if we are emitting
// NonSemantic.Shader.DebugInfo.100 debug info.
if (spirvOptions.debugInfoVulkan && debugFunction)
spvBuilder.createDebugFunctionDef(debugFunction, entryFunction);

// Initialize all global variables at the beginning of the wrapper
for (const VarDecl *varDecl : toInitGloalVars) {
const auto varInfo =
Expand Down Expand Up @@ -13598,8 +13607,36 @@ bool SpirvEmitter::processMeshOrAmplificationShaderAttributes(
return true;
}

bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
SpirvFunction *entryFuncInstr) {
SpirvDebugFunction *SpirvEmitter::emitDebugFunction(const FunctionDecl *decl,
SpirvFunction *func,
RichDebugInfo **info,
std::string name) {
auto loc = decl->getLocStart();
const auto &sm = astContext.getSourceManager();
const uint32_t line = sm.getPresumedLineNumber(loc);
const uint32_t column = sm.getPresumedColumnNumber(loc);
*info = getOrCreateRichDebugInfo(loc);

SpirvDebugSource *source = (*info)->source;
// Note that info->scopeStack.back() is a lexical scope of the function
// caller.
SpirvDebugInstruction *parentScope = (*info)->compilationUnit;
// TODO: figure out the proper flag based on the function decl.
// using FlagIsPublic for now.
uint32_t flags = 3u;
// The line number in the source program at which the function scope begins.
auto scopeLine = sm.getPresumedLineNumber(decl->getBody()->getLocStart());
SpirvDebugFunction *debugFunction =
spvBuilder.createDebugFunction(decl, name, source, line, column,
parentScope, "", flags, scopeLine, func);
func->setDebugScope(new (spvContext) SpirvDebugScope(debugFunction));

return debugFunction;
}

SpirvFunction *SpirvEmitter::emitEntryFunctionWrapper(
const FunctionDecl *decl, RichDebugInfo **info,
SpirvDebugFunction **debugFunction, SpirvFunction *entryFuncInstr) {
// HS specific attributes
uint32_t numOutputControlPoints = 0;
SpirvInstruction *outputControlPointIdVal =
Expand All @@ -13620,6 +13657,10 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
entryFunction = spvBuilder.beginFunction(
astContext.VoidTy, decl->getLocStart(), decl->getName());

if (spirvOptions.debugInfoRich && decl->hasBody()) {
*debugFunction = emitDebugFunction(decl, entryFunction, info, "wrapper");
}

// Specify that entryFunction is an entry function wrapper.
entryFunction->setEntryFunctionWrapper();

Expand All @@ -13634,7 +13675,10 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
entryInfo->entryFunction = entryFunction;

if (spvContext.isRay()) {
return emitEntryFunctionWrapperForRayTracing(decl, entryFuncInstr);
return emitEntryFunctionWrapperForRayTracing(decl, *debugFunction,
entryFuncInstr)
? entryFunction
: nullptr;
}
// Handle attributes specific to each shader stage
if (spvContext.isPS()) {
Expand All @@ -13643,7 +13687,7 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
processComputeShaderAttributes(decl);
} else if (spvContext.isHS()) {
if (!processTessellationShaderAttributes(decl, &numOutputControlPoints))
return false;
return nullptr;

// The input array size for HS is specified in the InputPatch parameter.
for (const auto *param : decl->params())
Expand All @@ -13655,7 +13699,7 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
outputArraySize = numOutputControlPoints;
} else if (spvContext.isDS()) {
if (!processTessellationShaderAttributes(decl, &numOutputControlPoints))
return false;
return nullptr;

// The input array size for HS is specified in the OutputPatch parameter.
for (const auto *param : decl->params())
Expand All @@ -13666,11 +13710,11 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
// The per-vertex output of DS is not an array.
} else if (spvContext.isGS()) {
if (!processGeometryShaderAttributes(decl, &inputArraySize))
return false;
return nullptr;
// The per-vertex output of GS is not an array.
} else if (spvContext.isMS() || spvContext.isAS()) {
if (!processMeshOrAmplificationShaderAttributes(decl, &outputArraySize))
return false;
return nullptr;
}

// Go through all parameters and record the declaration of SV_ClipDistance
Expand All @@ -13686,14 +13730,14 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
for (const auto *param : decl->params()) {
if (canActAsInParmVar(param))
if (!declIdMapper.glPerVertex.recordGlPerVertexDeclFacts(param, true))
return false;
return nullptr;
if (canActAsOutParmVar(param))
if (!declIdMapper.glPerVertex.recordGlPerVertexDeclFacts(param, false))
return false;
return nullptr;
}
// Also consider the SV_ClipDistance/SV_CullDistance in the return type
if (!declIdMapper.glPerVertex.recordGlPerVertexDeclFacts(decl, false))
return false;
return nullptr;

// Calculate the total size of the ClipDistance/CullDistance array and the
// offset of SV_ClipDistance/SV_CullDistance variables within the array.
Expand All @@ -13715,6 +13759,11 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
// after the basic block is created and insert point is set.
processInlineSpirvAttributes(decl);

// Add DebugFunctionDefinition if we are emitting
// NonSemantic.Shader.DebugInfo.100 debug info.
if (spirvOptions.debugInfoVulkan && *debugFunction)
spvBuilder.createDebugFunctionDef(*debugFunction, entryFunction);

// Initialize all global variables at the beginning of the wrapper
for (const VarDecl *varDecl : toInitGloalVars) {
// SPIR-V does not have string variables
Expand Down Expand Up @@ -13766,7 +13815,7 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
SpirvInstruction *loadedValue = nullptr;

if (!declIdMapper.createStageInputVar(param, &loadedValue, false))
return false;
return nullptr;

// Only initialize the temporary variable if the parameter is indeed used,
// or if it is an inout parameter.
Expand Down Expand Up @@ -13805,15 +13854,15 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
// Create stage output variables out of the return type.
if (!declIdMapper.createStageOutputVar(decl, numOutputControlPoints,
outputControlPointIdVal, retVal))
return false;
return nullptr;
if (!processHSEntryPointOutputAndPCF(
decl, retType, retVal, numOutputControlPoints,
outputControlPointIdVal, primitiveIdVar, viewIdVar,
hullMainInputPatchParam))
return false;
return nullptr;
} else {
if (!declIdMapper.createStageOutputVar(decl, retVal, /*forPCF*/ false))
return false;
return nullptr;
}

// Create and write stage output variables for parameters marked as
Expand All @@ -13836,7 +13885,7 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
param->getLocStart());

if (!declIdMapper.createStageOutputVar(param, loadedParam, false))
return false;
return nullptr;
}
}

Expand All @@ -13851,7 +13900,7 @@ bool SpirvEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
if (spvContext.isHS())
doDecl(patchConstFunc);

return true;
return entryFunction;
}

bool SpirvEmitter::processHSEntryPointOutputAndPCF(
Expand Down
17 changes: 13 additions & 4 deletions tools/clang/lib/SPIRV/SpirvEmitter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -848,8 +848,14 @@ class SpirvEmitter : public ASTConsumer {
processMeshOrAmplificationShaderAttributes(const FunctionDecl *decl,
uint32_t *outVerticesArraySize);

/// \brief Emits a wrapper function for the entry function and returns true
/// on success.
/// \brief Emits a SpirvDebugFunction to match given SpirvFunction, and
/// returns a pointer to it.
SpirvDebugFunction *emitDebugFunction(const FunctionDecl *decl,
SpirvFunction *func,
RichDebugInfo **info, std::string name);

/// \brief Emits a wrapper function for the entry function and returns a
/// pointer to the wrapper SpirvFunction on success.
///
/// The wrapper function loads the values of all stage input variables and
/// creates composites as expected by the source code entry function. It then
Expand All @@ -859,8 +865,10 @@ class SpirvEmitter : public ASTConsumer {
///
/// The wrapper function is also responsible for initializing global static
/// variables for some cases.
bool emitEntryFunctionWrapper(const FunctionDecl *entryFunction,
SpirvFunction *entryFuncId);
SpirvFunction *emitEntryFunctionWrapper(const FunctionDecl *entryFunction,
RichDebugInfo **info,
SpirvDebugFunction **debugFunction,
SpirvFunction *entryFuncId);

/// \brief Emits a wrapper function for the entry functions for raytracing
/// stages and returns true on success.
Expand All @@ -870,6 +878,7 @@ class SpirvEmitter : public ASTConsumer {
/// The wrapper function is also responsible for initializing global static
/// variables for some cases.
bool emitEntryFunctionWrapperForRayTracing(const FunctionDecl *entryFunction,
SpirvDebugFunction *debugFunction,
SpirvFunction *entryFuncId);

/// \brief Performs the following operations for the Hull shader:
Expand Down
14 changes: 8 additions & 6 deletions tools/clang/test/CodeGenSPIRV/rich.debug.debugscope.hlsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
// CHECK: [[set:%[0-9]+]] = OpExtInstImport "OpenCL.DebugInfo.100"
// CHECK: [[compUnit:%[0-9]+]] = OpExtInst %void [[set]] DebugCompilationUnit
// CHECK: [[main:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction
// CHECK: [[mainFnLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 15 1 [[main]]
// CHECK: [[whileLoopLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 35 3 [[mainFnLexBlock]]
// CHECK: [[ifStmtLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 42 20 [[whileLoopLexBlock]]
// CHECK: [[tempLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 47 7 [[ifStmtLexBlock]]
// CHECK: [[forLoopLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 20 12 [[mainFnLexBlock]]
// CHECK: [[mainFnLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 17 1 [[main]]
// CHECK: [[whileLoopLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 37 3 [[mainFnLexBlock]]
// CHECK: [[ifStmtLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 44 20 [[whileLoopLexBlock]]
// CHECK: [[tempLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 49 7 [[ifStmtLexBlock]]
// CHECK: [[forLoopLexBlock:%[0-9]+]] = OpExtInst %void [[set]] DebugLexicalBlock {{%[0-9]+}} 22 12 [[mainFnLexBlock]]
// CHECK: [[srcMain:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction

float4 main(float4 color : COLOR) : SV_TARGET
// CHECK: %main = OpFunction
// CHECK: %src_main = OpFunction
// CHECK-NEXT: {{%[0-9]+}} = OpExtInst %void [[set]] DebugScope [[main]]
// CHECK-NEXT: {{%[0-9]+}} = OpExtInst %void [[set]] DebugScope [[srcMain]]
{
// CHECK: %bb_entry = OpLabel
// CHECK-NEXT: {{%[0-9]+}} = OpExtInst %void [[set]] DebugScope [[mainFnLexBlock]]
Expand Down
14 changes: 8 additions & 6 deletions tools/clang/test/CodeGenSPIRV/rich.debug.function.param.hlsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@
// CHECK: [[emptyStr:%[0-9]+]] = OpString ""
// CHECK: [[y:%[0-9]+]] = OpString "y"
// CHECK: [[x:%[0-9]+]] = OpString "x"
// CHECK: [[mainName:%[0-9]+]] = OpString "main"
// CHECK: [[mainName:%[0-9]+]] = OpString "wrapper"
// CHECK: [[color:%[0-9]+]] = OpString "color"
// CHECK: [[srcMainName:%[0-9]+]] = OpString "main"

// CHECK: [[int:%[0-9]+]] = OpExtInst %void [[set]] DebugTypeBasic {{%[0-9]+}} %uint_32 Signed
// CHECK: [[float:%[0-9]+]] = OpExtInst %void [[set]] DebugTypeBasic {{%[0-9]+}} %uint_32 Float
// CHECK: [[source:%[0-9]+]] = OpExtInst %void [[set]] DebugSource
// CHECK: [[foo:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction [[fooName]] {{%[0-9]+}} [[source]] 23 1 {{%[0-9]+}} [[emptyStr]] FlagIsProtected|FlagIsPrivate 24 %foo
// CHECK: {{%[0-9]+}} = OpExtInst %void [[set]] DebugLocalVariable [[y]] [[float]] [[source]] 23 23 [[foo]] FlagIsLocal 2
// CHECK: {{%[0-9]+}} = OpExtInst %void [[set]] DebugLocalVariable [[x]] [[int]] [[source]] 23 14 [[foo]] FlagIsLocal 1
// CHECK: [[foo:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction [[fooName]] {{%[0-9]+}} [[source]] 25 1 {{%[0-9]+}} [[emptyStr]] FlagIsProtected|FlagIsPrivate 26 %foo
// CHECK: {{%[0-9]+}} = OpExtInst %void [[set]] DebugLocalVariable [[y]] [[float]] [[source]] 25 23 [[foo]] FlagIsLocal 2
// CHECK: {{%[0-9]+}} = OpExtInst %void [[set]] DebugLocalVariable [[x]] [[int]] [[source]] 25 14 [[foo]] FlagIsLocal 1
// CHECK: [[srcMain:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction [[mainName]] {{%[0-9]+}} [[source]] 30 1 {{%[0-9]+}} [[emptyStr]] FlagIsProtected|FlagIsPrivate 31 %main
// CHECK: [[float4:%[0-9]+]] = OpExtInst %void [[set]] DebugTypeVector [[float]] 4
// CHECK: [[main:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction [[mainName]] {{%[0-9]+}} [[source]] 28 1 {{%[0-9]+}} [[emptyStr]] FlagIsProtected|FlagIsPrivate 29 %src_main
// CHECK: {{%[0-9]+}} = OpExtInst %void [[set]] DebugLocalVariable [[color]] [[float4]] [[source]] 28 20 [[main]] FlagIsLocal 1
// CHECK: {{%[0-9]+}} = OpExtInst %void [[set]] DebugLocalVariable [[color]] [[float4]] [[source]] 30 20 [[srcMain]] FlagIsLocal 1
// CHECK: [[main:%[0-9]+]] = OpExtInst %void [[set]] DebugFunction [[srcMainName]] {{%[0-9]+}} [[source]] 30 1 {{%[0-9]+}} [[emptyStr]] FlagIsProtected|FlagIsPrivate 31 %src_main

void foo(int x, float y)
{
Expand Down
Loading

0 comments on commit 891392e

Please sign in to comment.