Commit e57f10e4 by Chris Forbes

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/+/26088Tested-by: 's avatarChris Forbes <chrisforbes@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
parent e95eeb19
...@@ -31,6 +31,13 @@ namespace sw ...@@ -31,6 +31,13 @@ namespace sw
// - There is exactly one entrypoint in the module, and it's the one we want // - 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 // - 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) for (auto insn : *this)
{ {
switch (insn.opcode()) switch (insn.opcode())
...@@ -101,6 +108,18 @@ namespace sw ...@@ -101,6 +108,18 @@ namespace sw
break; 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::OpTypeVoid:
case spv::OpTypeBool: case spv::OpTypeBool:
case spv::OpTypeInt: case spv::OpTypeInt:
...@@ -278,7 +297,6 @@ namespace sw ...@@ -278,7 +297,6 @@ namespace sw
} }
case spv::OpStore: case spv::OpStore:
case spv::OpReturn:
// Don't need to do anything during analysis pass // Don't need to do anything during analysis pass
break; break;
...@@ -287,8 +305,7 @@ namespace sw ...@@ -287,8 +305,7 @@ namespace sw
break; break;
default: default:
printf("Warning: ignored opcode %s\n", OpcodeName(insn.opcode()).c_str()); UNIMPLEMENTED(OpcodeName(insn.opcode()).c_str());
break; // This is OK, these passes are intentionally partial
} }
} }
} }
...@@ -875,6 +892,14 @@ namespace sw ...@@ -875,6 +892,14 @@ namespace sw
// or don't require any work at all. // or don't require any work at all.
break; 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: case spv::OpVariable:
EmitVariable(insn, routine); EmitVariable(insn, routine);
break; break;
...@@ -956,7 +981,7 @@ namespace sw ...@@ -956,7 +981,7 @@ namespace sw
break; break;
default: default:
printf("emit: ignoring opcode %s\n", OpcodeName(insn.opcode()).c_str()); UNIMPLEMENTED(OpcodeName(insn.opcode()).c_str());
break; break;
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment