AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-12-24 00:40:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Validate nested constructs (#3068)

Browse Source 
* Validate that if a construct contains a header and it's merge is
reachable, the construct also contains the merge
* updated block merging to not merge into the continue
* update inlining to mark the original block of a single block loop as
the continue
* updated some tests
* remove dead code
* rename kBlockTypeHeader to kBlockTypeSelection for clarity
This commit is contained in:
alan-baker 2019-11-27 16:45:57 -05:00 committed by David Neto
parent 52e9cc9301
commit b334829a91
18 changed files with 291 additions and 256 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
source/opt/block_merge_util.cpp
View File

@ -49,6 +49,18 @@ bool IsMerge(IRContext* context, BasicBlock* block) {
return IsMerge(context, block->id());
}
// Returns true if |id| is the continue target of a merge instruction.
bool IsContinue(IRContext* context, uint32_t id) {
return !context->get_def_use_mgr()->WhileEachUse(
id, [](Instruction* user, uint32_t index) {
SpvOp op = user->opcode();
if (op == SpvOpLoopMerge && index == 1u) {
return false;
}
return true;
});
}
// Removes any OpPhi instructions in |block|, which should have exactly one
// predecessor, replacing uses of OpPhi ids with the ids associated with the
// predecessor.
@ -86,6 +98,11 @@ bool CanMergeWithSuccessor(IRContext* context, BasicBlock* block) {
return false;
}
if (pred_is_merge && IsContinue(context, lab_id)) {
// Cannot merge a continue target with a merge block.
return false;
}
// Don't bother trying to merge unreachable blocks.
if (auto dominators = context->GetDominatorAnalysis(block->GetParent())) {
if (!dominators->IsReachable(block)) return false;

22
source/opt/inline_pass.cpp
View File

@ -27,7 +27,6 @@
static const int kSpvFunctionCallFunctionId = 2;
static const int kSpvFunctionCallArgumentId = 3;
static const int kSpvReturnValueId = 0;
static const int kSpvLoopMergeContinueTargetIdInIdx = 1;
namespace spvtools {
namespace opt {
@ -285,19 +284,14 @@ bool InlinePass::GenInlineCode(
if (rid != 0) callee_result_ids.insert(rid);
});
// If the caller is in a single-block loop, and the callee has multiple
// blocks, then the normal inlining logic will place the OpLoopMerge in
// the last of several blocks in the loop. Instead, it should be placed
// at the end of the first block. First determine if the caller is in a
// single block loop. We'll wait to move the OpLoopMerge until the end
// of the regular inlining logic, and only if necessary.
bool caller_is_single_block_loop = false;
// If the caller is a loop header and the callee has multiple blocks, then the
// normal inlining logic will place the OpLoopMerge in the last of several
// blocks in the loop. Instead, it should be placed at the end of the first
// block. We'll wait to move the OpLoopMerge until the end of the regular
// inlining logic, and only if necessary.
bool caller_is_loop_header = false;
if (auto* loop_merge = call_block_itr->GetLoopMergeInst()) {
if (call_block_itr->GetLoopMergeInst()) {
caller_is_loop_header = true;
caller_is_single_block_loop =
call_block_itr->id() ==
loop_merge->GetSingleWordInOperand(kSpvLoopMergeContinueTargetIdInIdx);
}
bool callee_begins_with_structured_header =
@ -611,10 +605,6 @@ bool InlinePass::GenInlineCode(
--loop_merge_itr;
assert(loop_merge_itr->opcode() == SpvOpLoopMerge);
std::unique_ptr<Instruction> cp_inst(loop_merge_itr->Clone(context()));
if (caller_is_single_block_loop) {
// Also, update its continue target to point to the last block.
cp_inst->SetInOperand(kSpvLoopMergeContinueTargetIdInIdx, {last->id()});
}
first->tail().InsertBefore(std::move(cp_inst));
// Remove the loop merge from the last block.

4
source/val/basic_block.h
View File

@ -15,8 +15,8 @@
#ifndef SOURCE_VAL_BASIC_BLOCK_H_
#define SOURCE_VAL_BASIC_BLOCK_H_
#include <cstdint>
#include <bitset>
#include <cstdint>
#include <functional>
#include <memory>
#include <vector>
@ -28,7 +28,7 @@ namespace val {
enum BlockType : uint32_t {
kBlockTypeUndefined,
kBlockTypeHeader,
kBlockTypeSelection,
kBlockTypeLoop,
kBlockTypeMerge,
kBlockTypeBreak,

7
source/val/function.cpp
View File

@ -14,9 +14,8 @@
#include "source/val/function.h"
#include <cassert>
#include <algorithm>
#include <cassert>
#include <sstream>
#include <unordered_map>
#include <unordered_set>
@ -99,7 +98,7 @@ spv_result_t Function::RegisterLoopMerge(uint32_t merge_id,
spv_result_t Function::RegisterSelectionMerge(uint32_t merge_id) {
RegisterBlock(merge_id, false);
BasicBlock& merge_block = blocks_.at(merge_id);
current_block_->set_type(kBlockTypeHeader);
current_block_->set_type(kBlockTypeSelection);
merge_block.set_type(kBlockTypeMerge);
merge_block_header_[&merge_block] = current_block_;
@ -344,7 +343,7 @@ int Function::GetBlockDepth(BasicBlock* bb) {
BasicBlock* header = merge_block_header_[bb];
assert(header);
block_depth_[bb] = GetBlockDepth(header);
} else if (bb_dom->is_type(kBlockTypeHeader) ||
} else if (bb_dom->is_type(kBlockTypeSelection) ||
bb_dom->is_type(kBlockTypeLoop)) {
// The dominator of the given block is a header block. So, the nesting
// depth of this block is: 1 + nesting depth of the header.

23
source/val/validate_cfg.cpp
View File

@ -12,8 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#include "source/val/validate.h"
#include <algorithm>
#include <cassert>
#include <functional>
@ -34,6 +32,7 @@
#include "source/val/basic_block.h"
#include "source/val/construct.h"
#include "source/val/function.h"
#include "source/val/validate.h"
#include "source/val/validation_state.h"
namespace spvtools {
@ -755,6 +754,26 @@ spv_result_t StructuredControlFlowChecks(
<< header_name << " <ID> " << header->id();
}
}
if (block->is_type(BlockType::kBlockTypeSelection) ||
block->is_type(BlockType::kBlockTypeLoop)) {
size_t index = (block->terminator() - &_.ordered_instructions()[0]) - 1;
const auto& merge_inst = _.ordered_instructions()[index];
if (merge_inst.opcode() == SpvOpSelectionMerge ||
merge_inst.opcode() == SpvOpLoopMerge) {
uint32_t merge_id = merge_inst.GetOperandAs<uint32_t>(0);
auto merge_block = function->GetBlock(merge_id).first;
if (merge_block->reachable() &&
!construct_blocks.count(merge_block)) {
return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(block->id()))
<< "Header block " << _.getIdName(block->id())
<< " is contained in the " << construct_name
<< " construct headed by " << _.getIdName(header->id())
<< ", but it's merge block " << _.getIdName(merge_id)
<< " is not";
}
}
}
}
// Checks rules for case constructs.

112
test/fuzz/transformation_add_dead_continue_test.cpp
View File

@ -209,117 +209,6 @@ TEST(TransformationAddDeadContinueTest, SimpleExample) {
ASSERT_TRUE(IsEqual(env, after_transformation, context.get()));
}
TEST(TransformationAddDeadContinueTest,
DoNotAllowContinueToMergeBlockOfAnotherLoop) {
// A loop header must dominate its merge block if that merge block is
// reachable. We are thus not allowed to add a dead continue that would result
// in violation of this property. This test checks for such a scenario.
std::string shader = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %16 %139
OpExecutionMode %4 OriginUpperLeft
OpSource ESSL 310
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeFloat 32
%7 = OpTypePointer Function %6
%8 = OpTypeBool
%14 = OpTypeVector %6 4
%15 = OpTypePointer Input %14
%16 = OpVariable %15 Input
%138 = OpTypePointer Output %14
%139 = OpVariable %138 Output
%400 = OpConstantTrue %8
%4 = OpFunction %2 None %3
%5 = OpLabel
OpBranch %500
%500 = OpLabel
OpLoopMerge %501 %502 None
OpBranch %503 ; We are not allowed to change this to OpBranchConditional %400 %503 %502
%503 = OpLabel
OpLoopMerge %502 %504 None
OpBranchConditional %400 %505 %504
%505 = OpLabel
OpBranch %502
%504 = OpLabel
OpBranch %503
%502 = OpLabel
OpBranchConditional %400 %501 %500
%501 = OpLabel
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;
ASSERT_FALSE(TransformationAddDeadContinue(500, true, {})
.IsApplicable(context.get(), fact_manager));
ASSERT_FALSE(TransformationAddDeadContinue(500, false, {})
.IsApplicable(context.get(), fact_manager));
}
TEST(TransformationAddDeadContinueTest, DoNotAllowContinueToSelectionMerge) {
// A selection header must dominate its merge block if that merge block is
// reachable. We are thus not allowed to add a dead continue that would result
// in violation of this property. This test checks for such a scenario.
std::string shader = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %16 %139
OpExecutionMode %4 OriginUpperLeft
OpSource ESSL 310
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeFloat 32
%7 = OpTypePointer Function %6
%8 = OpTypeBool
%14 = OpTypeVector %6 4
%15 = OpTypePointer Input %14
%16 = OpVariable %15 Input
%138 = OpTypePointer Output %14
%139 = OpVariable %138 Output
%400 = OpConstantTrue %8
%4 = OpFunction %2 None %3
%5 = OpLabel
OpBranch %500
%500 = OpLabel
OpLoopMerge %501 %502 None
OpBranch %503 ; We are not allowed to change this to OpBranchConditional %400 %503 %502
%503 = OpLabel
OpSelectionMerge %502 None
OpBranchConditional %400 %505 %504
%505 = OpLabel
OpBranch %502
%504 = OpLabel
OpBranch %502
%502 = OpLabel
OpBranchConditional %400 %501 %500
%501 = OpLabel
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;
ASSERT_FALSE(TransformationAddDeadContinue(500, true, {})
.IsApplicable(context.get(), fact_manager));
ASSERT_FALSE(TransformationAddDeadContinue(500, false, {})
.IsApplicable(context.get(), fact_manager));
}
TEST(TransformationAddDeadContinueTest, LoopNest) {
// Checks some allowed and disallowed scenarios for a nest of loops, including
// continuing a loop from an if or switch.
@ -1420,7 +1309,6 @@ TEST(TransformationAddDeadContinueTest, Miscellaneous2) {
OpLoopMerge %1557 %1570 None
OpBranchConditional %395 %1562 %1557
%1562 = OpLabel
OpSelectionMerge %1570 None
OpBranchConditional %395 %1571 %1570
%1571 = OpLabel
OpBranch %1557

2
test/fuzz/transformation_copy_object_test.cpp
View File

@ -291,7 +291,7 @@ TEST(TransformationCopyObjectTest, CheckIllegalCases) {
%31 = OpLabel
%42 = OpAccessChain %36 %18 %41
%43 = OpLoad %11 %42
OpSelectionMerge %47 None
OpSelectionMerge %45 None
OpSwitch %43 %46 0 %44 1 %45
%46 = OpLabel
%69 = OpIAdd %11 %96 %27

20
test/fuzz/transformation_replace_id_with_synonym_test.cpp
View File

@ -159,18 +159,20 @@ const std::string kComplexShader = R"(
%65 = OpAccessChain %13 %11 %64
%66 = OpLoad %6 %65
%67 = OpSGreaterThan %29 %84 %66
OpSelectionMerge %69 None
OpSelectionMerge %1000 None
OpBranchConditional %67 %68 %72
%68 = OpLabel
%71 = OpIAdd %6 %84 %26
OpBranch %69
OpBranch %1000
%72 = OpLabel
%74 = OpIAdd %6 %84 %64
%205 = OpCopyObject %6 %74
OpBranch %69
%69 = OpLabel
OpBranch %1000
%1000 = OpLabel
%86 = OpPhi %6 %71 %68 %74 %72
%301 = OpPhi %6 %71 %68 %15 %72
OpBranch %69
%69 = OpLabel
OpBranch %20
%22 = OpLabel
%75 = OpAccessChain %46 %42 %50
@ -421,18 +423,20 @@ TEST(TransformationReplaceIdWithSynonymTest, LegalTransformations) {
%65 = OpAccessChain %13 %11 %64
%66 = OpLoad %6 %65
%67 = OpSGreaterThan %29 %84 %66
OpSelectionMerge %69 None
OpSelectionMerge %1000 None
OpBranchConditional %67 %68 %72
%68 = OpLabel
%71 = OpIAdd %6 %84 %26
OpBranch %69
OpBranch %1000
%72 = OpLabel
%74 = OpIAdd %6 %84 %64
%205 = OpCopyObject %6 %74
OpBranch %69
%69 = OpLabel
OpBranch %1000
%1000 = OpLabel
%86 = OpPhi %6 %71 %68 %205 %72
%301 = OpPhi %6 %71 %68 %15 %72
OpBranch %69
%69 = OpLabel
OpBranch %20
%22 = OpLabel
%75 = OpAccessChain %46 %42 %50

1
test/opt/aggressive_dead_code_elim_test.cpp
View File

@ -5932,7 +5932,6 @@ OpBranch %42
%42 = OpLabel
%43 = OpLoad %int %i
%44 = OpSLessThan %bool %43 %int_1
OpSelectionMerge %45 None
OpBranchConditional %44 %46 %40
%46 = OpLabel
%47 = OpLoad %int %i

47
test/opt/block_merge_test.cpp
View File

@ -447,10 +447,53 @@ OpBranch %header
OpLoopMerge %merge %continue None
OpBranch %inner_header
%inner_header = OpLabel
OpSelectionMerge %continue None
OpBranchConditional %true %then %continue
OpSelectionMerge %if_merge None
OpBranchConditional %true %then %if_merge
%then = OpLabel
OpBranch %continue
%if_merge = OpLabel
OpBranch %continue
%continue = OpLabel
OpBranchConditional %false %merge %header
%merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<BlockMergePass>(text, true);
}
TEST_F(BlockMergeTest, CannotMergeContinue) {
const std::string text = R"(
; CHECK: OpBranch [[loop_header:%\w+]]
; CHECK: [[loop_header]] = OpLabel
; CHECK-NEXT: OpLoopMerge {{%\w+}} [[continue:%\w+]]
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[if_header:%\w+]]
; CHECK: [[if_header]] = OpLabel
; CHECK-NEXT: OpSelectionMerge
; CHECK: [[continue]] = OpLabel
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
%void = OpTypeVoid
%bool = OpTypeBool
%true = OpConstantTrue %bool
%false = OpConstantFalse %bool
%functy = OpTypeFunction %void
%func = OpFunction %void None %functy
%entry = OpLabel
OpBranch %header
%header = OpLabel
OpLoopMerge %merge %continue None
OpBranchConditional %true %inner_header %merge
%inner_header = OpLabel
OpSelectionMerge %if_merge None
OpBranchConditional %true %then %if_merge
%then = OpLabel
OpBranch %continue
%if_merge = OpLabel
OpBranch %continue
%continue = OpLabel
OpBranchConditional %false %merge %header
%merge = OpLabel

8
test/opt/dead_branch_elim_test.cpp
View File

@ -2798,7 +2798,9 @@ OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
// The selection merge in the loop naming the continue target as merge is
// invalid, but handled by this pass so validation is disabled.
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, false);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithNestedLoop) {
@ -2942,7 +2944,9 @@ OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(body, true);
// The selection merge in the loop naming the continue target as merge is
// invalid, but handled by this pass so validation is disabled.
SinglePassRunAndMatch<DeadBranchElimPass>(body, false);
}
TEST_F(DeadBranchElimTest, UnreachableMergeAndContinueSameBlock) {

6
test/opt/inline_test.cpp
View File

@ -1819,7 +1819,7 @@ OpFunctionEnd
%9 = OpLabel
OpBranch %10
%10 = OpLabel
OpLoopMerge %12 %13 None
OpLoopMerge %12 %10 None
OpBranch %13
%13 = OpLabel
OpBranchConditional %true %10 %12
@ -1980,7 +1980,7 @@ OpFunctionEnd
OpBranch %13
%13 = OpLabel
%14 = OpCopyObject %bool %false
OpLoopMerge %16 %19 None
OpLoopMerge %16 %13 None
OpBranch %17
%17 = OpLabel
%18 = OpCopyObject %bool %true
@ -2145,7 +2145,7 @@ OpBranch %19
%19 = OpLabel
%20 = OpCopyObject %int %int_2
%25 = OpCopyObject %int %int_0
OpLoopMerge %23 %26 None
OpLoopMerge %23 %19 None
OpBranch %26
%27 = OpLabel
%28 = OpCopyObject %int %int_1

70
test/opt/local_ssa_elim_test.cpp
View File

@ -193,7 +193,24 @@ OpDecorate %fo Location 0
)";
const std::string before =
R"(%main = OpFunction %void None %9
R"(
; CHECK: = OpFunction
; CHECK-NEXT: [[entry:%\w+]] = OpLabel
; CHECK: [[outer_header:%\w+]] = OpLabel
; CHECK-NEXT: [[outer_f:%\w+]] = OpPhi %float %float_0 [[entry]] [[inner_f:%\w+]] [[outer_be:%\w+]]
; CHECK-NEXT: [[i:%\w+]] = OpPhi %int %int_0 [[entry]] [[i_next:%\w+]] [[outer_be]]
; CHECK-NEXT: OpSLessThan {{%\w+}} [[i]]
; CHECK: [[inner_pre_header:%\w+]] = OpLabel
; CHECK: [[inner_header:%\w+]] = OpLabel
; CHECK-NEXT: [[inner_f]] = OpPhi %float [[outer_f]] [[inner_pre_header]] [[f_next:%\w+]] [[inner_be:%\w+]]
; CHECK-NEXT: [[j:%\w+]] = OpPhi %int %int_0 [[inner_pre_header]] [[j_next:%\w+]] [[inner_be]]
; CHECK: [[inner_be]] = OpLabel
; CHECK: [[f_next]] = OpFAdd %float [[inner_f]]
; CHECK: [[j_next]] = OpIAdd %int [[j]] %int_1
; CHECK: [[outer_be]] = OpLabel
; CHECK: [[i_next]] = OpIAdd
; CHECK: OpStore %fo [[outer_f]]
%main = OpFunction %void None %9
%24 = OpLabel
%f = OpVariable %_ptr_Function_float Function
%i = OpVariable %_ptr_Function_int Function
@ -212,8 +229,8 @@ OpBranch %31
%31 = OpLabel
%32 = OpLoad %int %j
%33 = OpSLessThan %bool %32 %int_4
OpLoopMerge %29 %34 None
OpBranchConditional %33 %34 %29
OpLoopMerge %50 %34 None
OpBranchConditional %33 %34 %50
%34 = OpLabel
%35 = OpLoad %float %f
%36 = OpLoad %int %i
@ -226,6 +243,8 @@ OpStore %f %40
%42 = OpIAdd %int %41 %int_1
OpStore %j %42
OpBranch %31
%50 = OpLabel
OpBranch %29
%29 = OpLabel
%43 = OpLoad %int %i
%44 = OpIAdd %int %43 %int_1
@ -238,50 +257,7 @@ OpReturn
OpFunctionEnd
)";
const std::string after =
R"(%main = OpFunction %void None %9
%24 = OpLabel
%f = OpVariable %_ptr_Function_float Function
%i = OpVariable %_ptr_Function_int Function
%j = OpVariable %_ptr_Function_int Function
OpStore %f %float_0
OpStore %i %int_0
OpBranch %25
%25 = OpLabel
%47 = OpPhi %float %float_0 %24 %50 %29
%46 = OpPhi %int %int_0 %24 %44 %29
%27 = OpSLessThan %bool %46 %int_4
OpLoopMerge %28 %29 None
OpBranchConditional %27 %30 %28
%30 = OpLabel
OpStore %j %int_0
OpBranch %31
%31 = OpLabel
%50 = OpPhi %float %47 %30 %40 %34
%48 = OpPhi %int %int_0 %30 %42 %34
%33 = OpSLessThan %bool %48 %int_4
OpLoopMerge %29 %34 None
OpBranchConditional %33 %34 %29
%34 = OpLabel
%38 = OpAccessChain %_ptr_Input_float %BC %46 %48
%39 = OpLoad %float %38
%40 = OpFAdd %float %50 %39
OpStore %f %40
%42 = OpIAdd %int %48 %int_1
OpStore %j %42
OpBranch %31
%29 = OpLabel
%44 = OpIAdd %int %46 %int_1
OpStore %i %44
OpBranch %25
%28 = OpLabel
OpStore %fo %47
OpReturn
OpFunctionEnd
)";
SinglePassRunAndCheck<SSARewritePass>(predefs + before, predefs + after, true,
true);
SinglePassRunAndMatch<SSARewritePass>(predefs + before, true);
}
TEST_F(LocalSSAElimTest, ForLoopWithContinue) {

6
test/opt/loop_optimizations/fusion_legal.cpp
View File

@ -3177,7 +3177,7 @@ TEST_F(FusionLegalTest, OuterloopWithBreakContinueInInner) {
%21 = OpLabel
%29 = OpSMod %6 %96 %28
%30 = OpIEqual %17 %29 %9
OpSelectionMerge %23 None
OpSelectionMerge %sel_merge None
OpBranchConditional %30 %31 %48
%31 = OpLabel
%44 = OpAccessChain %7 %41 %91 %96
@ -3187,8 +3187,10 @@ TEST_F(FusionLegalTest, OuterloopWithBreakContinueInInner) {
OpStore %47 %46
OpBranch %32
%48 = OpLabel
OpBranch %23
OpBranch %sel_merge
%32 = OpLabel
OpBranch %sel_merge
%sel_merge = OpLabel
OpBranch %23
%23 = OpLabel
%52 = OpIAdd %6 %96 %51

9
test/opt/pass_merge_return_test.cpp
View File

@ -1390,7 +1390,6 @@ TEST_F(MergeReturnPassTest, SerialLoopsUpdateBlockMapping) {
OpLoopMerge %36 %40 None
OpBranch %35
%35 = OpLabel
OpSelectionMerge %40 None
OpBranchConditional %77 %39 %40
%39 = OpLabel
OpReturnValue %16
@ -1402,7 +1401,6 @@ TEST_F(MergeReturnPassTest, SerialLoopsUpdateBlockMapping) {
OpLoopMerge %45 %49 None
OpBranch %44
%44 = OpLabel
OpSelectionMerge %49 None
OpBranchConditional %77 %48 %49
%48 = OpLabel
OpReturnValue %16
@ -1415,7 +1413,6 @@ TEST_F(MergeReturnPassTest, SerialLoopsUpdateBlockMapping) {
OpLoopMerge %64 %68 None
OpBranch %63
%63 = OpLabel
OpSelectionMerge %68 None
OpBranchConditional %77 %67 %68
%67 = OpLabel
OpReturnValue %16
@ -1813,12 +1810,14 @@ TEST_F(MergeReturnPassTest, PhiInSecondMerge) {
OpLoopMerge %11 %12 None
OpBranch %13
%13 = OpLabel
OpLoopMerge %12 %14 None
OpBranchConditional %8 %15 %12
OpLoopMerge %18 %14 None
OpBranchConditional %8 %15 %18
%15 = OpLabel
OpReturn
%14 = OpLabel
OpBranch %13
%18 = OpLabel
OpBranch %12
%12 = OpLabel
%16 = OpUndef %float
OpBranchConditional %8 %10 %11

17
test/reduce/reducer_test.cpp
View File

@ -125,19 +125,21 @@ TEST(ReducerTest, ExprToConstantAndRemoveUnreferenced) {
%29 = OpAccessChain %28 %27 %9
%30 = OpLoad %24 %29
%32 = OpFOrdGreaterThan %22 %30 %31
OpSelectionMerge %34 None
OpSelectionMerge %90 None
OpBranchConditional %32 %33 %46
%33 = OpLabel
%40 = OpFAdd %24 %71 %30
%45 = OpISub %6 %73 %21
OpBranch %34
OpBranch %90
%46 = OpLabel
%50 = OpFMul %24 %71 %30
%54 = OpSDiv %6 %73 %21
OpBranch %34
%34 = OpLabel
OpBranch %90
%90 = OpLabel
%77 = OpPhi %6 %45 %33 %54 %46
%76 = OpPhi %24 %40 %33 %50 %46
OpBranch %34
%34 = OpLabel
%57 = OpIAdd %6 %70 %56
OpBranch %10
%12 = OpLabel
@ -193,11 +195,13 @@ TEST(ReducerTest, ExprToConstantAndRemoveUnreferenced) {
OpLoopMerge %12 %34 None
OpBranchConditional %100 %11 %12
%11 = OpLabel
OpSelectionMerge %34 None
OpSelectionMerge %90 None
OpBranchConditional %100 %33 %46
%33 = OpLabel
OpBranch %34
OpBranch %90
%46 = OpLabel
OpBranch %90
%90 = OpLabel
OpBranch %34
%34 = OpLabel
OpBranch %10
@ -345,7 +349,6 @@ const std::string kShaderWithLoopsDivAndMul = R"(
OpLoopMerge %33 %38 None
OpBranch %32
%32 = OpLabel
OpSelectionMerge %38 None
OpBranchConditional %30 %37 %38
%37 = OpLabel
OpSelectionMerge %42 None

33
test/reduce/validation_during_reduction_test.cpp
View File

@ -13,7 +13,6 @@
// limitations under the License.
#include "source/reduce/reducer.h"
#include "source/reduce/reduction_opportunity.h"
#include "source/reduce/remove_instruction_reduction_opportunity.h"
#include "test/reduce/reduce_test_util.h"
@ -23,8 +22,8 @@ namespace reduce {
namespace {
using opt::Function;
using opt::IRContext;
using opt::Instruction;
using opt::IRContext;
// A dumb reduction opportunity finder that finds opportunities to remove global
// values regardless of whether they are referenced. This is very likely to make
@ -181,19 +180,21 @@ TEST(ValidationDuringReductionTest, CheckInvalidPassMakesNoProgress) {
%29 = OpAccessChain %28 %27 %9
%30 = OpLoad %24 %29
%32 = OpFOrdGreaterThan %22 %30 %31
OpSelectionMerge %34 None
OpSelectionMerge %90 None
OpBranchConditional %32 %33 %46
%33 = OpLabel
%40 = OpFAdd %24 %71 %30
%45 = OpISub %6 %73 %21
OpBranch %34
OpBranch %90
%46 = OpLabel
%50 = OpFMul %24 %71 %30
%54 = OpSDiv %6 %73 %21
OpBranch %34
%34 = OpLabel
OpBranch %90
%90 = OpLabel
%77 = OpPhi %6 %45 %33 %54 %46
%76 = OpPhi %24 %40 %33 %50 %46
OpBranch %34
%34 = OpLabel
%57 = OpIAdd %6 %70 %56
OpBranch %10
%12 = OpLabel
@ -303,19 +304,21 @@ TEST(ValidationDuringReductionTest, CheckNotAlwaysInvalidCanMakeProgress) {
%29 = OpAccessChain %28 %27 %9
%30 = OpLoad %24 %29
%32 = OpFOrdGreaterThan %22 %30 %31
OpSelectionMerge %34 None
OpSelectionMerge %90 None
OpBranchConditional %32 %33 %46
%33 = OpLabel
%40 = OpFAdd %24 %71 %30
%45 = OpISub %6 %73 %21
OpBranch %34
OpBranch %90
%46 = OpLabel
%50 = OpFMul %24 %71 %30
%54 = OpSDiv %6 %73 %21
OpBranch %34
%34 = OpLabel
OpBranch %90
%90 = OpLabel
%77 = OpPhi %6 %45 %33 %54 %46
%76 = OpPhi %24 %40 %33 %50 %46
OpBranch %34
%34 = OpLabel
%57 = OpIAdd %6 %70 %56
OpBranch %10
%12 = OpLabel
@ -392,19 +395,21 @@ TEST(ValidationDuringReductionTest, CheckNotAlwaysInvalidCanMakeProgress) {
%29 = OpAccessChain %28 %27 %9
%30 = OpLoad %24 %29
%32 = OpFOrdGreaterThan %22 %30 %31
OpSelectionMerge %34 None
OpSelectionMerge %90 None
OpBranchConditional %32 %33 %46
%33 = OpLabel
%40 = OpFAdd %24 %71 %30
%45 = OpISub %6 %73 %21
OpBranch %34
OpBranch %90
%46 = OpLabel
%50 = OpFMul %24 %71 %30
%54 = OpSDiv %6 %73 %21
OpBranch %34
%34 = OpLabel
OpBranch %90
%90 = OpLabel
%77 = OpPhi %6 %45 %33 %54 %46
%76 = OpPhi %24 %40 %33 %50 %46
OpBranch %34
%34 = OpLabel
%57 = OpIAdd %6 %70 %56
OpBranch %10
%12 = OpLabel

143
test/val/val_cfg_test.cpp
View File

@ -23,7 +23,6 @@
#include <vector>
#include "gmock/gmock.h"
#include "source/diagnostic.h"
#include "source/spirv_target_env.h"
#include "source/val/validate.h"
@ -1355,7 +1354,7 @@ std::string GetReachableMergeAndContinue(SpvCapability cap,
}
if (cap == SpvCapabilityShader) {
branch.AppendBody("OpLoopMerge %merge %target None\n");
body.AppendBody("OpSelectionMerge %target None\n");
body.AppendBody("OpSelectionMerge %f None\n");
}
if (!spvIsWebGPUEnv(env))
@ -1650,25 +1649,27 @@ TEST_P(ValidateCFG, BackEdgeBlockDoesntPostDominateContinueTargetBad) {
Block entry("entry");
Block loop1("loop1", SpvOpBranchConditional);
Block loop2("loop2", SpvOpBranchConditional);
Block loop2_merge("loop2_merge", SpvOpBranchConditional);
Block loop2_merge("loop2_merge");
Block loop1_cont("loop1_cont", SpvOpBranchConditional);
Block be_block("be_block");
Block exit("exit", SpvOpReturn);
entry.SetBody("%cond = OpSLessThan %boolt %one %two\n");
if (is_shader) {
loop1.SetBody("OpLoopMerge %exit %loop2_merge None\n");
loop1.SetBody("OpLoopMerge %exit %loop1_cont None\n");
loop2.SetBody("OpLoopMerge %loop2_merge %loop2 None\n");
}
std::string str = GetDefaultHeader(GetParam()) +
nameOps("loop1", "loop2", "be_block", "loop2_merge") +
types_consts() +
"%func = OpFunction %voidt None %funct\n";
std::string str =
GetDefaultHeader(GetParam()) +
nameOps("loop1", "loop2", "be_block", "loop1_cont", "loop2_merge") +
types_consts() + "%func = OpFunction %voidt None %funct\n";
str += entry >> loop1;
str += loop1 >> std::vector<Block>({loop2, exit});
str += loop2 >> std::vector<Block>({loop2, loop2_merge});
str += loop2_merge >> std::vector<Block>({be_block, exit});
str += loop2_merge >> loop1_cont;
str += loop1_cont >> std::vector<Block>({be_block, exit});
str += be_block >> loop1;
str += exit;
str += "OpFunctionEnd";
@ -1678,7 +1679,7 @@ TEST_P(ValidateCFG, BackEdgeBlockDoesntPostDominateContinueTargetBad) {
ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
MatchesRegex("The continue construct with the continue target "
".\\[%loop2_merge\\] is not post dominated by the "
".\\[%loop1_cont\\] is not post dominated by the "
"back-edge block .\\[%be_block\\]\n"
" %be_block = OpLabel\n"));
} else {
@ -2037,13 +2038,10 @@ TEST_P(ValidateCFG,
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
}
TEST_P(ValidateCFG, ContinueTargetCanBeMergeBlockForNestedStructureGood) {
// This example is valid. It shows that the validator can't just add
// an edge from the loop head to the continue target. If that edge
// is added, then the "if_merge" block is both the continue target
// for the loop and also the merge block for the nested selection, but
// then it wouldn't be dominated by "if_head", the header block for the
// nested selection.
TEST_P(ValidateCFG, ContinueTargetCanBeMergeBlockForNestedStructure) {
// The continue construct cannot be the merge target of a nested selection
// because the loop construct must contain "if_merge" because it contains
// "if_head".
bool is_shader = GetParam() == SpvCapabilityShader;
Block entry("entry");
Block loop("loop");
@ -2072,7 +2070,16 @@ TEST_P(ValidateCFG, ContinueTargetCanBeMergeBlockForNestedStructureGood) {
str += "OpFunctionEnd";
CompileSuccessfully(str);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
if (is_shader) {
EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("Header block 3[%if_head] is contained in the loop construct "
"headed "
"by 2[%loop], but it's merge block 5[%if_merge] is not"));
} else {
EXPECT_THAT(SPV_SUCCESS, ValidateInstructions());
}
}
TEST_P(ValidateCFG, SingleLatchBlockMultipleBranchesToLoopHeader) {
@ -3681,17 +3688,21 @@ OpBranch %7
OpLoopMerge %8 %9 None
OpBranch %10
%10 = OpLabel
OpLoopMerge %9 %11 None
OpLoopMerge %11 %12 None
OpBranch %13
%13 = OpLabel
OpSelectionMerge %14 None
OpBranchConditional %3 %14 %15
%15 = OpLabel
OpBranch %8
%14 = OpLabel
OpBranch %12
%12 = OpLabel
OpSelectionMerge %11 None
OpBranchConditional %3 %11 %13
%13 = OpLabel
OpBranch %8
OpBranchConditional %3 %10 %11
%11 = OpLabel
OpBranchConditional %3 %9 %10
OpBranch %9
%9 = OpLabel
OpBranchConditional %3 %8 %7
OpBranchConditional %3 %7 %8
%8 = OpLabel
OpReturn
OpFunctionEnd
@ -3700,7 +3711,7 @@ OpFunctionEnd
CompileSuccessfully(text);
EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("block <ID> 13[%13] exits the loop headed by <ID> "
HasSubstr("block <ID> 15[%15] exits the loop headed by <ID> "
"10[%10], but not via a structured exit"));
}
@ -3726,9 +3737,11 @@ OpBranchConditional %undef %11 %12
OpSelectionMerge %31 None
OpBranchConditional %undef %30 %31
%30 = OpLabel
OpSelectionMerge %37 None
OpBranchConditional %undef %36 %37
OpSelectionMerge %38 None
OpBranchConditional %undef %36 %38
%36 = OpLabel
OpBranch %38
%38 = OpLabel
OpBranch %37
%37 = OpLabel
OpBranch %10
@ -4100,6 +4113,80 @@ OpFunctionEnd
EXPECT_THAT(getDiagnosticString(), HasSubstr("Selection must be structured"));
}
TEST_F(ValidateCFG, ContinueCannotBeSelectionMergeTarget) {
const std::string text = R"(
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpName %loop "loop"
OpName %continue "continue"
OpName %body "body"
%void = OpTypeVoid
%void_fn = OpTypeFunction %void
%bool = OpTypeBool
%undef = OpUndef %bool
%func = OpFunction %void None %void_fn
%entry = OpLabel
OpBranch %loop
%loop = OpLabel
OpLoopMerge %exit %continue None
OpBranch %body
%body = OpLabel
OpSelectionMerge %continue None
OpBranchConditional %undef %exit %continue
%continue = OpLabel
OpBranch %loop
%exit = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Header block 3[%body] is contained in the loop construct headed by "
"1[%loop], but it's merge block 2[%continue] is not"));
}
TEST_F(ValidateCFG, ContinueCannotBeLoopMergeTarget) {
const std::string text = R"(
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpName %loop "loop"
OpName %continue "continue"
OpName %inner "inner"
%void = OpTypeVoid
%void_fn = OpTypeFunction %void
%bool = OpTypeBool
%undef = OpUndef %bool
%func = OpFunction %void None %void_fn
%entry = OpLabel
OpBranch %loop
%loop = OpLabel
OpLoopMerge %exit %continue None
OpBranchConditional %undef %exit %inner
%inner = OpLabel
OpLoopMerge %continue %inner None
OpBranchConditional %undef %inner %continue
%continue = OpLabel
OpBranch %loop
%exit = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Header block 3[%inner] is contained in the loop construct headed by "
"1[%loop], but it's merge block 2[%continue] is not"));
}
} // namespace
} // namespace val
} // namespace spvtools