Assert on unimplemented instructions
Replace the runtime `warning` with something that will actually crash
us, and add stub implementations of OpLabel and OpReturn to allow
trivial shaders to work.
There's little point in continuing beyond an unimplemented instruction
-- if you get lucky, you hit a crash on use of the instruction's result.
If you get unlucky, you wander off into undefined behavior.
Change-Id: I3b222ea74446754276096c81fc3669c311872316
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/26088
Tested-by: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index ed2beee..77d3393 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -31,6 +31,13 @@
// - There is exactly one entrypoint in the module, and it's the one we want
// - The only input/output OpVariables present are those used by the entrypoint
+ // TODO: Add real support for control flow. For now, track whether we've seen
+ // a label or a return already (if so, the shader does things we will mishandle).
+ // We expect there to be one of each in a simple shader -- the first and last instruction
+ // of the entrypoint function.
+ bool seenLabel = false;
+ bool seenReturn = false;
+
for (auto insn : *this)
{
switch (insn.opcode())
@@ -101,6 +108,18 @@
break;
}
+ case spv::OpLabel:
+ if (seenLabel)
+ UNIMPLEMENTED("Shader contains multiple labels, has control flow");
+ seenLabel = true;
+ break;
+
+ case spv::OpReturn:
+ if (seenReturn)
+ UNIMPLEMENTED("Shader contains multiple returns, has control flow");
+ seenReturn = true;
+ break;
+
case spv::OpTypeVoid:
case spv::OpTypeBool:
case spv::OpTypeInt:
@@ -278,7 +297,6 @@
}
case spv::OpStore:
- case spv::OpReturn:
// Don't need to do anything during analysis pass
break;
@@ -287,8 +305,7 @@
break;
default:
- printf("Warning: ignored opcode %s\n", OpcodeName(insn.opcode()).c_str());
- break; // This is OK, these passes are intentionally partial
+ UNIMPLEMENTED(OpcodeName(insn.opcode()).c_str());
}
}
}
@@ -875,6 +892,14 @@
// or don't require any work at all.
break;
+ case spv::OpLabel:
+ case spv::OpReturn:
+ // TODO: when we do control flow, will need to do some work here.
+ // Until then, there is nothing to do -- we expect there to be an initial OpLabel
+ // in the entrypoint function, for which we do nothing; and a final OpReturn at the
+ // end of the entrypoint function, for which we do nothing.
+ break;
+
case spv::OpVariable:
EmitVariable(insn, routine);
break;
@@ -956,7 +981,7 @@
break;
default:
- printf("emit: ignoring opcode %s\n", OpcodeName(insn.opcode()).c_str());
+ UNIMPLEMENTED(OpcodeName(insn.opcode()).c_str());
break;
}
}
