Skip to content

Commit

Permalink
Check for undefined %id in spirv_asm block. (#5966)
Browse files Browse the repository at this point in the history
  • Loading branch information
csyonghe authored Dec 31, 2024
1 parent 1f98bd6 commit b7eb585
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 1 deletion.
62 changes: 62 additions & 0 deletions source/slang/slang-check-expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5311,6 +5311,10 @@ Expr* SemanticsExprVisitor::visitSPIRVAsmExpr(SPIRVAsmExpr* expr)
// We will iterate over all the operands in all the insts and check
// them
bool failed = false;

// Track %id's that have been defined in this asm block.
HashSet<Name*> definedIds;

for (auto& inst : expr->insts)
{
// It's not automatically a failure to not have info, we just won't
Expand All @@ -5327,6 +5331,52 @@ Expr* SemanticsExprVisitor::visitSPIRVAsmExpr(SPIRVAsmExpr* expr)
0);
continue;
}
int resultIdIndex = -1;
if (opInfo)
{
resultIdIndex = opInfo->resultIdIndex;
}
else if (inst.opcode.flavor == SPIRVAsmOperand::TruncateMarker)
{
// If this is __truncate, register the result id in the third operand.
resultIdIndex = 1;
}
else
{
// If there is no opInfo, just register all Ids as defined.
for (auto& operand : inst.operands)
{
if (operand.flavor == SPIRVAsmOperand::Id)
{
definedIds.add(operand.token.getName());
}
}
}

// Register result ID.
if (resultIdIndex != -1)
{
if (inst.operands.getCount() <= resultIdIndex)
{
failed = true;
getSink()->diagnose(
inst.opcode.token,
Diagnostics::spirvInstructionWithNotEnoughOperands,
inst.opcode.token);
continue;
}
auto& resultIdOperand = inst.operands[resultIdIndex];

if (!definedIds.add(resultIdOperand.token.getName()))
{
failed = true;
getSink()->diagnose(
inst.opcode.token,
Diagnostics::spirvIdRedefinition,
inst.opcode.token);
continue;
}
}

const bool isLast = &inst == &expr->insts.getLast();
for (Index operandIndex = 0; operandIndex < inst.operands.getCount(); ++operandIndex)
Expand Down Expand Up @@ -5438,6 +5488,18 @@ Expr* SemanticsExprVisitor::visitSPIRVAsmExpr(SPIRVAsmExpr* expr)
}
operand.knownValue = builtinVarKind.value();
}
else if (operand.flavor == SPIRVAsmOperand::Id)
{
if (!definedIds.contains(operand.token.getName()))
{
failed = true;
getSink()->diagnose(
operand.token,
Diagnostics::spirvUndefinedId,
operand.token);
return;
}
}
if (operand.bitwiseOrWith.getCount() &&
operand.flavor != SPIRVAsmOperand::Literal &&
operand.flavor != SPIRVAsmOperand::NamedValue)
Expand Down
12 changes: 11 additions & 1 deletion source/slang/slang-diagnostic-defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,17 @@ DIAGNOSTIC(
Error,
spirvInvalidTruncate,
"__truncate has been given a source smaller than its target")

DIAGNOSTIC(29112, Error, spirvInstructionWithNotEnoughOperands, "not enough operands for $0")
DIAGNOSTIC(
29113,
Error,
spirvIdRedefinition,
"SPIRV id '%$0' is already defined in the current assembly block")
DIAGNOSTIC(
29114,
Error,
spirvUndefinedId,
"SPIRV id '%$0' is not defined in the current assembly block location")
//
// 3xxxx - Semantic analysis
//
Expand Down
10 changes: 10 additions & 0 deletions tests/diagnostics/invalid-spv-obj.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//TEST:SIMPLE(filecheck=CHECK): -target spirv

[shader("compute")]
[numthreads(1,1,1)]
void main() {
int var2 = spirv_asm {
// CHECK: ([[# @LINE+1]]): error 29114
result: $$int = OpIMul $(15) %invalidId
};
}

0 comments on commit b7eb585

Please sign in to comment.