spirv-reader: fix storage class for pointer-to pipeline builtins

The DefInfo structure is used for remapping storage buffer types
as well as tracking special values like builtin variables.
In the latter case, don't take the defaulted storage class value
from the DefInfo initialization.

Fixed: tint:1040, tint:1043
Change-Id: I41ee364d76e632736d51f4474c97036bcc136c93
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/59484
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: David Neto <dneto@google.com>
This commit is contained in:
David Neto 2021-07-26 18:23:58 +00:00 committed by Tint LUCI CQ
parent 4e4cef90fe
commit ad8eccea6f
4 changed files with 15 additions and 12 deletions

View File

@ -4699,7 +4699,10 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() {
ast::StorageClass FunctionEmitter::GetStorageClassForPointerValue(uint32_t id) { ast::StorageClass FunctionEmitter::GetStorageClassForPointerValue(uint32_t id) {
auto where = def_info_.find(id); auto where = def_info_.find(id);
if (where != def_info_.end()) { if (where != def_info_.end()) {
return where->second.get()->storage_class; auto candidate = where->second.get()->storage_class;
if (candidate != ast::StorageClass::kInvalid) {
return candidate;
}
} }
const auto type_id = def_use_mgr_->GetDef(id)->type_id(); const auto type_id = def_use_mgr_->GetDef(id)->type_id();
if (type_id) { if (type_id) {
@ -4868,7 +4871,7 @@ void FunctionEmitter::FindValuesNeedingNamedOrHoistedDefinition() {
// Avoid moving combinatorial values across constructs. This is a // Avoid moving combinatorial values across constructs. This is a
// 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::kNone) && if ((def_info->storage_class == ast::StorageClass::kInvalid) &&
def_info->used_in_another_construct) { def_info->used_in_another_construct) {
should_hoist = true; should_hoist = true;
} }

View File

@ -293,8 +293,8 @@ struct DefInfo {
/// This is required to carry a storage class override from a storage /// This is required to carry a storage class override from a storage
/// buffer expressed in the old style (with Uniform storage class) /// buffer expressed in the old style (with Uniform storage class)
/// that needs to be remapped to StorageBuffer storage class. /// that needs to be remapped to StorageBuffer storage class.
/// This is kNone for non-pointers. /// This is kInvalid for non-pointers.
ast::StorageClass storage_class = ast::StorageClass::kNone; ast::StorageClass storage_class = ast::StorageClass::kInvalid;
/// The reason, if any, that this value should be ignored. /// The reason, if any, that this value should be ignored.
/// Normally no values are ignored. This field can be updated while /// Normally no values are ignored. This field can be updated while

View File

@ -1216,7 +1216,7 @@ const Type* ParserImpl::ConvertType(uint32_t type_id,
remap_buffer_block_type_.insert(type_id); remap_buffer_block_type_.insert(type_id);
} }
// Pipeline intput and output variables map to private variables. // Pipeline input and output variables map to private variables.
if (ast_storage_class == ast::StorageClass::kInput || if (ast_storage_class == ast::StorageClass::kInput ||
ast_storage_class == ast::StorageClass::kOutput) { ast_storage_class == ast::StorageClass::kOutput) {
ast_storage_class = ast::StorageClass::kPrivate; ast_storage_class = ast::StorageClass::kPrivate;

View File

@ -2544,7 +2544,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_I32_Load_CopyObject) {
x_11 x_11
none none
undefined undefined
__ptr_none__i32 __ptr_private__i32
{ {
UnaryOp[not set]{ UnaryOp[not set]{
address-of address-of
@ -2779,7 +2779,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_U32_Load_CopyObject) {
x_11 x_11
none none
undefined undefined
__ptr_none__u32 __ptr_private__u32
{ {
UnaryOp[not set]{ UnaryOp[not set]{
address-of address-of
@ -4120,7 +4120,7 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_I32_Load_CopyObject) {
x_14 x_14
none none
undefined undefined
__ptr_none__i32 __ptr_private__i32
{ {
UnaryOp[not set]{ UnaryOp[not set]{
address-of address-of
@ -4389,7 +4389,7 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_U32_Load_CopyObject) {
x_14 x_14
none none
undefined undefined
__ptr_none__u32 __ptr_private__u32
{ {
UnaryOp[not set]{ UnaryOp[not set]{
address-of address-of
@ -4705,7 +4705,7 @@ TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_Load_CopyObject) {
x_14 x_14
none none
undefined undefined
__ptr_none__i32 __ptr_private__i32
{ {
UnaryOp[not set]{ UnaryOp[not set]{
address-of address-of
@ -4998,7 +4998,7 @@ TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_Load_CopyObject) {
x_14 x_14
none none
undefined undefined
__ptr_none__u32 __ptr_private__u32
{ {
UnaryOp[not set]{ UnaryOp[not set]{
address-of address-of
@ -5371,7 +5371,7 @@ TEST_P(SpvModuleScopeVarParserTest_ComputeBuiltin, Load_CopyObject) {
x_13 x_13
none none
undefined undefined
__ptr_none)" + wgsl_type + __ptr_private)" + wgsl_type +
R"( R"(
{ {
UnaryOp[not set]{ UnaryOp[not set]{