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>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index aaa6a13..40332d6 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -4699,7 +4699,10 @@
ast::StorageClass FunctionEmitter::GetStorageClassForPointerValue(uint32_t id) {
auto where = def_info_.find(id);
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();
if (type_id) {
@@ -4868,7 +4871,7 @@
// 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->storage_class == ast::StorageClass::kNone) &&
+ if ((def_info->storage_class == ast::StorageClass::kInvalid) &&
def_info->used_in_another_construct) {
should_hoist = true;
}
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index f4885b2..b803ad5 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -293,8 +293,8 @@
/// This is required to carry a storage class override from a storage
/// buffer expressed in the old style (with Uniform storage class)
/// that needs to be remapped to StorageBuffer storage class.
- /// This is kNone for non-pointers.
- ast::StorageClass storage_class = ast::StorageClass::kNone;
+ /// This is kInvalid for non-pointers.
+ ast::StorageClass storage_class = ast::StorageClass::kInvalid;
/// The reason, if any, that this value should be ignored.
/// Normally no values are ignored. This field can be updated while
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index 81a2805..0a7c053 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -1216,7 +1216,7 @@
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 ||
ast_storage_class == ast::StorageClass::kOutput) {
ast_storage_class = ast::StorageClass::kPrivate;
diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc
index 95ed7dd..b844a16 100644
--- a/src/reader/spirv/parser_impl_module_var_test.cc
+++ b/src/reader/spirv/parser_impl_module_var_test.cc
@@ -2544,7 +2544,7 @@
x_11
none
undefined
- __ptr_none__i32
+ __ptr_private__i32
{
UnaryOp[not set]{
address-of
@@ -2779,7 +2779,7 @@
x_11
none
undefined
- __ptr_none__u32
+ __ptr_private__u32
{
UnaryOp[not set]{
address-of
@@ -4120,7 +4120,7 @@
x_14
none
undefined
- __ptr_none__i32
+ __ptr_private__i32
{
UnaryOp[not set]{
address-of
@@ -4389,7 +4389,7 @@
x_14
none
undefined
- __ptr_none__u32
+ __ptr_private__u32
{
UnaryOp[not set]{
address-of
@@ -4705,7 +4705,7 @@
x_14
none
undefined
- __ptr_none__i32
+ __ptr_private__i32
{
UnaryOp[not set]{
address-of
@@ -4998,7 +4998,7 @@
x_14
none
undefined
- __ptr_none__u32
+ __ptr_private__u32
{
UnaryOp[not set]{
address-of
@@ -5371,7 +5371,7 @@
x_13
none
undefined
- __ptr_none)" + wgsl_type +
+ __ptr_private)" + wgsl_type +
R"(
{
UnaryOp[not set]{
