Execution scope, memory semantics operands are IDs

They shouldn't be parsed or printed as masks.

include/libspirv/libspirv.h

@@ -172,8 +172,9 @@ typedef enum spv_operand_type_t {
   SPV_OPERAND_TYPE_SELECTION_CONTROL,
   SPV_OPERAND_TYPE_LOOP_CONTROL,
   SPV_OPERAND_TYPE_FUNCTION_CONTROL,
-  SPV_OPERAND_TYPE_MEMORY_SEMANTICS,
 
+  // The ID for a memory semantics value.
+  SPV_OPERAND_TYPE_MEMORY_SEMANTICS,
   // The ID for an execution scope value.
   SPV_OPERAND_TYPE_EXECUTION_SCOPE,
 

readme.md

@@ -28,7 +28,7 @@ The validator is incomplete.  See the Future Work section for more information.
 
 ## CHANGES (for tools hackers)
 
-2015-09-24
+2015-09-25
 * Updated to Rev32 headers
   * Core instructions and enumerants from Rev 32 are supported by the
     assembler.
@@ -44,6 +44,8 @@ The validator is incomplete.  See the Future Work section for more information.
     map to that number. E.g. `%2` doesn't necessarily map to ID 2.
 * Disassembler emits mask expressions for mask combinations instead of
   erroring out.
+* Fixed parsing and printing of Execution Scope and Memory Semantics
+  operands.  They are supplied as IDs, not as literals.
 
 2015-09-18
 * MILESTONE: This version of the assembler supports all of SPIR-V Rev31,

source/binary.cpp

@@ -222,10 +222,12 @@ spv_result_t spvBinaryDecodeOperand(
       print && spvIsInBitfield(SPV_BINARY_TO_TEXT_OPTION_COLOR, options);
 
   switch (type) {
+    case SPV_OPERAND_TYPE_EXECUTION_SCOPE:
     case SPV_OPERAND_TYPE_ID:
-    case SPV_OPERAND_TYPE_RESULT_ID:
+    case SPV_OPERAND_TYPE_ID_IN_OPTIONAL_TUPLE:
     case SPV_OPERAND_TYPE_OPTIONAL_ID:
-    case SPV_OPERAND_TYPE_ID_IN_OPTIONAL_TUPLE: {
+    case SPV_OPERAND_TYPE_MEMORY_SEMANTICS:
+    case SPV_OPERAND_TYPE_RESULT_ID: {
       if (color) {
         if (type == SPV_OPERAND_TYPE_RESULT_ID) {
           stream.get() << clr::blue();
@@ -311,8 +313,6 @@ spv_result_t spvBinaryDecodeOperand(
     case SPV_OPERAND_TYPE_FUNCTION_PARAMETER_ATTRIBUTE:
     case SPV_OPERAND_TYPE_DECORATION:
     case SPV_OPERAND_TYPE_BUILT_IN:
-    case SPV_OPERAND_TYPE_MEMORY_SEMANTICS:
-    case SPV_OPERAND_TYPE_EXECUTION_SCOPE:
     case SPV_OPERAND_TYPE_GROUP_OPERATION:
     case SPV_OPERAND_TYPE_KERNEL_ENQ_FLAGS:
     case SPV_OPERAND_TYPE_KERNEL_PROFILING_INFO: {

source/text.cpp

@@ -351,6 +351,7 @@ bool isIdType(spv_operand_type_t type) {
     case SPV_OPERAND_TYPE_EXECUTION_SCOPE:
     case SPV_OPERAND_TYPE_ID:
     case SPV_OPERAND_TYPE_ID_IN_OPTIONAL_TUPLE:
+    case SPV_OPERAND_TYPE_MEMORY_SEMANTICS:
     case SPV_OPERAND_TYPE_OPTIONAL_ID:
     case SPV_OPERAND_TYPE_RESULT_ID:
       return true;
@@ -437,11 +438,12 @@ spv_result_t spvTextEncodeOperand(
   }
 
   switch (type) {
+    case SPV_OPERAND_TYPE_EXECUTION_SCOPE:
     case SPV_OPERAND_TYPE_ID:
     case SPV_OPERAND_TYPE_ID_IN_OPTIONAL_TUPLE:
     case SPV_OPERAND_TYPE_OPTIONAL_ID:
-    case SPV_OPERAND_TYPE_RESULT_ID:
-    case SPV_OPERAND_TYPE_EXECUTION_SCOPE: {
+    case SPV_OPERAND_TYPE_MEMORY_SEMANTICS:
+    case SPV_OPERAND_TYPE_RESULT_ID: {
       if ('%' == textValue[0]) {
         textValue++;
       } else {
@@ -559,7 +561,6 @@ spv_result_t spvTextEncodeOperand(
     case SPV_OPERAND_TYPE_FP_FAST_MATH_MODE:
     case SPV_OPERAND_TYPE_FUNCTION_CONTROL:
     case SPV_OPERAND_TYPE_LOOP_CONTROL:
-    case SPV_OPERAND_TYPE_MEMORY_SEMANTICS:
     case SPV_OPERAND_TYPE_OPTIONAL_IMAGE:
     case SPV_OPERAND_TYPE_OPTIONAL_MEMORY_ACCESS:
     case SPV_OPERAND_TYPE_SELECTION_CONTROL: {

test/TextToBinary.Barrier.cpp

@@ -40,46 +40,13 @@ using ::testing::Eq;
 
 // Test OpMemoryBarrier
 
-using MemorySemanticsTest = spvtest::TextToBinaryTestBase<
-    ::testing::TestWithParam<EnumCase<spv::MemorySemanticsMask>>>;
+using OpMemoryBarrier = spvtest::TextToBinaryTest;
 
-TEST_P(MemorySemanticsTest, AnySingleMemorySemanticsMask) {
-  std::string input = "OpMemoryBarrier %1 " + GetParam().name();
-  EXPECT_THAT(
-      CompiledInstructions(input),
-      Eq(MakeInstruction(spv::OpMemoryBarrier, {1, GetParam().value()})));
-}
-
-#define CASE(NAME)                              \
-  {                                             \
-    spv::MemorySemantics##NAME##Mask, #NAME, {} \
-  }
-INSTANTIATE_TEST_CASE_P(
-    TextToBinaryMemorySemanticsTest, MemorySemanticsTest,
-    ::testing::ValuesIn(std::vector<EnumCase<spv::MemorySemanticsMask>>{
-        {spv::MemorySemanticsMaskNone, "None", {}},
-        // Relaxed is a synonym for None.
-        {spv::MemorySemanticsMaskNone, "Relaxed", {}},
-        CASE(Acquire),
-        CASE(Release),
-        CASE(SequentiallyConsistent),
-        CASE(UniformMemory),
-        CASE(SubgroupMemory),
-        CASE(WorkgroupLocalMemory),
-        CASE(WorkgroupGlobalMemory),
-        CASE(AtomicCounterMemory),
-        CASE(ImageMemory),
-    }));
-#undef CASE
-
-TEST_F(TextToBinaryTest, CombinedMemorySemanticsMask) {
-  // Sample a single combination.  This ensures we've integrated
-  // the instruction parsing logic with spvTextParseMask.
-  const std::string input = "OpMemoryBarrier %1 Acquire|WorkgroupLocalMemory";
-  const uint32_t expected_mask = spv::MemorySemanticsAcquireMask |
-                                 spv::MemorySemanticsWorkgroupLocalMemoryMask;
+TEST_F(OpMemoryBarrier, Sample) {
+  std::string input = "OpMemoryBarrier %1 %2\n";
   EXPECT_THAT(CompiledInstructions(input),
-              Eq(MakeInstruction(spv::OpMemoryBarrier, {1, expected_mask})));
+              Eq(MakeInstruction(spv::OpMemoryBarrier, {1, 2})));
+  EXPECT_THAT(EncodeAndDecodeSuccessfully(input), Eq(input));
 }
 
 // TODO(dneto): OpControlBarrier