OpExecutionMode only takes a single ExecutionMode

Previously, the grammar allowed many execution modes for a single
OpExecutionMode instruction.

Removes the variable- and optional- execution mode operand type
enum values.

Issue found by antiagainst@

include/spirv-tools/libspirv.h

@@ -205,8 +205,6 @@ typedef enum spv_operand_type_t {
   SPV_OPERAND_TYPE_OPTIONAL_TYPED_LITERAL_INTEGER,
   // An optional literal string.
   SPV_OPERAND_TYPE_OPTIONAL_LITERAL_STRING,
-  // An optional execution mode.
-  SPV_OPERAND_TYPE_OPTIONAL_EXECUTION_MODE,
   // An optional access qualifier
   SPV_OPERAND_TYPE_OPTIONAL_ACCESS_QUALIFIER,
   // An optional context-independent value, or CIV.  CIVs are tokens that we can
@@ -225,9 +223,7 @@ typedef enum spv_operand_type_t {
   // where the literal number must always be an integer of some sort.
   SPV_OPERAND_TYPE_VARIABLE_LITERAL_INTEGER_ID,
   // A sequence of zero or more pairs of (Id, Literal integer)
-  SPV_OPERAND_TYPE_VARIABLE_ID_LITERAL_INTEGER,
+  LAST_VARIABLE(SPV_OPERAND_TYPE_VARIABLE_ID_LITERAL_INTEGER),
-  // A sequence of zero or more execution modes
-  LAST_VARIABLE(SPV_OPERAND_TYPE_VARIABLE_EXECUTION_MODE),
 
   // This is a sentinel value, and does not represent an operand type.
   // It should come last.

source/binary.cpp

@@ -599,9 +599,6 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
       }
     } break;
 
-    case SPV_OPERAND_TYPE_OPTIONAL_EXECUTION_MODE:
-      parsed_operand.type = SPV_OPERAND_TYPE_EXECUTION_MODE;
-    // Fall through
     case SPV_OPERAND_TYPE_CAPABILITY:
     case SPV_OPERAND_TYPE_SOURCE_LANGUAGE:
     case SPV_OPERAND_TYPE_EXECUTION_MODEL:

source/opcode.cpp

@@ -87,7 +87,14 @@ spv_operand_type_t convertOperandClassToType(SpvOp opcode,
   if (operandClass == OperandOptionalLiteral) {
     switch (opcode) {
       case SpvOpExecutionMode:
-        return SPV_OPERAND_TYPE_VARIABLE_EXECUTION_MODE;
+        // An OpExecutionMode only takes a single ExecutionMode operand and the
+        // operands for that execution mode.  The OperandOptionalLiteral in the
+        // grammar from the spec is only used to generate the text "Optional
+        // literal(s)".  But we've already recorded the
+        // SPV_OPERAND_TYPE_EXECUTION_MODE which will absorb those extra
+        // literals.  Use a NONE operand type here to terminate the operands
+        // to the instruction.
+        return SPV_OPERAND_TYPE_NONE;
       default:
         break;
     }

source/operand.cpp

@@ -1178,7 +1178,6 @@ const char* spvOperandTypeStr(spv_operand_type_t type) {
     case SPV_OPERAND_TYPE_MEMORY_MODEL:
       return "memory model";
     case SPV_OPERAND_TYPE_EXECUTION_MODE:
-    case SPV_OPERAND_TYPE_OPTIONAL_EXECUTION_MODE:
       return "execution mode";
     case SPV_OPERAND_TYPE_STORAGE_CLASS:
       return "storage class";
@@ -1312,10 +1311,6 @@ bool spvExpandOperandSequenceOnce(spv_operand_type_t type,
                       {SPV_OPERAND_TYPE_OPTIONAL_ID,
                        SPV_OPERAND_TYPE_LITERAL_INTEGER, type});
       return true;
-    case SPV_OPERAND_TYPE_VARIABLE_EXECUTION_MODE:
-      pattern->insert(pattern->begin(),
-                      {SPV_OPERAND_TYPE_OPTIONAL_EXECUTION_MODE, type});
-      return true;
     default:
       break;
   }

test/OperandPattern.cpp

@@ -189,9 +189,6 @@ TEST(AlternatePatternFollowingImmediate, SingleElement) {
               Eq(spv_operand_pattern_t{SPV_OPERAND_TYPE_OPTIONAL_CIV}));
   EXPECT_THAT(spvAlternatePatternFollowingImmediate({SPV_OPERAND_TYPE_ID}),
               Eq(spv_operand_pattern_t{SPV_OPERAND_TYPE_OPTIONAL_CIV}));
-  EXPECT_THAT(spvAlternatePatternFollowingImmediate(
-                  {SPV_OPERAND_TYPE_VARIABLE_EXECUTION_MODE}),
-              Eq(spv_operand_pattern_t{SPV_OPERAND_TYPE_OPTIONAL_CIV}));
 }
 
 TEST(AlternatePatternFollowingImmediate, SingleResultId) {
@@ -206,8 +203,7 @@ TEST(AlternatePatternFollowingImmediate, MultipleNonResultIds) {
       spvAlternatePatternFollowingImmediate(
           {SPV_OPERAND_TYPE_VARIABLE_ID_LITERAL_INTEGER,
            SPV_OPERAND_TYPE_CAPABILITY, SPV_OPERAND_TYPE_LOOP_CONTROL,
-           SPV_OPERAND_TYPE_OPTIONAL_LITERAL_INTEGER, SPV_OPERAND_TYPE_ID,
+           SPV_OPERAND_TYPE_OPTIONAL_LITERAL_INTEGER, SPV_OPERAND_TYPE_ID}),
-           SPV_OPERAND_TYPE_VARIABLE_EXECUTION_MODE}),
       Eq(spv_operand_pattern_t{SPV_OPERAND_TYPE_OPTIONAL_CIV}));
 }
 

test/TextToBinary.ModeSetting.cpp

@@ -198,6 +198,12 @@ TEST_F(OpExecutionModeTest, WrongMode) {
               Eq("Invalid execution mode 'xxyyzz'."));
 }
 
+TEST_F(OpExecutionModeTest, TooManyModes) {
+  EXPECT_THAT(CompileFailure("OpExecutionMode %1 Xfb PointMode"),
+              Eq("Expected <opcode> or <result-id> at the beginning of an "
+                 "instruction, found 'PointMode'."));
+}
+
 // Test OpCapability
 
 using OpCapabilityTest = spvtest::TextToBinaryTestBase<