From 861099fc818370e863c4cbd22dac1a35e7444c17 Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 6 Oct 2022 16:55:10 +0000 Subject: [PATCH] spirv-reader: refactor definition info pertaining to pointers Change-Id: I3999f85c60a08f3eeb921b6931c07ca3bdd1f417 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/104760 Reviewed-by: Dan Sinclair Auto-Submit: David Neto Kokoro: Kokoro Commit-Queue: Dan Sinclair --- src/tint/reader/spirv/function.cc | 8 ++++---- src/tint/reader/spirv/function.h | 24 ++++++++++++++++-------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/tint/reader/spirv/function.cc b/src/tint/reader/spirv/function.cc index d00dad0eb5..4b7b3d3bb8 100644 --- a/src/tint/reader/spirv/function.cc +++ b/src/tint/reader/spirv/function.cc @@ -4809,7 +4809,7 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() { if (type->AsPointer()) { if (auto* ast_type = parser_impl_.ConvertType(inst.type_id())) { if (auto* ptr = ast_type->As()) { - info->address_space = ptr->address_space; + info->pointer.address_space = ptr->address_space; } } switch (inst.opcode()) { @@ -4823,7 +4823,7 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() { case SpvOpCopyObject: // Inherit from the first operand. We need this so we can pick up // a remapped storage buffer. - info->address_space = + info->pointer.address_space = GetAddressSpaceForPointerValue(inst.GetSingleWordInOperand(0)); break; default: @@ -4849,7 +4849,7 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() { ast::AddressSpace FunctionEmitter::GetAddressSpaceForPointerValue(uint32_t id) { auto where = def_info_.find(id); if (where != def_info_.end()) { - auto candidate = where->second.get()->address_space; + auto candidate = where->second.get()->pointer.address_space; if (candidate != ast::AddressSpace::kInvalid) { return candidate; } @@ -5053,7 +5053,7 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() { // Avoid moving combinatorial values across constructs. This is a // simple heuristic to avoid changing the cost of an operation // by moving it into or out of a loop, for example. - if ((def_info->address_space == ast::AddressSpace::kInvalid) && + if ((def_info->pointer.address_space == ast::AddressSpace::kInvalid) && local_def.used_in_another_construct) { should_hoist_to_let = true; } diff --git a/src/tint/reader/spirv/function.h b/src/tint/reader/spirv/function.h index 3295551884..dc6002e4a9 100644 --- a/src/tint/reader/spirv/function.h +++ b/src/tint/reader/spirv/function.h @@ -325,18 +325,26 @@ struct DefInfo { /// example, pointers. crbug.com/tint/98 bool requires_hoisted_var_def = false; - /// The address space to use for this value, if it is of pointer type. - /// This is required to carry an address space override from a storage - /// buffer expressed in the old style (with Uniform address space) - /// that needs to be remapped to StorageBuffer address space. - /// This is kInvalid for non-pointers. - ast::AddressSpace address_space = ast::AddressSpace::kInvalid; + /// Information about a pointer value, used to construct its WGSL type. + struct Pointer { + /// The address space to use for this value, if it is of pointer type. + /// This is required to carry an address space override from a storage + /// buffer expressed in the old style (with Uniform address space) + /// that needs to be remapped to StorageBuffer address space. + /// This is kInvalid for non-pointers. + ast::AddressSpace address_space = ast::AddressSpace::kInvalid; + + // TODO(crbug.com/tint/1041) track access mode. + }; /// The expression to use when sinking pointers into their use. /// When encountering a use of this instruction, we will emit this expression /// instead. TypedExpression sink_pointer_source_expr = {}; + /// Collected information about a pointer value. + Pointer pointer; + /// The reason, if any, that this value should be ignored. /// Normally no values are ignored. This field can be updated while /// generating code because sometimes we only discover necessary facts @@ -360,8 +368,8 @@ inline std::ostream& operator<<(std::ostream& o, const DefInfo& di) { } 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.address_space != ast::AddressSpace::kNone) { - o << " sc:" << int(di.address_space); + if (di.pointer.address_space != ast::AddressSpace::kNone) { + o << " sc:" << int(di.pointer.address_space); } switch (di.skip) { case SkipReason::kDontSkip: