From 72ea7bec4ab394c40db94f8c6d3b5920f1aca094 Mon Sep 17 00:00:00 2001 From: Vasyl Teliman Date: Fri, 14 Aug 2020 14:29:36 +0300 Subject: [PATCH] spirv-fuzz: Support identical predecessors in TransformationPropagateInstructionUp (#3689) Support identical predecessors in TransformationPropagateInstructionUp. A basic block may have multiple identical predecessors as follows: %1 = OpLabel OpSelectionMerge %2 None OpBranchConditional %true %2 %2 %2 = OpLabel ... This case wasn't supported before. --- .../fuzzer_pass_propagate_instructions_up.cpp | 8 +- ...ransformation_propagate_instruction_up.cpp | 15 +++- ...ormation_propagate_instruction_up_test.cpp | 75 +++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/source/fuzz/fuzzer_pass_propagate_instructions_up.cpp b/source/fuzz/fuzzer_pass_propagate_instructions_up.cpp index 52af774d2..2042d7c2c 100644 --- a/source/fuzz/fuzzer_pass_propagate_instructions_up.cpp +++ b/source/fuzz/fuzzer_pass_propagate_instructions_up.cpp @@ -44,7 +44,13 @@ void FuzzerPassPropagateInstructionsUp::Apply() { GetIRContext(), block.id())) { std::map fresh_ids; for (auto id : GetIRContext()->cfg()->preds(block.id())) { - fresh_ids[id] = GetFuzzerContext()->GetFreshId(); + auto& fresh_id = fresh_ids[id]; + + if (!fresh_id) { + // Create a fresh id if it hasn't been created yet. |fresh_id| will + // be default-initialized to 0 in this case. + fresh_id = GetFuzzerContext()->GetFreshId(); + } } ApplyTransformation( diff --git a/source/fuzz/transformation_propagate_instruction_up.cpp b/source/fuzz/transformation_propagate_instruction_up.cpp index c41ee5b21..adf3a5169 100644 --- a/source/fuzz/transformation_propagate_instruction_up.cpp +++ b/source/fuzz/transformation_propagate_instruction_up.cpp @@ -99,15 +99,18 @@ bool TransformationPropagateInstructionUp::IsApplicable( const auto predecessor_id_to_fresh_id = fuzzerutil::RepeatedUInt32PairToMap( message_.predecessor_id_to_fresh_id()); - std::vector maybe_fresh_ids; for (auto id : ir_context->cfg()->preds(message_.block_id())) { // Each predecessor must have a fresh id in the |predecessor_id_to_fresh_id| // map. if (!predecessor_id_to_fresh_id.count(id)) { return false; } + } - maybe_fresh_ids.push_back(predecessor_id_to_fresh_id.at(id)); + std::vector maybe_fresh_ids; + maybe_fresh_ids.reserve(predecessor_id_to_fresh_id.size()); + for (const auto& entry : predecessor_id_to_fresh_id) { + maybe_fresh_ids.push_back(entry.second); } // All ids must be unique and fresh. @@ -129,7 +132,15 @@ void TransformationPropagateInstructionUp::Apply( opt::Instruction::OperandList op_phi_operands; const auto predecessor_id_to_fresh_id = fuzzerutil::RepeatedUInt32PairToMap( message_.predecessor_id_to_fresh_id()); + std::unordered_set visited_predecessors; for (auto predecessor_id : ir_context->cfg()->preds(message_.block_id())) { + // A block can have multiple identical predecessors. + if (visited_predecessors.count(predecessor_id)) { + continue; + } + + visited_predecessors.insert(predecessor_id); + auto new_result_id = predecessor_id_to_fresh_id.at(predecessor_id); // Compute InOperands for the OpPhi instruction to be inserted later. diff --git a/test/fuzz/transformation_propagate_instruction_up_test.cpp b/test/fuzz/transformation_propagate_instruction_up_test.cpp index 9837a013e..de20e1510 100644 --- a/test/fuzz/transformation_propagate_instruction_up_test.cpp +++ b/test/fuzz/transformation_propagate_instruction_up_test.cpp @@ -815,6 +815,81 @@ TEST(TransformationPropagateInstructionUpTest, ASSERT_TRUE(IsEqual(env, after_transformation, context.get())); } +TEST(TransformationPropagateInstructionUpTest, MultipleIdenticalPredecessors) { + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeFloat 32 + %11 = OpConstant %6 23 + %12 = OpTypeBool + %13 = OpConstantTrue %12 + %4 = OpFunction %2 None %3 + + %5 = OpLabel + OpSelectionMerge %9 None + OpBranchConditional %13 %9 %9 + + %9 = OpLabel + %14 = OpPhi %6 %11 %5 + %10 = OpCopyObject %6 %14 + OpReturn + + OpFunctionEnd + )"; + + const auto env = SPV_ENV_UNIVERSAL_1_3; + const auto consumer = nullptr; + const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption); + ASSERT_TRUE(IsValid(env, context.get())); + + FactManager fact_manager; + spvtools::ValidatorOptions validator_options; + TransformationContext transformation_context(&fact_manager, + validator_options); + + TransformationPropagateInstructionUp transformation(9, {{{5, 40}}}); + ASSERT_TRUE( + transformation.IsApplicable(context.get(), transformation_context)); + transformation.Apply(context.get(), &transformation_context); + ASSERT_TRUE(IsValid(env, context.get())); + + std::string after_transformation = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeFloat 32 + %11 = OpConstant %6 23 + %12 = OpTypeBool + %13 = OpConstantTrue %12 + %4 = OpFunction %2 None %3 + + %5 = OpLabel + %40 = OpCopyObject %6 %11 + OpSelectionMerge %9 None + OpBranchConditional %13 %9 %9 + + %9 = OpLabel + %10 = OpPhi %6 %40 %5 + %14 = OpPhi %6 %11 %5 + OpReturn + + OpFunctionEnd + )"; + + ASSERT_TRUE(IsEqual(env, after_transformation, context.get())); +} + } // namespace } // namespace fuzz } // namespace spvtools