Merge pull request #884 from KhronosGroup/negative-loop-tests

Extend loop tests to cover negative tests.
This commit is contained in:
Hans-Kristian Arntzen 2019-03-06 13:27:00 +01:00 committed by GitHub
commit a206afa785
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 423 additions and 26 deletions

View File

@ -0,0 +1,13 @@
#version 450
void main()
{
int j = 0;
int i = 0;
do
{
j = ((j + i) + 1) * j;
i++;
} while (!(i == 20));
}

View File

@ -0,0 +1,11 @@
#version 450
void main()
{
int _13;
for (int _12 = 0; !(_12 == 16); _12 = _13)
{
_13 = _12 + 1;
}
}

View File

@ -0,0 +1,11 @@
#version 450
void main()
{
int _13;
for (int _12 = 0; _12 != 16; _12 = _13)
{
_13 = _12 + 1;
}
}

View File

@ -0,0 +1,11 @@
#version 450
void main()
{
int _13;
for (int _12 = 0; !(_12 == 16); _12 = _13)
{
_13 = _12 + 1;
}
}

View File

@ -0,0 +1,13 @@
#version 450
void main()
{
int i = 0;
int j = 0;
while (!(i == 20))
{
j = ((j + i) + 1) * j;
i++;
}
}

View File

@ -0,0 +1,51 @@
; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 7
; Bound: 28
; Schema: 0
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
OpName %i "i"
OpName %j "j"
%void = OpTypeVoid
%3 = OpTypeFunction %void
%int = OpTypeInt 32 1
%_ptr_Function_int = OpTypePointer Function %int
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%int_20 = OpConstant %int 20
%bool = OpTypeBool
%main = OpFunction %void None %3
%5 = OpLabel
%i = OpVariable %_ptr_Function_int Function
%j = OpVariable %_ptr_Function_int Function
OpStore %i %int_0
OpStore %j %int_0
OpBranch %11
%11 = OpLabel
OpLoopMerge %13 %14 None
OpBranch %12
%12 = OpLabel
%15 = OpLoad %int %j
%16 = OpLoad %int %i
%17 = OpIAdd %int %15 %16
%19 = OpIAdd %int %17 %int_1
%20 = OpLoad %int %j
%21 = OpIMul %int %19 %20
OpStore %j %21
%22 = OpLoad %int %i
%23 = OpIAdd %int %22 %int_1
OpStore %i %23
OpBranch %14
%14 = OpLabel
%24 = OpLoad %int %i
%27 = OpIEqual %bool %24 %int_20
OpBranchConditional %27 %13 %11
%13 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -0,0 +1,37 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
%void = OpTypeVoid
%3 = OpTypeFunction %void
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_16 = OpConstant %int 16
%bool = OpTypeBool
%int_1 = OpConstant %int 1
%main = OpFunction %void None %3
%5 = OpLabel
OpBranch %8
%8 = OpLabel
%10 = OpPhi %int %12 %7 %int_0 %5
OpLoopMerge %6 %7 None
OpBranch %11
%11 = OpLabel
%16 = OpIEqual %bool %10 %int_16
OpBranchConditional %16 %18 %19
%18 = OpLabel
OpBranch %6
%19 = OpLabel
OpBranch %17
%17 = OpLabel
%21 = OpIAdd %int %10 %int_1
OpBranch %7
%7 = OpLabel
%12 = OpPhi %int %21 %17
OpBranch %8
%6 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -0,0 +1,37 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
%void = OpTypeVoid
%3 = OpTypeFunction %void
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_16 = OpConstant %int 16
%bool = OpTypeBool
%int_1 = OpConstant %int 1
%main = OpFunction %void None %3
%5 = OpLabel
OpBranch %8
%8 = OpLabel
%10 = OpPhi %int %12 %7 %int_0 %5
OpLoopMerge %6 %7 None
OpBranch %11
%11 = OpLabel
%16 = OpINotEqual %bool %10 %int_16
OpBranchConditional %16 %19 %18
%18 = OpLabel
OpBranch %6
%19 = OpLabel
OpBranch %17
%17 = OpLabel
%21 = OpIAdd %int %10 %int_1
OpBranch %7
%7 = OpLabel
%12 = OpPhi %int %21 %17
OpBranch %8
%6 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -0,0 +1,35 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
%void = OpTypeVoid
%3 = OpTypeFunction %void
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_16 = OpConstant %int 16
%bool = OpTypeBool
%int_1 = OpConstant %int 1
%main = OpFunction %void None %3
%5 = OpLabel
OpBranch %8
%8 = OpLabel
%10 = OpPhi %int %12 %7 %int_0 %5
OpLoopMerge %6 %7 None
OpBranch %11
%11 = OpLabel
%16 = OpIEqual %bool %10 %int_16
OpBranchConditional %16 %6 %19
%19 = OpLabel
OpBranch %17
%17 = OpLabel
%21 = OpIAdd %int %10 %int_1
OpBranch %7
%7 = OpLabel
%12 = OpPhi %int %21 %17
OpBranch %8
%6 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -0,0 +1,53 @@
; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 7
; Bound: 29
; Schema: 0
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
OpName %i "i"
OpName %j "j"
%void = OpTypeVoid
%3 = OpTypeFunction %void
%int = OpTypeInt 32 1
%_ptr_Function_int = OpTypePointer Function %int
%int_0 = OpConstant %int 0
%int_20 = OpConstant %int 20
%bool = OpTypeBool
%int_1 = OpConstant %int 1
%main = OpFunction %void None %3
%5 = OpLabel
%i = OpVariable %_ptr_Function_int Function
%j = OpVariable %_ptr_Function_int Function
OpStore %i %int_0
OpStore %j %int_0
OpBranch %11
%11 = OpLabel
OpLoopMerge %13 %14 None
OpBranch %15
%15 = OpLabel
%16 = OpLoad %int %i
%19 = OpIEqual %bool %16 %int_20
OpBranchConditional %19 %13 %12
%12 = OpLabel
%20 = OpLoad %int %j
%21 = OpLoad %int %i
%22 = OpIAdd %int %20 %21
%24 = OpIAdd %int %22 %int_1
%25 = OpLoad %int %j
%26 = OpIMul %int %24 %25
OpStore %j %26
%27 = OpLoad %int %i
%28 = OpIAdd %int %27 %int_1
OpStore %i %28
OpBranch %14
%14 = OpLabel
OpBranch %11
%13 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -1420,12 +1420,29 @@ bool Compiler::block_is_loop_candidate(const SPIRBlock &block, SPIRBlock::Method
// which the code backend can use to create cleaner code.
// for(;;) { if (cond) { some_body; } else { break; } }
// is the pattern we're looking for.
bool ret = block.terminator == SPIRBlock::Select && block.merge == SPIRBlock::MergeLoop &&
block.true_block != block.merge_block && block.true_block != block.self &&
block.false_block == block.merge_block;
const auto *false_block = maybe_get<SPIRBlock>(block.false_block);
const auto *true_block = maybe_get<SPIRBlock>(block.true_block);
const auto *merge_block = maybe_get<SPIRBlock>(block.merge_block);
if (ret && method == SPIRBlock::MergeToSelectContinueForLoop)
bool false_block_is_merge = block.false_block == block.merge_block ||
(false_block && merge_block && execution_is_noop(*false_block, *merge_block));
bool true_block_is_merge = block.true_block == block.merge_block ||
(true_block && merge_block && execution_is_noop(*true_block, *merge_block));
bool positive_candidate =
block.true_block != block.merge_block && block.true_block != block.self && false_block_is_merge;
bool negative_candidate =
block.false_block != block.merge_block && block.false_block != block.self && true_block_is_merge;
bool ret = block.terminator == SPIRBlock::Select && block.merge == SPIRBlock::MergeLoop &&
(positive_candidate || negative_candidate);
if (ret && positive_candidate && method == SPIRBlock::MergeToSelectContinueForLoop)
ret = block.true_block == block.continue_block;
else if (ret && negative_candidate && method == SPIRBlock::MergeToSelectContinueForLoop)
ret = block.false_block == block.continue_block;
// If we have OpPhi which depends on branches which came from our own block,
// we need to flush phi variables in else block instead of a trivial break,
@ -1454,9 +1471,25 @@ bool Compiler::block_is_loop_candidate(const SPIRBlock &block, SPIRBlock::Method
return false;
auto &child = get<SPIRBlock>(block.next_block);
const auto *false_block = maybe_get<SPIRBlock>(child.false_block);
const auto *true_block = maybe_get<SPIRBlock>(child.true_block);
const auto *merge_block = maybe_get<SPIRBlock>(block.merge_block);
bool false_block_is_merge = child.false_block == block.merge_block ||
(false_block && merge_block && execution_is_noop(*false_block, *merge_block));
bool true_block_is_merge = child.true_block == block.merge_block ||
(true_block && merge_block && execution_is_noop(*true_block, *merge_block));
bool positive_candidate =
child.true_block != block.merge_block && child.true_block != block.self && false_block_is_merge;
bool negative_candidate =
child.false_block != block.merge_block && child.false_block != block.self && true_block_is_merge;
ret = child.terminator == SPIRBlock::Select && child.merge == SPIRBlock::MergeNone &&
child.false_block == block.merge_block && child.true_block != block.merge_block &&
child.true_block != block.self;
(positive_candidate || negative_candidate);
// If we have OpPhi which depends on branches which came from our own block,
// we need to flush phi variables in else block instead of a trivial break,
@ -1574,8 +1607,20 @@ SPIRBlock::ContinueBlockType Compiler::continue_block_type(const SPIRBlock &bloc
return SPIRBlock::ForLoop;
else
{
const auto *false_block = maybe_get<SPIRBlock>(block.false_block);
const auto *true_block = maybe_get<SPIRBlock>(block.true_block);
const auto *merge_block = maybe_get<SPIRBlock>(dominator.merge_block);
bool positive_do_while = block.true_block == dominator.self &&
(block.false_block == dominator.merge_block ||
(false_block && merge_block && execution_is_noop(*false_block, *merge_block)));
bool negative_do_while = block.false_block == dominator.self &&
(block.true_block == dominator.merge_block ||
(true_block && merge_block && execution_is_noop(*true_block, *merge_block)));
if (block.merge == SPIRBlock::MergeNone && block.terminator == SPIRBlock::Select &&
block.true_block == dominator.self && block.false_block == dominator.merge_block)
(positive_do_while || negative_do_while))
{
return SPIRBlock::DoWhileLoop;
}

View File

@ -24,6 +24,8 @@
#include <memory>
#include <new>
// clang-format off
#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4996)
@ -1652,4 +1654,4 @@ void spvc_get_version(unsigned *major, unsigned *minor, unsigned *patch)
#ifdef _MSC_VER
#pragma warning(pop)
#endif
#endif

View File

@ -10205,7 +10205,7 @@ void CompilerGLSL::propagate_loop_dominators(const SPIRBlock &block)
// FIXME: This currently cannot handle complex continue blocks
// as in do-while.
// This should be seen as a "trivial" continue block.
string CompilerGLSL::emit_continue_block(uint32_t continue_block)
string CompilerGLSL::emit_continue_block(uint32_t continue_block, bool follow_true_block, bool follow_false_block)
{
auto *block = &get<SPIRBlock>(continue_block);
@ -10233,11 +10233,20 @@ string CompilerGLSL::emit_continue_block(uint32_t continue_block)
block = &get<SPIRBlock>(block->next_block);
}
// For do while blocks. The last block will be a select block.
else if (block->true_block)
else if (block->true_block && follow_true_block)
{
flush_phi(continue_block, block->true_block);
block = &get<SPIRBlock>(block->true_block);
}
else if (block->false_block && follow_false_block)
{
flush_phi(continue_block, block->false_block);
block = &get<SPIRBlock>(block->false_block);
}
else
{
SPIRV_CROSS_THROW("Invalid continue block detected!");
}
}
// Restore old pointer.
@ -10392,10 +10401,15 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method
// emitting the continue block can invalidate the condition expression.
auto initializer = emit_for_loop_initializers(block);
auto condition = to_expression(block.condition);
// Condition might have to be inverted.
if (execution_is_noop(get<SPIRBlock>(block.true_block), get<SPIRBlock>(block.merge_block)))
condition = join("!", enclose_expression(condition));
emit_block_hints(block);
if (method != SPIRBlock::MergeToSelectContinueForLoop)
{
auto continue_block = emit_continue_block(block.continue_block);
auto continue_block = emit_continue_block(block.continue_block, false, false);
statement("for (", initializer, "; ", condition, "; ", continue_block, ")");
}
else
@ -10404,12 +10418,20 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method
}
case SPIRBlock::WhileLoop:
{
// This block may be a dominating block, so make sure we flush undeclared variables before building the while loop header.
flush_undeclared_variables(block);
emit_while_loop_initializers(block);
emit_block_hints(block);
statement("while (", to_expression(block.condition), ")");
auto condition = to_expression(block.condition);
// Condition might have to be inverted.
if (execution_is_noop(get<SPIRBlock>(block.true_block), get<SPIRBlock>(block.merge_block)))
condition = join("!", enclose_expression(condition));
statement("while (", condition, ")");
break;
}
default:
SPIRV_CROSS_THROW("For/while loop detected, but need while/for loop semantics.");
@ -10445,6 +10467,7 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method
if (current_count == statement_count && condition_is_temporary)
{
propagate_loop_dominators(child);
uint32_t target_block = child.true_block;
switch (continue_type)
{
@ -10454,24 +10477,43 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method
// emitting the continue block can invalidate the condition expression.
auto initializer = emit_for_loop_initializers(block);
auto condition = to_expression(child.condition);
auto continue_block = emit_continue_block(block.continue_block);
// Condition might have to be inverted.
if (execution_is_noop(get<SPIRBlock>(child.true_block), get<SPIRBlock>(block.merge_block)))
{
condition = join("!", enclose_expression(condition));
target_block = child.false_block;
}
auto continue_block = emit_continue_block(block.continue_block, false, false);
emit_block_hints(block);
statement("for (", initializer, "; ", condition, "; ", continue_block, ")");
break;
}
case SPIRBlock::WhileLoop:
{
emit_while_loop_initializers(block);
emit_block_hints(block);
statement("while (", to_expression(child.condition), ")");
auto condition = to_expression(child.condition);
// Condition might have to be inverted.
if (execution_is_noop(get<SPIRBlock>(child.true_block), get<SPIRBlock>(block.merge_block)))
{
condition = join("!", enclose_expression(condition));
target_block = child.false_block;
}
statement("while (", condition, ")");
break;
}
default:
SPIRV_CROSS_THROW("For/while loop detected, but need while/for loop semantics.");
}
begin_scope();
branch(child.self, child.true_block);
branch(child.self, target_block);
return true;
}
else
@ -10521,8 +10563,9 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
propagate_loop_dominators(block);
bool select_branch_to_true_block = false;
bool select_branch_to_false_block = false;
bool skip_direct_branch = false;
bool emitted_for_loop_header = false;
bool emitted_loop_header_variables = false;
bool force_complex_continue_block = false;
emit_hoisted_temporaries(block.declare_temporary);
@ -10544,8 +10587,12 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
flush_undeclared_variables(block);
if (attempt_emit_loop_header(block, SPIRBlock::MergeToSelectContinueForLoop))
{
select_branch_to_true_block = true;
emitted_for_loop_header = true;
if (execution_is_noop(get<SPIRBlock>(block.true_block), get<SPIRBlock>(block.merge_block)))
select_branch_to_false_block = true;
else
select_branch_to_true_block = true;
emitted_loop_header_variables = true;
force_complex_continue_block = true;
}
}
@ -10555,9 +10602,13 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
flush_undeclared_variables(block);
if (attempt_emit_loop_header(block, SPIRBlock::MergeToSelectForLoop))
{
// The body of while, is actually just the true block, so always branch there unconditionally.
select_branch_to_true_block = true;
emitted_for_loop_header = true;
// The body of while, is actually just the true (or false) block, so always branch there unconditionally.
if (execution_is_noop(get<SPIRBlock>(block.true_block), get<SPIRBlock>(block.merge_block)))
select_branch_to_false_block = true;
else
select_branch_to_true_block = true;
emitted_loop_header_variables = true;
}
}
// This is the newer loop behavior in glslang which branches from Loop header directly to
@ -10568,13 +10619,14 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
if (attempt_emit_loop_header(block, SPIRBlock::MergeToDirectForLoop))
{
skip_direct_branch = true;
emitted_for_loop_header = true;
emitted_loop_header_variables = true;
}
}
else if (continue_type == SPIRBlock::DoWhileLoop)
{
flush_undeclared_variables(block);
emit_while_loop_initializers(block);
emitted_loop_header_variables = true;
// We have some temporaries where the loop header is the dominator.
// We risk a case where we have code like:
// for (;;) { create-temporary; break; } consume-temporary;
@ -10589,6 +10641,7 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
{
flush_undeclared_variables(block);
emit_while_loop_initializers(block);
emitted_loop_header_variables = true;
// We have a generic loop without any distinguishable pattern like for, while or do while.
get<SPIRBlock>(block.continue_block).complex_continue = true;
@ -10611,7 +10664,7 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
// If we didn't successfully emit a loop header and we had loop variable candidates, we have a problem
// as writes to said loop variables might have been masked out, we need a recompile.
if (!emitted_for_loop_header && !block.loop_variables.empty())
if (!emitted_loop_header_variables && !block.loop_variables.empty())
{
force_recompile = true;
for (auto var : block.loop_variables)
@ -10660,6 +10713,22 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
else
branch(block.self, block.true_block);
}
else if (select_branch_to_false_block)
{
if (force_complex_continue_block)
{
assert(block.false_block == block.continue_block);
// We're going to emit a continue block directly here, so make sure it's marked as complex.
auto &complex_continue = get<SPIRBlock>(block.continue_block).complex_continue;
bool old_complex = complex_continue;
complex_continue = true;
branch(block.self, block.false_block);
complex_continue = old_complex;
}
else
branch(block.self, block.false_block);
}
else
branch(block.self, block.condition, block.true_block, block.false_block);
break;
@ -10850,7 +10919,11 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
// Make sure that we run the continue block to get the expressions set, but this
// should become an empty string.
// We have no fallbacks if we cannot forward everything to temporaries ...
auto statements = emit_continue_block(block.continue_block);
const auto &continue_block = get<SPIRBlock>(block.continue_block);
bool positive_test = execution_is_noop(get<SPIRBlock>(continue_block.true_block),
get<SPIRBlock>(continue_block.loop_dominator));
auto statements = emit_continue_block(block.continue_block, positive_test, !positive_test);
if (!statements.empty())
{
// The DoWhile block has side effects, force ComplexLoop pattern next pass.
@ -10858,7 +10931,12 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
force_recompile = true;
}
end_scope_decl(join("while (", to_expression(get<SPIRBlock>(block.continue_block).condition), ")"));
// Might have to invert the do-while test here.
auto condition = to_expression(continue_block.condition);
if (!positive_test)
condition = join("!", enclose_expression(condition));
end_scope_decl(join("while (", condition, ")"));
}
else
end_scope();

View File

@ -403,7 +403,7 @@ protected:
std::string constant_value_macro_name(uint32_t id);
void emit_constant(const SPIRConstant &constant);
void emit_specialization_constant_op(const SPIRConstantOp &constant);
std::string emit_continue_block(uint32_t continue_block);
std::string emit_continue_block(uint32_t continue_block, bool follow_true_block, bool follow_false_block);
bool attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method method);
void propagate_loop_dominators(const SPIRBlock &block);