Fix operand access (#3427)

Fixes #3426.
This commit is contained in:
Vasyl Teliman 2020-06-13 02:03:25 +03:00 committed by GitHub
parent 5543d5faa9
commit 30bf46dbe0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 169 additions and 88 deletions

View File

@ -101,6 +101,14 @@ void TransformationSetMemoryOperandsMask::Apply(
// 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 @@ uint32_t TransformationSetMemoryOperandsMask::GetInOperandIndexForMask(
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.

View File

@ -75,6 +75,7 @@ TEST(TransformationSetMemoryOperandsMaskTest, PreSpirv14) {
%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 @@ TEST(TransformationSetMemoryOperandsMaskTest, PreSpirv14) {
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 @@ TEST(TransformationSetMemoryOperandsMaskTest, PreSpirv14) {
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 @@ TEST(TransformationSetMemoryOperandsMaskTest, PreSpirv14) {
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 @@ TEST(TransformationSetMemoryOperandsMaskTest, PreSpirv14) {
%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 @@ TEST(TransformationSetMemoryOperandsMaskTest, Spirv14) {
%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 @@ TEST(TransformationSetMemoryOperandsMaskTest, Spirv14) {
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 @@ TEST(TransformationSetMemoryOperandsMaskTest, Spirv14) {
%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