From 919011af0a2ae1b663aae0aaa4083a3f9f13e66d Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Mon, 13 Jul 2020 14:16:12 +0000
Subject: [PATCH] [spirv-reader] Refactor how we emit const definitions

Take the type of a const-definition (or a write to a hoisted variable)
from the generated expression rather than the directly-converted type
from the SPIR-V ID.

This prepares the way for remapping storage buffers from
Uniform/BufferBlock to StorageBuffer/Block representation.

Bug: tint:99
Change-Id: I6104d8ad96312053562468a3cdb247932c1cb6cb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/24604
Reviewed-by: dan sinclair <dsinclair@chromium.org>
---
 src/reader/spirv/function.cc | 16 ++++++++++------
 src/reader/spirv/function.h  |  9 +++++----
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index b5bcb9d92a..c6ed309d8e 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -50,6 +50,7 @@
 #include "src/ast/storage_class.h"
 #include "src/ast/switch_statement.h"
 #include "src/ast/type/bool_type.h"
+#include "src/ast/type/pointer_type.h"
 #include "src/ast/type/type.h"
 #include "src/ast/type/u32_type.h"
 #include "src/ast/type/vector_type.h"
@@ -2480,9 +2481,8 @@ bool FunctionEmitter::EmitConstDefinition(
   if (!ast_expr.expr) {
     return false;
   }
-  auto ast_const =
-      parser_impl_.MakeVariable(inst.result_id(), ast::StorageClass::kNone,
-                                parser_impl_.ConvertType(inst.type_id()));
+  auto ast_const = parser_impl_.MakeVariable(
+      inst.result_id(), ast::StorageClass::kNone, ast_expr.type);
   if (!ast_const) {
     return false;
   }
@@ -2549,11 +2549,15 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
           std::move(lhs.expr), std::move(rhs.expr)));
       return success();
     }
-    case SpvOpLoad:
+    case SpvOpLoad: {
       // Memory accesses must be issued in SPIR-V program order.
       // So represent a load by a new const definition.
-      return EmitConstDefOrWriteToHoistedVar(
-          inst, MakeExpression(inst.GetSingleWordInOperand(0)));
+      auto expr = MakeExpression(inst.GetSingleWordInOperand(0));
+      // The load result type is the pointee type of its operand.
+      assert(expr.type->IsPointer());
+      expr.type = expr.type->AsPointer()->type();
+      return EmitConstDefOrWriteToHoistedVar(inst, std::move(expr));
+    }
     case SpvOpCopyObject:
       // Arguably, OpCopyObject is purely combinatorial. On the other hand,
       // it exists to make a new name for something. So we choose to make
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index 606bcdeed3..476818fe1b 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -525,16 +525,17 @@ class FunctionEmitter {
   /// @returns false if emission failed.
   bool EmitStatement(const spvtools::opt::Instruction& inst);
 
-  /// Emits a const definition for a SPIR-V value.
+  /// Emits a const definition for the typed value in |ast_expr|, and
+  /// records it as the translation for the result ID from |inst|.
   /// @param inst the SPIR-V instruction defining the value
   /// @param ast_expr the already-computed AST expression for the value
   /// @returns false if emission failed.
   bool EmitConstDefinition(const spvtools::opt::Instruction& inst,
                            TypedExpression ast_expr);
 
-  /// Emits a write to a hoisted variable for the given SPIR-V id,
-  /// if that ID has a hoisted declaration. Otherwise, emits a const
-  /// definition instead.
+  /// Emits a write of the typed value in |ast_expr| to a hoisted variable
+  /// for the given SPIR-V ID, if that ID has a hoisted declaration. Otherwise,
+  /// emits a const definition instead.
   /// @param inst the SPIR-V instruction defining the value
   /// @param ast_expr the already-computed AST expression for the value
   /// @returns false if emission failed.