spirv-fuzz: Don't replace memory semantics / scope operands (#4349)

This change is responsible for avoiding the replacement of constant
operands with another one not constant, in the context of atomic
operations.  The related rule from the SPIR-V spec is: "All used for
Scope and Memory Semantics in shader capability must be of an
OpConstant."

Fixes #4346.

source/fuzz/fuzzer_util.cpp

@@ -1616,6 +1616,44 @@ bool IdUseCanBeReplaced(opt::IRContext* ir_context,
     return false;
   }
 
+  if (ir_context->get_feature_mgr()->HasCapability(SpvCapabilityShader)) {
+    // With the Shader capability, memory scope and memory semantics operands
+    // are required to be constants, so they cannot be replaced arbitrarily.
+    switch (use_instruction->opcode()) {
+      case SpvOpAtomicLoad:
+      case SpvOpAtomicStore:
+      case SpvOpAtomicExchange:
+      case SpvOpAtomicIIncrement:
+      case SpvOpAtomicIDecrement:
+      case SpvOpAtomicIAdd:
+      case SpvOpAtomicISub:
+      case SpvOpAtomicSMin:
+      case SpvOpAtomicUMin:
+      case SpvOpAtomicSMax:
+      case SpvOpAtomicUMax:
+      case SpvOpAtomicAnd:
+      case SpvOpAtomicOr:
+      case SpvOpAtomicXor:
+        if (use_in_operand_index == 1 || use_in_operand_index == 2) {
+          return false;
+        }
+        break;
+      case SpvOpAtomicCompareExchange:
+        if (use_in_operand_index == 1 || use_in_operand_index == 2 ||
+            use_in_operand_index == 3) {
+          return false;
+        }
+        break;
+      case SpvOpAtomicCompareExchangeWeak:
+      case SpvOpAtomicFlagTestAndSet:
+      case SpvOpAtomicFlagClear:
+      case SpvOpAtomicFAddEXT:
+        assert(false && "Not allowed with the Shader capability.");
+      default:
+        break;
+    }
+  }
+
   return true;
 }
 

test/fuzz/transformation_replace_id_with_synonym_test.cpp

@@ -1796,27 +1796,8 @@ TEST(TransformationReplaceIdWithSynonymTest, IncompatibleTypes) {
                    .IsApplicable(context.get(), transformation_context));
 }
 
-// TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/4346): This test
-//  should be updated to cover more atomic operations, and once the issue is
-//  fixed the test should be enabled.
 TEST(TransformationReplaceIdWithSynonymTest,
-     DISABLED_AtomicScopeAndMemorySemanticsMustBeConstant) {
+     AtomicScopeAndMemorySemanticsMustBeConstant) {
-  // The following SPIR-V came from this GLSL, edited to add some synonyms:
-  //
-  // #version 320 es
-  //
-  // #extension GL_KHR_memory_scope_semantics : enable
-  //
-  // layout(set = 0, binding = 0) buffer Buf {
-  //   int x;
-  // };
-  //
-  // void main() {
-  //   int tmp = atomicLoad(x,
-  //                        gl_ScopeWorkgroup,
-  //                        gl_StorageSemanticsBuffer,
-  //                        gl_SemanticsRelaxed);
-  // }
   const std::string shader = R"(
                OpCapability Shader
           %1 = OpExtInstImport "GLSL.std.450"
@@ -1832,15 +1813,19 @@ TEST(TransformationReplaceIdWithSynonymTest,
           %2 = OpTypeVoid
           %3 = OpTypeFunction %2
           %6 = OpTypeInt 32 1
+         %17 = OpTypeInt 32 0
           %7 = OpTypePointer Function %6
           %9 = OpTypeStruct %6
          %10 = OpTypePointer StorageBuffer %9
          %11 = OpVariable %10 StorageBuffer
+         %86 = OpTypeStruct %17
+         %87 = OpTypePointer Workgroup %86         
+         %88 = OpVariable %87 Workgroup
          %12 = OpConstant %6 0
          %13 = OpTypePointer StorageBuffer %6
          %15 = OpConstant %6 2
          %16 = OpConstant %6 64
-         %17 = OpTypeInt 32 0
+         %89 = OpTypePointer Workgroup %17
          %18 = OpConstant %17 1
          %19 = OpConstant %17 0
          %20 = OpConstant %17 64
@@ -1850,8 +1835,23 @@ TEST(TransformationReplaceIdWithSynonymTest,
         %100 = OpCopyObject %6 %15 ; A non-constant version of %15
         %101 = OpCopyObject %17 %20 ; A non-constant version of %20
          %14 = OpAccessChain %13 %11 %12
+         %90 = OpAccessChain %89 %88 %19
          %21 = OpAtomicLoad %6 %14 %15 %20
+         %22 = OpAtomicExchange %6 %14 %15 %20 %16
+         %23 = OpAtomicCompareExchange %6 %14 %15 %20 %12 %16 %15
+         %24 = OpAtomicIIncrement %6 %14 %15 %20
+         %25 = OpAtomicIDecrement %6 %14 %15 %20
+         %26 = OpAtomicIAdd %6  %14 %15 %20 %16
+         %27 = OpAtomicISub %6  %14 %15 %20 %16
+         %28 = OpAtomicSMin %6  %14 %15 %20 %16
+         %29 = OpAtomicUMin %17 %90 %15 %20 %18
+         %30 = OpAtomicSMax %6  %14 %15 %20 %15
+         %31 = OpAtomicUMax %17 %90 %15 %20 %18
+         %32 = OpAtomicAnd  %6  %14 %15 %20 %16
+         %33 = OpAtomicOr   %6  %14 %15 %20 %16
+         %34 = OpAtomicXor  %6  %14 %15 %20 %16
                OpStore %8 %21
+               OpAtomicStore %14 %15 %20 %12
                OpReturn
                OpFunctionEnd
   )";
@@ -1872,7 +1872,7 @@ TEST(TransformationReplaceIdWithSynonymTest,
   // Tell the fact manager that %101 and %20 are synonymous
   transformation_context.GetFactManager()->AddFactDataSynonym(
       MakeDataDescriptor(101, {}), MakeDataDescriptor(20, {}));
-
+  // OpAtomicLoad
   const auto& scope_operand = MakeIdUseDescriptorFromUse(
       context.get(), context->get_def_use_mgr()->GetDef(21), 1);
   ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand, 100)
@@ -1882,6 +1882,148 @@ TEST(TransformationReplaceIdWithSynonymTest,
       context.get(), context->get_def_use_mgr()->GetDef(21), 2);
   ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand, 101)
                    .IsApplicable(context.get(), transformation_context));
+  // OpAtomicExchange.
+  const auto& scope_operand2 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(22), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand2, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand2 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(22), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand2, 101)
+                   .IsApplicable(context.get(), transformation_context));
+  // OpAtomicCompareExchange.
+  const auto& scope_operand3 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(23), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand3, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_equal_operand3 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(23), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_equal_operand3, 101)
+                   .IsApplicable(context.get(), transformation_context));
+  const auto& semantics_unequal_operand3 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(23), 3);
+  ASSERT_FALSE(
+      TransformationReplaceIdWithSynonym(semantics_unequal_operand3, 101)
+          .IsApplicable(context.get(), transformation_context));
+  // OpAtomicIIncrement.
+  const auto& scope_operand4 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(24), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand4, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand4 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(24), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand4, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicIDecrement.
+  const auto& scope_operand5 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(25), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand5, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand5 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(25), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand5, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicIAdd.
+  const auto& scope_operand6 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(26), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand6, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand6 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(26), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand6, 101)
+                   .IsApplicable(context.get(), transformation_context));
+  // OpAtomicISub
+  const auto& scope_operand8 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(27), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand8, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand8 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(27), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand8, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicSMin
+  const auto& scope_operand9 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(28), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand9, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand9 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(28), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand9, 101)
+                   .IsApplicable(context.get(), transformation_context));
+  // OpAtomicUMin
+  const auto& scope_operand10 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(29), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand10, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand10 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(29), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand10, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicSMax
+  const auto& scope_operand11 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(30), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand11, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand11 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(30), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand11, 101)
+                   .IsApplicable(context.get(), transformation_context));
+  // OpAtomicUMax
+  const auto& scope_operand12 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(31), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand12, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand12 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(31), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand12, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicAnd
+  const auto& scope_operand13 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(32), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand13, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand13 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(32), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand13, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicOr
+  const auto& scope_operand14 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(33), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand14, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand14 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(33), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand14, 101)
+                   .IsApplicable(context.get(), transformation_context));
+
+  // OpAtomicXor
+  const auto& scope_operand15 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(34), 1);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(scope_operand15, 100)
+                   .IsApplicable(context.get(), transformation_context));
+
+  const auto& semantics_operand15 = MakeIdUseDescriptorFromUse(
+      context.get(), context->get_def_use_mgr()->GetDef(34), 2);
+  ASSERT_FALSE(TransformationReplaceIdWithSynonym(semantics_operand15, 101)
+                   .IsApplicable(context.get(), transformation_context));
 }
 
 // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/4345): Improve this