encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

spirv-reader: Refactor tracking of locally-defined values 

Based on suggestions from bclayton to use std::optional

Change-Id: I372472dbd5e239713eee5c2ec6ae6ea05fc384fa
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/102660
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
David Neto 2022-09-20 15:37:19 +00:00 committed by Dawn LUCI CQ
parent 6270ea7675
commit 7d9140feb9
2 changed files with 129 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -759,17 +759,22 @@ BlockInfo::BlockInfo(const spvtools::opt::BasicBlock& bb) : basic_block(&bb), id
BlockInfo::~BlockInfo() = default; BlockInfo::~BlockInfo() = default;
DefInfo::DefInfo(const spvtools::opt::Instruction& def_inst, DefInfo::DefInfo(size_t the_index,
bool the_locally_defined, const spvtools::opt::Instruction& def_inst,
uint32_t the_block_pos, uint32_t the_block_pos)
size_t the_index) : index(the_index), inst(def_inst), local(DefInfo::Local(the_block_pos)) {}
: inst(def_inst),
locally_defined(the_locally_defined), DefInfo::DefInfo(size_t the_index, const spvtools::opt::Instruction& def_inst)
block_pos(the_block_pos), : index(the_index), inst(def_inst) {}
index(the_index) {}
DefInfo::~DefInfo() = default; DefInfo::~DefInfo() = default;
DefInfo::Local::Local(uint32_t the_block_pos) : block_pos(the_block_pos) {}
DefInfo::Local::Local(const Local& other) = default;
DefInfo::Local::~Local() = default;
ast::Node* StatementBuilder::Clone(CloneContext*) const { ast::Node* StatementBuilder::Clone(CloneContext*) const {
return nullptr; return nullptr;
} }
@ -3380,7 +3385,7 @@ bool FunctionEmitter::EmitStatementsInBasicBlock(const BlockInfo& block_info,
utils::Vector<BlockInfo::PhiAssignment, 4> worklist; utils::Vector<BlockInfo::PhiAssignment, 4> worklist;
worklist.Reserve(block_info.phi_assignments.Length()); worklist.Reserve(block_info.phi_assignments.Length());
for (const auto assignment : block_info.phi_assignments) { for (const auto assignment : block_info.phi_assignments) {
if (GetDefInfo(assignment.phi_id)->num_uses > 0) { if (GetDefInfo(assignment.phi_id)->local->num_uses > 0) {
worklist.Push(assignment); worklist.Push(assignment);
} }
} }
@ -3462,7 +3467,7 @@ bool FunctionEmitter::WriteIfHoistedVar(const spvtools::opt::Instruction& inst,
TypedExpression expr) { TypedExpression expr) {
const auto result_id = inst.result_id(); const auto result_id = inst.result_id();
const auto* def_info = GetDefInfo(result_id); const auto* def_info = GetDefInfo(result_id);
if (def_info && def_info->requires_hoisted_def) { if (def_info && def_info->requires_hoisted_var_def) {
auto name = namer_.Name(result_id); auto name = namer_.Name(result_id);
// Emit an assignment of the expression to the hoisted variable. // Emit an assignment of the expression to the hoisted variable.
AddStatement(create<ast::AssignmentStatement>( AddStatement(create<ast::AssignmentStatement>(
@ -3512,8 +3517,11 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
} }
if (combinatorial_expr.expr != nullptr) { if (combinatorial_expr.expr != nullptr) {
if (def_info->requires_hoisted_def || def_info->requires_named_const_def || // If the expression is combinatorial, then it's not a direct access
def_info->num_uses != 1) { // of a builtin variable.
TINT_ASSERT(Reader, def_info->local.has_value());
if (def_info->requires_hoisted_var_def || def_info->requires_named_let_def ||
def_info->local->num_uses != 1) {
// Generate a const definition or an assignment to a hoisted definition // Generate a const definition or an assignment to a hoisted definition
// now and later use the const or variable name at the uses of this // now and later use the const or variable name at the uses of this
// value. // value.
@ -4739,7 +4747,7 @@ bool FunctionEmitter::RegisterSpecialBuiltInVariables() {
const auto id = special_var.first; const auto id = special_var.first;
const auto builtin = special_var.second; const auto builtin = special_var.second;
const auto* var = def_use_mgr_->GetDef(id); const auto* var = def_use_mgr_->GetDef(id);
def_info_[id] = std::make_unique<DefInfo>(*var, false, 0, index); def_info_[id] = std::make_unique<DefInfo>(index, *var);
++index; ++index;
auto& def = def_info_[id]; auto& def = def_info_[id];
// Builtins are always defined outside the function. // Builtins are always defined outside the function.
@ -4787,7 +4795,7 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() {
if ((result_id == 0) || inst.opcode() == SpvOpLabel) { if ((result_id == 0) || inst.opcode() == SpvOpLabel) {
continue; continue;
} }
def_info_[result_id] = std::make_unique<DefInfo>(inst, true, block_pos, index); def_info_[result_id] = std::make_unique<DefInfo>(index, inst, block_pos);
++index; ++index;
auto& info = def_info_[result_id]; auto& info = def_info_[result_id];
@ -4876,7 +4884,7 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
const auto id = inst.GetSingleWordInOperand(static_cast<uint32_t>(in_operand_index)); const auto id = inst.GetSingleWordInOperand(static_cast<uint32_t>(in_operand_index));
auto* const operand_def = GetDefInfo(id); auto* const operand_def = GetDefInfo(id);
if (operand_def) { if (operand_def) {
operand_def->requires_named_const_def = true; operand_def->requires_named_let_def = true;
} }
}; };
for (auto& id_def_info_pair : def_info_) { for (auto& id_def_info_pair : def_info_) {
@ -4913,18 +4921,21 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// Ignores values defined outside this function. // Ignores values defined outside this function.
auto record_value_use = [this](uint32_t id, const BlockInfo* block_info) { auto record_value_use = [this](uint32_t id, const BlockInfo* block_info) {
if (auto* def_info = GetDefInfo(id)) { if (auto* def_info = GetDefInfo(id)) {
if (def_info->local.has_value()) {
auto& local_def = def_info->local.value();
// Update usage count. // Update usage count.
def_info->num_uses++; local_def.num_uses++;
// Update usage span. // Update usage span.
def_info->first_use_pos = std::min(def_info->first_use_pos, block_info->pos); local_def.first_use_pos = std::min(local_def.first_use_pos, block_info->pos);
def_info->last_use_pos = std::max(def_info->last_use_pos, block_info->pos); local_def.last_use_pos = std::max(local_def.last_use_pos, block_info->pos);
// Determine whether this ID is defined in a different construct // Determine whether this ID is defined in a different construct
// from this use. // from this use.
const auto defining_block = block_order_[def_info->block_pos]; const auto defining_block = block_order_[local_def.block_pos];
const auto* def_in_construct = GetBlockInfo(defining_block)->construct; const auto* def_in_construct = GetBlockInfo(defining_block)->construct;
if (def_in_construct != block_info->construct) { if (def_in_construct != block_info->construct) {
def_info->used_in_another_construct = true; local_def.used_in_another_construct = true;
}
} }
} }
}; };
@ -4941,8 +4952,8 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// in the parent block B. // in the parent block B.
const auto phi_id = inst.result_id(); const auto phi_id = inst.result_id();
auto* phi_def_info = GetDefInfo(phi_id); auto& phi_local_def = GetDefInfo(phi_id)->local.value();
phi_def_info->is_phi = true; phi_local_def.is_phi = true;
// Track all the places where we need to mention the variable, // Track all the places where we need to mention the variable,
// so we can place its declaration. First, record the location of // so we can place its declaration. First, record the location of
@ -4962,9 +4973,9 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// Track where P needs to be in scope. It's not an ordinary use, so don't // Track where P needs to be in scope. It's not an ordinary use, so don't
// count it as one. // count it as one.
const auto pred_pos = pred_block_info->pos; const auto pred_pos = pred_block_info->pos;
phi_def_info->first_use_pos = phi_local_def.first_use_pos =
std::min(phi_def_info->first_use_pos, pred_pos); std::min(phi_local_def.first_use_pos, pred_pos);
phi_def_info->last_use_pos = std::max(phi_def_info->last_use_pos, pred_pos); phi_local_def.last_use_pos = std::max(phi_local_def.last_use_pos, pred_pos);
// Record the assignment that needs to occur at the end // Record the assignment that needs to occur at the end
// of the predecessor block. // of the predecessor block.
@ -4974,7 +4985,7 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// Schedule the declaration of the state variable. // Schedule the declaration of the state variable.
const auto* enclosing_construct = const auto* enclosing_construct =
GetEnclosingScope(phi_def_info->first_use_pos, phi_def_info->last_use_pos); GetEnclosingScope(phi_local_def.first_use_pos, phi_local_def.last_use_pos);
GetBlockInfo(enclosing_construct->begin_id)->phis_needing_state_vars.Push(phi_id); GetBlockInfo(enclosing_construct->begin_id)->phis_needing_state_vars.Push(phi_id);
} else { } else {
inst.ForEachInId([block_info, &record_value_use](const uint32_t* id_ptr) { inst.ForEachInId([block_info, &record_value_use](const uint32_t* id_ptr) {
@ -4998,22 +5009,24 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
for (auto& id_def_info_pair : def_info_) { for (auto& id_def_info_pair : def_info_) {
const auto def_id = id_def_info_pair.first; const auto def_id = id_def_info_pair.first;
auto* def_info = id_def_info_pair.second.get(); auto* def_info = id_def_info_pair.second.get();
if (def_info->num_uses == 0) { if (!def_info->local.has_value()) {
// There is no need to adjust the location of the declaration.
continue;
}
if (!def_info->locally_defined) {
// Never hoist a variable declared at module scope. // Never hoist a variable declared at module scope.
// This occurs for builtin variables, which are mapped to module-scope // This occurs for builtin variables, which are mapped to module-scope
// private variables. // private variables.
continue; continue;
} }
auto& local_def = def_info->local.value();
const auto* def_in_construct = GetBlockInfo(block_order_[def_info->block_pos])->construct; if (local_def.num_uses == 0) {
// There is no need to adjust the location of the declaration.
continue;
}
const auto* def_in_construct = GetBlockInfo(block_order_[local_def.block_pos])->construct;
// A definition in the first block of an kIfSelection or kSwitchSelection // A definition in the first block of an kIfSelection or kSwitchSelection
// occurs before the branch, and so that definition should count as // occurs before the branch, and so that definition should count as
// having been defined at the scope of the parent construct. // having been defined at the scope of the parent construct.
if (def_info->block_pos == def_in_construct->begin_pos) { if (local_def.block_pos == def_in_construct->begin_pos) {
if ((def_in_construct->kind == Construct::kIfSelection) || if ((def_in_construct->kind == Construct::kIfSelection) ||
(def_in_construct->kind == Construct::kSwitchSelection)) { (def_in_construct->kind == Construct::kSwitchSelection)) {
def_in_construct = def_in_construct->parent; def_in_construct = def_in_construct->parent;
@ -5022,12 +5035,12 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// We care about the earliest between the place of definition, and the first // We care about the earliest between the place of definition, and the first
// use of the value. // use of the value.
const auto first_pos = std::min(def_info->block_pos, def_info->first_use_pos); const auto first_pos = std::min(local_def.block_pos, local_def.first_use_pos);
const auto last_use_pos = def_info->last_use_pos; const auto last_use_pos = local_def.last_use_pos;
bool should_hoist_to_let = false; bool should_hoist_to_let = false;
bool should_hoist_to_var = false; bool should_hoist_to_var = false;
if (def_info->is_phi) { if (local_def.is_phi) {
// We need to generate a variable, and assignments to that variable in // We need to generate a variable, and assignments to that variable in
// all the phi parent blocks. // all the phi parent blocks.
should_hoist_to_var = true; should_hoist_to_var = true;
@ -5041,7 +5054,7 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// simple heuristic to avoid changing the cost of an operation // simple heuristic to avoid changing the cost of an operation
// by moving it into or out of a loop, for example. // by moving it into or out of a loop, for example.
if ((def_info->storage_class == ast::StorageClass::kInvalid) && if ((def_info->storage_class == ast::StorageClass::kInvalid) &&
def_info->used_in_another_construct) { local_def.used_in_another_construct) {
should_hoist_to_let = true; should_hoist_to_let = true;
} }
} }
@ -5050,11 +5063,11 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
const auto* enclosing_construct = GetEnclosingScope(first_pos, last_use_pos); const auto* enclosing_construct = GetEnclosingScope(first_pos, last_use_pos);
if (should_hoist_to_let && (enclosing_construct == def_in_construct)) { if (should_hoist_to_let && (enclosing_construct == def_in_construct)) {
// We can use a plain 'let' declaration. // We can use a plain 'let' declaration.
def_info->requires_named_const_def = true; def_info->requires_named_let_def = true;
} else { } else {
// We need to make a hoisted variable definition. // We need to make a hoisted variable definition.
// TODO(dneto): Handle non-storable types, particularly pointers. // TODO(dneto): Handle non-storable types, particularly pointers.
def_info->requires_hoisted_def = true; def_info->requires_hoisted_var_def = true;
auto* hoist_to_block = GetBlockInfo(enclosing_construct->begin_id); auto* hoist_to_block = GetBlockInfo(enclosing_construct->begin_id);
hoist_to_block->hoisted_ids.Push(def_id); hoist_to_block->hoisted_ids.Push(def_id);
} }

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

@ -159,7 +159,7 @@ struct BlockInfo {
/// The result IDs that this block is responsible for declaring as a /// The result IDs that this block is responsible for declaring as a
/// hoisted variable. /// hoisted variable.
/// @see DefInfo#requires_hoisted_def /// @see DefInfo#requires_hoisted_var_def
utils::Vector<uint32_t, 4> hoisted_ids; utils::Vector<uint32_t, 4> hoisted_ids;
/// A PhiAssignment represents the assignment of a value to the state /// A PhiAssignment represents the assignment of a value to the state
@ -241,34 +241,44 @@ enum class SkipReason {
/// function. /// function.
/// - certain module-scope builtin variables. /// - certain module-scope builtin variables.
struct DefInfo { struct DefInfo {
/// Constructor. /// Constructor for a locally defined value.
/// @param index an ordering index for uniqueness.
/// @param def_inst the SPIR-V instruction defining the ID /// @param def_inst the SPIR-V instruction defining the ID
/// @param locally_defined true if the defining instruction is in the function /// @param block_pos the position of the first basic block dominated by the
/// @param block_pos the position of the basic block where the ID is defined /// definition
/// @param index an ordering index for this local definition DefInfo(size_t index, const spvtools::opt::Instruction& def_inst, uint32_t block_pos);
DefInfo(const spvtools::opt::Instruction& def_inst, /// Constructor for a value defined at module scope.
bool locally_defined, /// @param index an ordering index for uniqueness.
uint32_t block_pos, /// @param def_inst the SPIR-V instruction defining the ID
size_t index); DefInfo(size_t index, const spvtools::opt::Instruction& def_inst);
/// Destructor. /// Destructor.
~DefInfo(); ~DefInfo();
/// The SPIR-V instruction that defines the ID.
const spvtools::opt::Instruction& inst;
/// True if the definition of this ID is inside the function.
const bool locally_defined = true;
/// For IDs defined in the function, this is the position of the block
/// containing the definition of the ID, in function block order.
/// For IDs defined outside of the function, it is 0.
/// See method `FunctionEmitter::ComputeBlockOrderAndPositions`
const uint32_t block_pos = 0;
/// An index for uniquely and deterministically ordering all DefInfo records /// An index for uniquely and deterministically ordering all DefInfo records
/// in a function. /// in a function.
const size_t index = 0; const size_t index = 0;
/// The SPIR-V instruction that defines the ID.
const spvtools::opt::Instruction& inst;
/// Information about a definition created inside a function.
struct Local {
/// Constructor.
/// @param block_pos the position of the basic block defining the value.
explicit Local(uint32_t block_pos);
/// Copy constructor.
/// @param other the original object to copy from.
Local(const Local& other);
/// Destructor.
~Local();
/// The position of the basic block defininig the value, in function
/// block order.
/// See method `FunctionEmitter::ComputeBlockOrderAndPositions` for block
/// ordering.
const uint32_t block_pos = 0;
/// The number of uses of this ID. /// The number of uses of this ID.
uint32_t num_uses = 0; uint32_t num_uses = 0;
@ -288,10 +298,17 @@ struct DefInfo {
/// Is this value used in a construct other than the one in which it was /// Is this value used in a construct other than the one in which it was
/// defined? /// defined?
bool used_in_another_construct = false; bool used_in_another_construct = false;
/// Is this ID an OpPhi?
bool is_phi = false;
};
/// True if this ID requires a WGSL 'const' definition, due to context. It /// Information about a definition inside the function. Populated if and only
/// if the definition actually is inside the function.
std::optional<Local> local;
/// True if this ID requires a WGSL 'let' definition, due to context. It
/// might get one anyway (so this is *not* an if-and-only-if condition). /// might get one anyway (so this is *not* an if-and-only-if condition).
bool requires_named_const_def = false; bool requires_named_let_def = false;
/// True if this ID must map to a WGSL variable declaration before the /// True if this ID must map to a WGSL variable declaration before the
/// corresponding position of the ID definition in SPIR-V. This compensates /// corresponding position of the ID definition in SPIR-V. This compensates
@ -306,10 +323,7 @@ struct DefInfo {
/// variable. /// variable.
/// TODO(dneto): This works for constants of storable type, but not, for /// TODO(dneto): This works for constants of storable type, but not, for
/// example, pointers. crbug.com/tint/98 /// example, pointers. crbug.com/tint/98
bool requires_hoisted_def = false; bool requires_hoisted_var_def = false;
/// Is this ID an OpPhi?
bool is_phi = false;
/// The storage class to use for this value, if it is of pointer type. /// The storage class to use for this value, if it is of pointer type.
/// This is required to carry a storage class override from a storage /// This is required to carry a storage class override from a storage
@ -336,14 +350,16 @@ struct DefInfo {
/// @returns the ostream so calls can be chained /// @returns the ostream so calls can be chained
inline std::ostream& operator<<(std::ostream& o, const DefInfo& di) { inline std::ostream& operator<<(std::ostream& o, const DefInfo& di) {
o << "DefInfo{" o << "DefInfo{"
<< " inst.result_id: " << di.inst.result_id() << " inst.result_id: " << di.inst.result_id();
<< " locally_defined: " << (di.locally_defined ? "true" : "false") if (di.local.has_value()) {
<< " block_pos: " << di.block_pos << " num_uses: " << di.num_uses const auto& dil = di.local.value();
<< " first_use_pos: " << di.first_use_pos << " last_use_pos: " << di.last_use_pos o << " block_pos: " << dil.block_pos << " num_uses: " << dil.num_uses
<< " used_in_another_construct: " << (di.used_in_another_construct ? "true" : "false") << " first_use_pos: " << dil.first_use_pos << " last_use_pos: " << dil.last_use_pos
<< " requires_named_const_def: " << (di.requires_named_const_def ? "true" : "false") << " used_in_another_construct: " << (dil.used_in_another_construct ? "true" : "false")
<< " requires_hoisted_def: " << (di.requires_hoisted_def ? "true" : "false") << " is_phi: " << (dil.is_phi ? "true" : "false") << "";
<< " is_phi: " << (di.is_phi ? "true" : "false") << ""; }
o << " requires_named_let_def: " << (di.requires_named_let_def ? "true" : "false")
<< " requires_hoisted_var_def: " << (di.requires_hoisted_var_def ? "true" : "false");
if (di.storage_class != ast::StorageClass::kNone) { if (di.storage_class != ast::StorageClass::kNone) {
o << " sc:" << int(di.storage_class); o << " sc:" << int(di.storage_class);
} }
@ -616,14 +632,14 @@ class FunctionEmitter {
/// - When a SPIR-V instruction might use the dynamically computed value /// - When a SPIR-V instruction might use the dynamically computed value
/// only once, but the WGSL code might reference it multiple times. /// only once, but the WGSL code might reference it multiple times.
/// For example, this occurs for the vector operands of OpVectorShuffle. /// For example, this occurs for the vector operands of OpVectorShuffle.
/// In this case the definition's DefInfo#requires_named_const_def property /// In this case the definition's DefInfo#requires_named_let_def property
/// is set to true. /// is set to true.
/// - When a definition and at least one of its uses are not in the /// - When a definition and at least one of its uses are not in the
/// same structured construct. /// same structured construct.
/// In this case the definition's DefInfo#requires_named_const_def property /// In this case the definition's DefInfo#requires_named_let_def property
/// is set to true. /// is set to true.
/// - When a definition is in a construct that does not enclose all the /// - When a definition is in a construct that does not enclose all the
/// uses. In this case the definition's DefInfo#requires_hoisted_def /// uses. In this case the definition's DefInfo#requires_hoisted_var_def
/// property is set to true. /// property is set to true.
/// Updates the `def_info_` mapping. /// Updates the `def_info_` mapping.
void FindValuesNeedingNamedOrHoistedDefinition(); void FindValuesNeedingNamedOrHoistedDefinition();