spirv-reader: infer function storage class on OpAccessChain without indices
Fixed: tint:941
Bug: tint:807
Change-Id: I37bec630c1b83dbb56d2f8937322a5b5be81f407
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56660
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 03f45d9..a3a9fe8 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -3786,6 +3786,21 @@
return parser_impl_.RectifyOperandSignedness(inst, std::move(expr));
}
+TypedExpression FunctionEmitter::InferFunctionStorageClass(
+ TypedExpression expr) {
+ TypedExpression result(expr);
+ if (const auto* ref = expr.type->UnwrapAlias()->As<Reference>()) {
+ if (ref->storage_class == ast::StorageClass::kNone) {
+ expr.type = ty_.Reference(ref->type, ast::StorageClass::kFunction);
+ }
+ } else if (const auto* ptr = expr.type->UnwrapAlias()->As<Pointer>()) {
+ if (ptr->storage_class == ast::StorageClass::kNone) {
+ expr.type = ty_.Pointer(ptr->type, ast::StorageClass::kFunction);
+ }
+ }
+ return expr;
+}
+
TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue(
const spvtools::opt::Instruction& inst) {
if (inst.result_id() == 0) {
@@ -4118,7 +4133,7 @@
// ever-deeper nested indexing expressions. Start off with an expression
// for the base, and then bury that inside nested indexing expressions.
if (!current_expr) {
- current_expr = MakeOperand(inst, 0);
+ current_expr = InferFunctionStorageClass(MakeOperand(inst, 0));
if (current_expr.type->Is<Pointer>()) {
current_expr = Dereference(current_expr);
}
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index 06f0450..8bfa7db 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -964,6 +964,13 @@
TypedExpression MakeOperand(const spvtools::opt::Instruction& inst,
uint32_t operand_index);
+ /// Copies a typed expression to the result, but when the type is a pointer
+ /// or reference type, ensures the storage class is not defaulted. That is,
+ /// it changes a storage class of "none" to "function".
+ /// @param expr a typed expression
+ /// @results a copy of the expression, with possibly updated type
+ TypedExpression InferFunctionStorageClass(TypedExpression expr);
+
/// Returns an expression for a SPIR-V OpAccessChain or OpInBoundsAccessChain
/// instruction.
/// @param inst the SPIR-V instruction
diff --git a/src/reader/spirv/function_memory_test.cc b/src/reader/spirv/function_memory_test.cc
index 1b30749..bb16d54 100644
--- a/src/reader/spirv/function_memory_test.cc
+++ b/src/reader/spirv/function_memory_test.cc
@@ -903,6 +903,76 @@
EXPECT_EQ(got, expected) << got;
}
+TEST_F(SpvParserMemoryTest,
+ EmitStatement_AccessChain_InferFunctionStorageClass) {
+ // An access chain can have no indices. When the base is a Function variable,
+ // the reference type has no explicit storage class in the AST representation.
+ // But the pointer type for the let declaration must have an explicit
+ // 'function' storage class. From crbug.com/tint/807
+ const std::string assembly = R"(
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+
+%uint = OpTypeInt 32 0
+%ptr_ty = OpTypePointer Function %uint
+
+ %void = OpTypeVoid
+%voidfn = OpTypeFunction %void
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %1 = OpVariable %ptr_ty Function
+ %2 = OpAccessChain %ptr_ty %1
+ OpReturn
+ OpFunctionEnd
+)";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << assembly;
+ const auto got = p->program().to_str();
+ const std::string expected = R"(Module{
+ Function main_1 -> __void
+ ()
+ {
+ VariableDeclStatement{
+ Variable{
+ x_1
+ none
+ undefined
+ __u32
+ }
+ }
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ undefined
+ __ptr_function__u32
+ {
+ UnaryOp[not set]{
+ address-of
+ Identifier[not set]{x_1}
+ }
+ }
+ }
+ }
+ Return{}
+ }
+ Function main -> __void
+ StageDecoration{fragment}
+ ()
+ {
+ Call[not set]{
+ Identifier[not set]{main_1}
+ (
+ )
+ }
+ }
+}
+)";
+ EXPECT_EQ(got, expected) << got;
+}
+
std::string OldStorageBufferPreamble() {
return Preamble() + R"(
OpName %myvar "myvar"
