Add operand type for extension instruction number

This is required to support extended instructions that
have literal numbers as operands.  An example is OpenCL's
vloadn.

The previous code in the assembler assumed that *any* literal
number argument in any part of an OpExtInst must be the name
of the extended instruction.  That's true only for the first
literal number argument.

diff --git a/include/libspirv/libspirv.h b/include/libspirv/libspirv.h
index 6dea226..75008ea 100644 (file)
--- a/include/libspirv/libspirv.h
+++ b/include/libspirv/libspirv.h
@@ -236,6 +236,11 @@ typedef enum spv_operand_type_t {
   // a literal floating-point number.
   SPV_OPERAND_TYPE_OPTIONAL_LITERAL_NUMBER,
 
+  // The Instruction argument to OpExtInst. It's an unsigned 32-bit literal
+  // number indicating which instruction to use from an extended instruction
+  // set.
+  SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER,
+
   // This is a sentinel value, and does not represent an operand type.
   // It should come last.
   SPV_OPERAND_TYPE_NUM_OPERAND_TYPES,

diff --git a/source/binary.cpp b/source/binary.cpp
index 83cf452..e7fc61d 100644 (file)
--- a/source/binary.cpp
+++ b/source/binary.cpp
@@ -208,8 +208,7 @@ spv_result_t spvBinaryDecodeOperand(
       stream.get() << ((color) ? clr::reset() : "");
       position->index++;
     } break;
-    case SPV_OPERAND_TYPE_LITERAL_INTEGER: {
-      // NOTE: Special case for extended instruction use
+    case SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER: {
       if (OpExtInst == opcode) {
         spv_ext_inst_desc extInst;
         if (grammar.lookupExtInst(*pExtInstType, words[0], &extInst)) {
@@ -221,9 +220,14 @@ spv_result_t spvBinaryDecodeOperand(
         stream.get() << extInst->name;
         stream.get() << (color ? clr::reset() : "");
         position->index++;
-        break;
+      } else {
+        DIAGNOSTIC << "Internal error: grammar thinks we need an "
+                      "extension instruction number for opcode "
+                   << opcode;
+        return SPV_ERROR_INTERNAL;
       }
-    }  // Fall through for the general case.
+    } break;
+    case SPV_OPERAND_TYPE_LITERAL_INTEGER:
     case SPV_OPERAND_TYPE_MULTIWORD_LITERAL_NUMBER:
     case SPV_OPERAND_TYPE_OPTIONAL_LITERAL_INTEGER:
     case SPV_OPERAND_TYPE_LITERAL_INTEGER_IN_OPTIONAL_TUPLE: {

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 57bd663..8946cde 100644 (file)
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -110,7 +110,17 @@ spv_operand_type_t convertOperandClassToType(spv::Op opcode,
     case OperandOptionalLiteralString: return SPV_OPERAND_TYPE_OPTIONAL_LITERAL_STRING;
     // This is only used for sequences of literal numbers.
     case OperandVariableLiterals: return SPV_OPERAND_TYPE_VARIABLE_LITERAL_INTEGER;
-    case OperandLiteralNumber: return SPV_OPERAND_TYPE_LITERAL_INTEGER;
+    case OperandLiteralNumber:
+      if (opcode == spv::OpExtInst) {
+        // We use a special operand type for the extension instruction number.
+        // For now, we assume there is only one LiteraNumber argument to OpExtInst,
+        // and it is the extension instruction argument.
+        // See the ExtInst entry in opcode.inc
+        // TODO(dneto): Use a function to confirm the assumption, and to verify
+        // that the index into the operandClass is 1, as expected.
+        return SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER;
+      }
+      return SPV_OPERAND_TYPE_LITERAL_INTEGER;
     case OperandLiteralString: return SPV_OPERAND_TYPE_LITERAL_STRING;
     case OperandSource: return SPV_OPERAND_TYPE_SOURCE_LANGUAGE;
     case OperandExecutionModel: return SPV_OPERAND_TYPE_EXECUTION_MODEL;

diff --git a/source/text.cpp b/source/text.cpp
index 4a65a71..3e09200 100644 (file)
--- a/source/text.cpp
+++ b/source/text.cpp
@@ -234,22 +234,22 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
       if (type == SPV_OPERAND_TYPE_TYPE_ID) pInst->resultTypeId = id;
       spvInstructionAddWord(pInst, id);
     } break;
-    case SPV_OPERAND_TYPE_LITERAL_INTEGER: {
-      // NOTE: Special case for extension instruction lookup
-      if (OpExtInst == pInst->opcode) {
-        spv_ext_inst_desc extInst;
-        if (grammar.lookupExtInst(pInst->extInstType, textValue, &extInst)) {
-          return context->diagnostic() << "Invalid extended instruction name '"
-                                       << textValue << "'.";
-        }
-        spvInstructionAddWord(pInst, extInst->ext_inst);
+    case SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER: {
+      // The assembler accepts the symbolic name for an extended instruction,
+      // and emits its corresponding number.
+      spv_ext_inst_desc extInst;
+      if (grammar.lookupExtInst(pInst->extInstType, textValue, &extInst)) {
+        return context->diagnostic() << "Invalid extended instruction name '"
+                                     << textValue << "'.";
+      }
+      spvInstructionAddWord(pInst, extInst->ext_inst);
 
-        // Prepare to parse the operands for the extended instructions.
-        spvPrependOperandTypes(extInst->operandTypes, pExpectedOperands);
+      // Prepare to parse the operands for the extended instructions.
+      spvPrependOperandTypes(extInst->operandTypes, pExpectedOperands);
 
-        return SPV_SUCCESS;
-      }
-    }  // Fall through for the general case.
+      return SPV_SUCCESS;
+    } break;
+    case SPV_OPERAND_TYPE_LITERAL_INTEGER:
     case SPV_OPERAND_TYPE_MULTIWORD_LITERAL_NUMBER:
     case SPV_OPERAND_TYPE_LITERAL_INTEGER_IN_OPTIONAL_TUPLE:
     case SPV_OPERAND_TYPE_OPTIONAL_LITERAL_NUMBER: