xenia-project/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/xenia-project/SPIRV-Tools.git synced 2024-11-23 11:19:41 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix ExtInst parsing: no IdRef* at end

Browse Source 
The operands following the extended instruction literal
number are determined by the extended instruction itself.
So drop the zero-or-more IdRef pattern at the end of OpExtInst.

It's arguable whether this should actually be a grammar fix.  I've
chosen to patch this in SPIRV-Tools instead of in the grammar file.

Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/233

Also fix two test cases for OpenCL extended instructions.  These
errors of supplying too many operands are now detected.
This commit is contained in:
David Neto 2016-06-14 16:41:27 -04:00
parent 37e4600c3e
commit 1a18739650
4 changed files with 31 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
test/BinaryParse.cpp
View File

@ -663,6 +663,13 @@ INSTANTIATE_TEST_CASE_P(
MakeInstruction(SpvOpExtInst, {2, 3, 100, 4, 5})}),
"OpExtInst set Id 100 does not reference an OpExtInstImport result "
"Id"},
{Concatenate({ExpectedHeaderForBound(101),
MakeInstruction(SpvOpExtInstImport, {100},
MakeVector("OpenCL.std")),
// OpenCL cos is #14
MakeInstruction(SpvOpExtInst, {2, 3, 100, 14, 5, 999})}),
"Invalid instruction OpExtInst starting at word 10: expected no "
"more operands after 6 words, but stated word count is 7."},
// In this case, the OpSwitch selector refers to an invalid ID.
{Concatenate({ExpectedHeaderForBound(3),
MakeInstruction(SpvOpSwitch, {1, 2, 42, 3})}),

4
test/ExtInst.OpenCL.std.cpp
View File

@ -233,8 +233,8 @@ INSTANTIATE_TEST_CASE_P(
CASE1(Popcount, popcount),
CASE3(SMad24, s_mad24),
CASE3(UMad24, u_mad24),
CASE3(SMul24, s_mul24),
CASE3(UMul24, u_mul24), // enum value 170
CASE2(SMul24, s_mul24),
CASE2(UMul24, u_mul24), // enum value 170
CASE1(UAbs, u_abs), // enum value 201
CASE2(UAbs_diff, u_abs_diff),
CASE2(UMul_hi, u_mul_hi),

6
test/TextToBinary.Extension.cpp
View File

@ -68,6 +68,12 @@ TEST_F(TextToBinaryTest, MultiImport) {
Eq("Import Id is being defined a second time"));
}
TEST_F(TextToBinaryTest, TooManyArguments) {
const std::string input = R"(%opencl = OpExtInstImport "OpenCL.std"
%2 = OpExtInst %float %opencl cos %x %oops")";
EXPECT_THAT(CompileFailure(input), Eq("Expected '=', found end of stream."));
}
TEST_F(TextToBinaryTest, ExtInstFromTwoDifferentImports) {
const std::string input = R"(%1 = OpExtInstImport "OpenCL.std"
%2 = OpExtInstImport "GLSL.std.450"

16
utils/generate_grammar_tables.py
View File

@ -141,10 +141,26 @@ class InstInitializer(object):
self.caps_mask = compose_capability_mask(caps)
self.operands = [convert_operand_kind(o) for o in operands]
self.fix_syntax()
operands = [o[0] for o in operands]
self.ref_type_id = 'IdResultType' in operands
self.def_result_id = 'IdResult' in operands
def fix_syntax(self):
"""Fix an instruction's syntax, adjusting for differences between
the officially released grammar and how SPIRV-Tools uses the grammar.
Fixes:
- ExtInst should not end with SPV_OPERAND_VARIABLE_ID.
https://github.com/KhronosGroup/SPIRV-Tools/issues/233
"""
if (self.opname == 'ExtInst'
and self.operands[-1] == 'SPV_OPERAND_TYPE_VARIABLE_ID'):
self.operands.pop()
def __str__(self):
template = ['{{"{opname}"', 'SpvOp{opname}', '{caps_mask}',
'{num_operands}', '{{{operands}}}',