spirv-fuzz: Fix to outliner (#3302)

Adds an extra condition on when a region can be outlined to avoid the
case where a region ends with a loop head but such that the loop's
continue target is in the region.  (Outlining such a region would mean
that the loop merge is in the original function and the continue target
in the outlined function.)
This commit is contained in:
Alastair Donaldson 2020-04-15 11:39:33 +01:00 committed by GitHub
parent c018fc6ae6
commit ed96301c6c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 77 additions and 5 deletions

View File

@ -198,10 +198,23 @@ bool TransformationOutlineFunction::IsApplicable(
for (auto& block : *entry_block->GetParent()) {
if (&block == exit_block) {
// It is OK (and typically expected) for the exit block of the region to
// have successors outside the region. It is also OK for the exit block
// to head a structured control flow construct - the block containing the
// call to the outlined function will end up heading this construct if
// outlining takes place.
// have successors outside the region.
//
// It is also OK for the exit block to head a structured control flow
// construct - the block containing the call to the outlined function will
// end up heading this construct if outlining takes place. However, we
// must ensure that if the exit block heads a loop, the continue target
// for this loop is outside the region.
if (auto loop_merge = block.GetLoopMergeInst()) {
// The exit block heads a loop
auto continue_target =
ir_context->cfg()->block(loop_merge->GetSingleWordOperand(1));
if (region_set.count(continue_target)) {
// The continue target for the loop is in the region.
return false;
}
}
continue;
}

View File

@ -2465,7 +2465,6 @@ TEST(TransformationOutlineFunctionTest,
// This checks that we cannot outline a region of code if it produces a
// pointer result id that gets used outside the region. This avoids creating
// a struct with a pointer member.
std::string shader = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
@ -2520,6 +2519,66 @@ TEST(TransformationOutlineFunctionTest,
transformation.IsApplicable(context.get(), transformation_context));
}
TEST(TransformationOutlineFunctionTest,
ExitBlockHeadsLoopButContinueConstructIsInRegion) {
// This checks that it is not possible outline a region that ends in a loop
// head if the continue target for the loop is inside the region.
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
%15 = OpTypeInt 32 1
%35 = OpTypeBool
%39 = OpConstant %15 1
%40 = OpConstantTrue %35
%4 = OpFunction %2 None %3
%5 = OpLabel
OpBranch %22
%22 = OpLabel
OpBranch %23
%23 = OpLabel
%24 = OpPhi %15 %39 %22 %39 %25
OpLoopMerge %26 %25 None
OpBranchConditional %40 %25 %26
%25 = OpLabel
OpBranch %23
%26 = OpLabel
OpReturn
OpFunctionEnd
)";
const auto env = SPV_ENV_UNIVERSAL_1_5;
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);
TransformationOutlineFunction transformation(
/*entry_block*/ 22,
/*exit_block*/ 23,
/*new_function_struct_return_type_id*/ 200,
/*new_function_type_id*/ 201,
/*new_function_id*/ 202,
/*new_function_region_entry_block*/ 203,
/*new_caller_result_id*/ 204,
/*new_callee_result_id*/ 205,
/*input_id_to_fresh_id*/ {},
/*output_id_to_fresh_id*/ {});
ASSERT_FALSE(
transformation.IsApplicable(context.get(), transformation_context));
transformation.Apply(context.get(), &transformation_context);
}
TEST(TransformationOutlineFunctionTest, Miscellaneous1) {
// This tests outlining of some non-trivial code.