encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-12-18 01:15:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove fallthrough support from SPIRV-Reader.

Browse Source 
This CL removes support for fallthrough from the SPIRV-Reader.

Bug: tint:1644
Change-Id: I80b63d627960a82ba90de83af407c539b0442080
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/109004
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
This commit is contained in:
dan sinclair
2022-11-15 00:19:05 +00:00
committed by Dawn LUCI CQ
parent f0e6f57959
commit d8a986beae
48 changed files with 38 additions and 4124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
src/tint/reader/spirv/function.cc
View File

@@ -25,7 +25,6 @@
#include "src/tint/ast/call_statement.h"
#include "src/tint/ast/continue_statement.h"
#include "src/tint/ast/discard_statement.h"
#include "src/tint/ast/fallthrough_statement.h"
#include "src/tint/ast/if_statement.h"
#include "src/tint/ast/loop_statement.h"
#include "src/tint/ast/return_statement.h"
@@ -2159,8 +2158,7 @@ bool FunctionEmitter::ClassifyCFGEdges() {
// For each branch encountered, classify each edge (S,T) as:
// - a back-edge
// - a structured exit (specific ways of branching to enclosing construct)
// - a normal (forward) edge, either natural control flow or a case
// fallthrough
// - a normal (forward) edge, either natural control flow or a case fallthrough
//
// If more than one block is targeted by a normal edge, then S must be a
// structured header.
@@ -2194,11 +2192,10 @@ bool FunctionEmitter::ClassifyCFGEdges() {
// There should only be one backedge per backedge block.
uint32_t num_backedges = 0;
// Track destinations for normal forward edges, either kForward
// or kCaseFallThrough. These count toward the need
// to have a merge instruction. We also track kIfBreak edges
// because when used with normal forward edges, we'll need
// to generate a flow guard variable.
// Track destinations for normal forward edges, either kForward or kCaseFallThrough.
// These count toward the need to have a merge instruction. We also track kIfBreak edges
// because when used with normal forward edges, we'll need to generate a flow guard
// variable.
utils::Vector<uint32_t, 4> normal_forward_edges;
utils::Vector<uint32_t, 4> if_break_edges;
@@ -2376,6 +2373,12 @@ bool FunctionEmitter::ClassifyCFGEdges() {
<< dest_construct.begin_id << " (dominance rule violated)";
}
}
// Error on the fallthrough at the end in order to allow the better error messages
// from the above checks to happen.
if (edge_kind == EdgeKind::kCaseFallThrough) {
return Fail() << "Fallthrough not permitted in WGSL";
}
} // end forward edge
} // end successor
@@ -3261,11 +3264,9 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) {
// The fallthrough case is special because WGSL requires the fallthrough
// statement to be last in the case clause.
if (true_kind == EdgeKind::kCaseFallThrough) {
return EmitConditionalCaseFallThrough(block_info, cond, false_kind, *false_info,
true);
return Fail() << "Fallthrough not supported in WGSL";
} else if (false_kind == EdgeKind::kCaseFallThrough) {
return EmitConditionalCaseFallThrough(block_info, cond, true_kind, *true_info,
false);
return Fail() << "Fallthrough not supported in WGSL";
}
// At this point, at most one edge is kForward or kIfBreak.
@@ -3304,7 +3305,7 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) {
const ast::Statement* FunctionEmitter::MakeBranchDetailed(const BlockInfo& src_info,
const BlockInfo& dest_info,
bool forced,
std::string* flow_guard_name_ptr) const {
std::string* flow_guard_name_ptr) {
auto kind = src_info.succ_edge.find(dest_info.id)->second;
switch (kind) {
case EdgeKind::kBack:
@@ -3367,8 +3368,10 @@ const ast::Statement* FunctionEmitter::MakeBranchDetailed(const BlockInfo& src_i
// merge block is implicit.
break;
}
case EdgeKind::kCaseFallThrough:
return create<ast::FallthroughStatement>(Source{});
case EdgeKind::kCaseFallThrough: {
Fail() << "Fallthrough not supported in WGSL";
return nullptr;
}
case EdgeKind::kForward:
// Unconditional forward branch is implicit.
break;
@@ -3398,45 +3401,6 @@ const ast::Statement* FunctionEmitter::MakeSimpleIf(const ast::Expression* condi
return if_stmt;
}
bool FunctionEmitter::EmitConditionalCaseFallThrough(const BlockInfo& src_info,
const ast::Expression* cond,
EdgeKind other_edge_kind,
const BlockInfo& other_dest,
bool fall_through_is_true_branch) {
// In WGSL, the fallthrough statement must come last in the case clause.
// So we'll emit an if statement for the other branch, and then emit
// the fallthrough.
// We have two distinct destinations. But we only get here if this
// is a normal terminator; in particular the source block is *not* the
// start of an if-selection. So at most one branch is a kForward or
// kCaseFallThrough.
if (other_edge_kind == EdgeKind::kForward) {
return Fail() << "internal error: normal terminator OpBranchConditional has "
"both forward and fallthrough edges";
}
if (other_edge_kind == EdgeKind::kIfBreak) {
return Fail() << "internal error: normal terminator OpBranchConditional has "
"both IfBreak and fallthrough edges. Violates nesting rule";
}
if (other_edge_kind == EdgeKind::kBack) {
return Fail() << "internal error: normal terminator OpBranchConditional has "
"both backedge and fallthrough edges. Violates nesting rule";
}
auto* other_branch = MakeForcedBranch(src_info, other_dest);
if (other_branch == nullptr) {
return Fail() << "internal error: expected a branch for edge-kind " << int(other_edge_kind);
}
if (fall_through_is_true_branch) {
AddStatement(MakeSimpleIf(cond, nullptr, other_branch));
} else {
AddStatement(MakeSimpleIf(cond, other_branch, nullptr));
}
AddStatement(create<ast::FallthroughStatement>(Source{}));
return success();
}
bool FunctionEmitter::EmitStatementsInBasicBlock(const BlockInfo& block_info,
bool* already_emitted) {
if (*already_emitted) {

29
src/tint/reader/spirv/function.h
View File

@@ -34,15 +34,13 @@ namespace tint::reader::spirv {
//
// The edge kinds are used in many ways.
//
// For example, consider the edges leaving a basic block and going to distinct
// targets. If the total number of kForward + kIfBreak + kCaseFallThrough edges
// is more than 1, then the block must be a structured header, i.e. it needs
// a merge instruction to declare the control flow divergence and associated
// reconvergence point. Those those edge kinds count toward divergence
// because SPIR-v is designed to easily map back to structured control flow
// in GLSL (and C). In GLSL and C, those forward-flow edges don't have a
// special statement to express them. The other forward edges: kSwitchBreak,
// kLoopBreak, and kLoopContinue directly map to 'break', 'break', and
// For example, consider the edges leaving a basic block and going to distinct targets. If the
// total number of kForward + kIfBreak + kCaseFallThrough edges is more than 1, then the block must
// be a structured header, i.e. it needs a merge instruction to declare the control flow divergence
// and associated reconvergence point. Those those edge kinds count toward divergence because
// SPIR-V is designed to easily map back to structured control flow in GLSL (and C). In GLSL and C,
// those forward-flow edges don't have a special statement to express them. The other forward
// edges: kSwitchBreak, kLoopBreak, and kLoopContinue directly map to 'break', 'break', and
// 'continue', respectively.
enum class EdgeKind {
// A back-edge: An edge from a node to one of its ancestors in a depth-first
@@ -64,7 +62,8 @@ enum class EdgeKind {
// This can only occur for an "if" selection, i.e. where the selection
// header ends in OpBranchConditional.
kIfBreak,
// An edge from one switch case to the next sibling switch case.
// An edge from one switch case to the next sibling switch case. Note, this is not valid in WGSL
// at the moment and will trigger an ICE if encountered. It is here for completeness.
kCaseFallThrough,
// None of the above.
kForward
@@ -708,8 +707,7 @@ class FunctionEmitter {
/// Emits code for terminators, but that aren't part of entering or
/// resolving structured control flow. That is, if the basic block
/// terminator calls for it, emit the fallthrough, break, continue, return,
/// or kill commands.
/// terminator calls for it, emit the fallthrough break, continue, return, or kill commands.
/// @param block_info the block with the terminator to emit (if any)
/// @returns false if emission failed
bool EmitNormalTerminator(const BlockInfo& block_info);
@@ -722,7 +720,7 @@ class FunctionEmitter {
/// @param src_info the source block
/// @param dest_info the destination block
/// @returns the new statement, or a null statement
const ast::Statement* MakeBranch(const BlockInfo& src_info, const BlockInfo& dest_info) const {
const ast::Statement* MakeBranch(const BlockInfo& src_info, const BlockInfo& dest_info) {
return MakeBranchDetailed(src_info, dest_info, false, nullptr);
}
@@ -732,8 +730,7 @@ class FunctionEmitter {
/// @param src_info the source block
/// @param dest_info the destination block
/// @returns the new statement, or a null statement
const ast::Statement* MakeForcedBranch(const BlockInfo& src_info,
const BlockInfo& dest_info) const {
const ast::Statement* MakeForcedBranch(const BlockInfo& src_info, const BlockInfo& dest_info) {
return MakeBranchDetailed(src_info, dest_info, true, nullptr);
}
@@ -754,7 +751,7 @@ class FunctionEmitter {
const ast::Statement* MakeBranchDetailed(const BlockInfo& src_info,
const BlockInfo& dest_info,
bool forced,
std::string* flow_guard_name_ptr) const;
std::string* flow_guard_name_ptr);
/// Returns a new if statement with the given statements as the then-clause
/// and the else-clause. Either or both clauses might be nullptr. If both

1095
src/tint/reader/spirv/function_cfg_test.cc
View File

File diff suppressed because it is too large Load Diff

67
src/tint/reader/spirv/function_var_test.cc
View File

@@ -1344,73 +1344,6 @@ return;
EXPECT_EQ(expect, got) << got;
}
TEST_F(SpvParserFunctionVarTest, EmitStatement_Phi_InMerge_PredecessorsDominatdByNestedSwitchCase) {
// This is the essence of the bug report from crbug.com/tint/495
auto assembly = Preamble() + R"(
%cond = OpConstantTrue %bool
%pty = OpTypePointer Private %uint
%1 = OpVariable %pty Private
%boolpty = OpTypePointer Private %bool
%7 = OpVariable %boolpty Private
%8 = OpVariable %boolpty Private
%100 = OpFunction %void None %voidfn
%10 = OpLabel
OpSelectionMerge %99 None
OpSwitch %uint_1 %20 0 %20 1 %30
%20 = OpLabel ; case 0
OpBranch %30 ;; fall through
%30 = OpLabel ; case 1
OpSelectionMerge %50 None
OpBranchConditional %true %40 %45
%40 = OpLabel
OpBranch %50
%45 = OpLabel
OpBranch %99 ; break
%50 = OpLabel ; end the case
OpBranch %99
%99 = OpLabel
; predecessors are all dominated by case construct head at %30
%41 = OpPhi %uint %uint_0 %45 %uint_1 %50
%101 = OpCopyObject %uint %41 ; give it a use so it's emitted
OpReturn
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << assembly;
auto fe = p->function_emitter(100);
EXPECT_TRUE(fe.EmitBody()) << p->error();
auto ast_body = fe.ast_body();
auto got = test::ToString(p->program(), ast_body);
auto* expect = R"(var x_41 : u32;
switch(1u) {
case 0u, default: {
fallthrough;
}
case 1u: {
if (true) {
} else {
x_41 = 0u;
break;
}
x_41 = 1u;
}
}
let x_101 : u32 = x_41;
return;
)";
EXPECT_EQ(expect, got) << got << assembly;
}
TEST_F(SpvParserFunctionVarTest, EmitStatement_Phi_UseInPhiCountsAsUse) {
// From crbug.com/215
// If the only use of a combinatorially computed ID is as the value