Fix operand access (#3427)
Fixes #3426.
diff --git a/source/fuzz/transformation_set_memory_operands_mask.cpp b/source/fuzz/transformation_set_memory_operands_mask.cpp
index 131a499..f52f170 100644
--- a/source/fuzz/transformation_set_memory_operands_mask.cpp
+++ b/source/fuzz/transformation_set_memory_operands_mask.cpp
@@ -101,6 +101,14 @@
// Either add a new operand, if no mask operand was already present, or
// replace an existing mask operand.
if (original_mask_in_operand_index >= instruction->NumInOperands()) {
+ // Add first memory operand if it's missing.
+ if (message_.memory_operands_mask_index() == 1 &&
+ GetInOperandIndexForMask(*instruction, 0) >=
+ instruction->NumInOperands()) {
+ instruction->AddOperand(
+ {SPV_OPERAND_TYPE_MEMORY_ACCESS, {SpvMemoryAccessMaskNone}});
+ }
+
instruction->AddOperand(
{SPV_OPERAND_TYPE_MEMORY_ACCESS, {message_.memory_operands_mask()}});
@@ -154,11 +162,26 @@
break;
}
// If we are looking for the input operand index of the first mask, return it.
+ // This will also return a correct value if the operand is missing.
if (mask_index == 0) {
return first_mask_in_operand_index;
}
assert(mask_index == 1 && "Memory operands mask index must be 0 or 1.");
+ // Memory mask operands are optional. Thus, if the second operand exists,
+ // its index will be >= |first_mask_in_operand_index + 1|. We can reason as
+ // follows to separate the cases where the index of the second operand is
+ // equal to |first_mask_in_operand_index + 1|:
+ // - If the first memory operand doesn't exist, its value is equal to None.
+ // This means that it doesn't have additional operands following it and the
+ // condition in the if statement below will be satisfied.
+ // - If the first memory operand exists and has no additional memory operands
+ // following it, the condition in the if statement below will be satisfied
+ // and we will return the correct value from the function.
+ if (first_mask_in_operand_index + 1 >= instruction.NumInOperands()) {
+ return first_mask_in_operand_index + 1;
+ }
+
// We are looking for the input operand index of the second mask. This is a
// little complicated because, depending on the contents of the first mask,
// there may be some input operands separating the two masks.
diff --git a/test/fuzz/transformation_set_memory_operands_mask_test.cpp b/test/fuzz/transformation_set_memory_operands_mask_test.cpp
index c02d8d4..518ce9d 100644
--- a/test/fuzz/transformation_set_memory_operands_mask_test.cpp
+++ b/test/fuzz/transformation_set_memory_operands_mask_test.cpp
@@ -75,6 +75,7 @@
%21 = OpAccessChain %20 %17 %19
OpCopyMemory %12 %21 Aligned 16
OpCopyMemory %133 %12 Volatile
+ OpCopyMemory %133 %12
%136 = OpAccessChain %135 %17 %30
%138 = OpAccessChain %24 %12 %19
OpCopyMemory %138 %136 None
@@ -109,12 +110,14 @@
0)
.IsApplicable(context.get(), transformation_context));
- TransformationSetMemoryOperandsMask transformation1(
- MakeInstructionDescriptor(147, SpvOpLoad, 0),
- SpvMemoryAccessAlignedMask | SpvMemoryAccessVolatileMask, 0);
- ASSERT_TRUE(
- transformation1.IsApplicable(context.get(), transformation_context));
- transformation1.Apply(context.get(), &transformation_context);
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(147, SpvOpLoad, 0),
+ SpvMemoryAccessAlignedMask | SpvMemoryAccessVolatileMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
// Not OK to remove Aligned
ASSERT_FALSE(TransformationSetMemoryOperandsMask(
@@ -128,15 +131,17 @@
SpvMemoryAccessAlignedMask, 0)
.IsApplicable(context.get(), transformation_context));
- // OK: adds Nontemporal and Volatile
- TransformationSetMemoryOperandsMask transformation2(
- MakeInstructionDescriptor(21, SpvOpCopyMemory, 0),
- SpvMemoryAccessAlignedMask | SpvMemoryAccessNontemporalMask |
- SpvMemoryAccessVolatileMask,
- 0);
- ASSERT_TRUE(
- transformation2.IsApplicable(context.get(), transformation_context));
- transformation2.Apply(context.get(), &transformation_context);
+ {
+ // OK: adds Nontemporal and Volatile
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 0),
+ SpvMemoryAccessAlignedMask | SpvMemoryAccessNontemporalMask |
+ SpvMemoryAccessVolatileMask,
+ 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
// Not OK to remove Volatile
ASSERT_FALSE(TransformationSetMemoryOperandsMask(
@@ -150,29 +155,45 @@
SpvMemoryAccessAlignedMask | SpvMemoryAccessVolatileMask, 0)
.IsApplicable(context.get(), transformation_context));
- // OK: adds Nontemporal
- TransformationSetMemoryOperandsMask transformation3(
- MakeInstructionDescriptor(21, SpvOpCopyMemory, 1),
- SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 0);
- ASSERT_TRUE(
- transformation3.IsApplicable(context.get(), transformation_context));
- transformation3.Apply(context.get(), &transformation_context);
+ {
+ // OK: adds Nontemporal
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 1),
+ SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- // OK: adds Nontemporal and Volatile
- TransformationSetMemoryOperandsMask transformation4(
- MakeInstructionDescriptor(138, SpvOpCopyMemory, 0),
- SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 0);
- ASSERT_TRUE(
- transformation4.IsApplicable(context.get(), transformation_context));
- transformation4.Apply(context.get(), &transformation_context);
+ {
+ // OK: adds Nontemporal (creates new operand)
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 2),
+ SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- // OK: removes Nontemporal, adds Volatile
- TransformationSetMemoryOperandsMask transformation5(
- MakeInstructionDescriptor(148, SpvOpStore, 0),
- SpvMemoryAccessVolatileMask, 0);
- ASSERT_TRUE(
- transformation5.IsApplicable(context.get(), transformation_context));
- transformation5.Apply(context.get(), &transformation_context);
+ {
+ // OK: adds Nontemporal and Volatile
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(138, SpvOpCopyMemory, 0),
+ SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
+
+ {
+ // OK: removes Nontemporal, adds Volatile
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(148, SpvOpStore, 0),
+ SpvMemoryAccessVolatileMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
std::string after_transformation = R"(
OpCapability Shader
@@ -228,6 +249,7 @@
%21 = OpAccessChain %20 %17 %19
OpCopyMemory %12 %21 Aligned|Nontemporal|Volatile 16
OpCopyMemory %133 %12 Nontemporal|Volatile
+ OpCopyMemory %133 %12 Nontemporal|Volatile
%136 = OpAccessChain %135 %17 %30
%138 = OpAccessChain %24 %12 %19
OpCopyMemory %138 %136 Nontemporal|Volatile
@@ -296,6 +318,8 @@
%21 = OpAccessChain %20 %17 %19
OpCopyMemory %12 %21 Aligned 16 Nontemporal|Aligned 16
OpCopyMemory %133 %12 Volatile
+ OpCopyMemory %133 %12
+ OpCopyMemory %133 %12
%136 = OpAccessChain %135 %17 %30
%138 = OpAccessChain %24 %12 %19
OpCopyMemory %138 %136 None Aligned 16
@@ -318,63 +342,95 @@
TransformationContext transformation_context(&fact_manager,
validator_options);
- TransformationSetMemoryOperandsMask transformation1(
- MakeInstructionDescriptor(21, SpvOpCopyMemory, 0),
- SpvMemoryAccessAlignedMask | SpvMemoryAccessVolatileMask, 1);
- // Bad: cannot remove aligned
- ASSERT_FALSE(TransformationSetMemoryOperandsMask(
- MakeInstructionDescriptor(21, SpvOpCopyMemory, 0),
- SpvMemoryAccessVolatileMask, 1)
- .IsApplicable(context.get(), transformation_context));
- ASSERT_TRUE(
- transformation1.IsApplicable(context.get(), transformation_context));
- transformation1.Apply(context.get(), &transformation_context);
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 0),
+ SpvMemoryAccessAlignedMask | SpvMemoryAccessVolatileMask, 1);
+ // Bad: cannot remove aligned
+ ASSERT_FALSE(TransformationSetMemoryOperandsMask(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 0),
+ SpvMemoryAccessVolatileMask, 1)
+ .IsApplicable(context.get(), transformation_context));
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- TransformationSetMemoryOperandsMask transformation2(
- MakeInstructionDescriptor(21, SpvOpCopyMemory, 1),
- SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 1);
- // Bad: cannot remove volatile
- ASSERT_FALSE(TransformationSetMemoryOperandsMask(
- MakeInstructionDescriptor(21, SpvOpCopyMemory, 1),
- SpvMemoryAccessNontemporalMask, 0)
- .IsApplicable(context.get(), transformation_context));
- ASSERT_TRUE(
- transformation2.IsApplicable(context.get(), transformation_context));
- transformation2.Apply(context.get(), &transformation_context);
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 1),
+ SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 1);
+ // Bad: cannot remove volatile
+ ASSERT_FALSE(TransformationSetMemoryOperandsMask(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 1),
+ SpvMemoryAccessNontemporalMask, 0)
+ .IsApplicable(context.get(), transformation_context));
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- TransformationSetMemoryOperandsMask transformation3(
- MakeInstructionDescriptor(138, SpvOpCopyMemory, 0),
- SpvMemoryAccessAlignedMask | SpvMemoryAccessNontemporalMask, 1);
- // Bad: the first mask is None, so Aligned cannot be added to it.
- ASSERT_FALSE(TransformationSetMemoryOperandsMask(
- MakeInstructionDescriptor(138, SpvOpCopyMemory, 0),
- SpvMemoryAccessAlignedMask | SpvMemoryAccessNontemporalMask,
- 0)
- .IsApplicable(context.get(), transformation_context));
- ASSERT_TRUE(
- transformation3.IsApplicable(context.get(), transformation_context));
- transformation3.Apply(context.get(), &transformation_context);
+ {
+ // Creates the first operand.
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 2),
+ SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- TransformationSetMemoryOperandsMask transformation4(
- MakeInstructionDescriptor(138, SpvOpCopyMemory, 1),
- SpvMemoryAccessVolatileMask, 1);
- ASSERT_TRUE(
- transformation4.IsApplicable(context.get(), transformation_context));
- transformation4.Apply(context.get(), &transformation_context);
+ {
+ // Creates both operands.
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(21, SpvOpCopyMemory, 3),
+ SpvMemoryAccessNontemporalMask | SpvMemoryAccessVolatileMask, 1);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- TransformationSetMemoryOperandsMask transformation5(
- MakeInstructionDescriptor(147, SpvOpLoad, 0),
- SpvMemoryAccessVolatileMask | SpvMemoryAccessAlignedMask, 0);
- ASSERT_TRUE(
- transformation5.IsApplicable(context.get(), transformation_context));
- transformation5.Apply(context.get(), &transformation_context);
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(138, SpvOpCopyMemory, 0),
+ SpvMemoryAccessAlignedMask | SpvMemoryAccessNontemporalMask, 1);
+ // Bad: the first mask is None, so Aligned cannot be added to it.
+ ASSERT_FALSE(
+ TransformationSetMemoryOperandsMask(
+ MakeInstructionDescriptor(138, SpvOpCopyMemory, 0),
+ SpvMemoryAccessAlignedMask | SpvMemoryAccessNontemporalMask, 0)
+ .IsApplicable(context.get(), transformation_context));
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
- TransformationSetMemoryOperandsMask transformation6(
- MakeInstructionDescriptor(148, SpvOpStore, 0), SpvMemoryAccessMaskNone,
- 0);
- ASSERT_TRUE(
- transformation6.IsApplicable(context.get(), transformation_context));
- transformation6.Apply(context.get(), &transformation_context);
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(138, SpvOpCopyMemory, 1),
+ SpvMemoryAccessVolatileMask, 1);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
+
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(147, SpvOpLoad, 0),
+ SpvMemoryAccessVolatileMask | SpvMemoryAccessAlignedMask, 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
+
+ {
+ TransformationSetMemoryOperandsMask transformation(
+ MakeInstructionDescriptor(148, SpvOpStore, 0), SpvMemoryAccessMaskNone,
+ 0);
+ ASSERT_TRUE(
+ transformation.IsApplicable(context.get(), transformation_context));
+ transformation.Apply(context.get(), &transformation_context);
+ }
std::string after_transformation = R"(
OpCapability Shader
@@ -430,6 +486,8 @@
%21 = OpAccessChain %20 %17 %19
OpCopyMemory %12 %21 Aligned 16 Aligned|Volatile 16
OpCopyMemory %133 %12 Volatile Nontemporal|Volatile
+ OpCopyMemory %133 %12 Nontemporal|Volatile
+ OpCopyMemory %133 %12 None Nontemporal|Volatile
%136 = OpAccessChain %135 %17 %30
%138 = OpAccessChain %24 %12 %19
OpCopyMemory %138 %136 None Aligned|Nontemporal 16
