From a98b3a1f0c3c1cf823c2dd6e7943855c94742602 Mon Sep 17 00:00:00 2001
From: John Stiles <johnstiles@google.com>
Date: Wed, 1 Jun 2022 18:00:43 -0400
Subject: [PATCH] Reduce the size of DSLType.

This CL removes the TypeConstant member from DSLType. The constructor
which takes a TypeConstant now sets the `fSkSLType` field immediately
instead of saving the value and deferring lookup until `skslType()` is
called.

This caused some ripple-effect issues in type setup code which were
fixable by replacing nullptr with Poison.

Change-Id: I8fa73cdf5f0bcd3de143c9a25ea43392d75c7dec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545780
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
---
 include/sksl/DSLType.h   |  7 ++-----
 src/sksl/dsl/DSLType.cpp | 26 +++++++++++---------------
 src/sksl/ir/SkSLType.cpp |  8 ++++----
 3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/include/sksl/DSLType.h b/include/sksl/DSLType.h
index 4a3a17290c..6288438ccc 100644
--- a/include/sksl/DSLType.h
+++ b/include/sksl/DSLType.h
@@ -82,9 +82,7 @@ enum TypeConstant : uint8_t {
 
 class DSLType {
 public:
-    DSLType(TypeConstant tc, Position pos = {})
-        : fTypeConstant(tc)
-        , fPosition(pos) {}
+    DSLType(TypeConstant tc, Position pos = {});
 
     DSLType(const SkSL::Type* type, Position pos = {});
 
@@ -169,10 +167,9 @@ public:
     static DSLExpression Construct(DSLType type, SkSpan<DSLExpression> argArray);
 
 private:
-    const SkSL::Type& skslType() const;
+    const SkSL::Type& skslType() const { return *fSkSLType; }
 
     const SkSL::Type* fSkSLType = nullptr;
-    TypeConstant fTypeConstant = kPoison_Type;
     Position fPosition;
 
     friend DSLType Array(const DSLType& base, int count, Position pos);
diff --git a/src/sksl/dsl/DSLType.cpp b/src/sksl/dsl/DSLType.cpp
index e12a835dae..179451ed43 100644
--- a/src/sksl/dsl/DSLType.cpp
+++ b/src/sksl/dsl/DSLType.cpp
@@ -75,12 +75,12 @@ static const SkSL::Type* find_type(const Context& context,
                                    Position modifiersPos,
                                    Modifiers* modifiers) {
     const Type* type = find_type(context, overallPos, name);
-    type = type->applyPrecisionQualifiers(context, modifiers, ThreadContext::SymbolTable().get(),
-            modifiersPos);
-    return type;
+    return type->applyPrecisionQualifiers(context, modifiers, ThreadContext::SymbolTable().get(),
+                                          modifiersPos);
 }
 
-static const SkSL::Type* get_type_from_type_constant(const Context& context, TypeConstant tc) {
+static const SkSL::Type* get_type_from_type_constant(TypeConstant tc) {
+    const Context& context = ThreadContext::Context();
     switch (tc) {
         case kBool_Type:
             return context.fTypes.fBool.get();
@@ -185,6 +185,13 @@ static const SkSL::Type* get_type_from_type_constant(const Context& context, Typ
     }
 }
 
+DSLType::DSLType(TypeConstant tc, Position pos)
+    : fSkSLType(verify_type(ThreadContext::Context(),
+                            get_type_from_type_constant(tc),
+                            /*allowPrivateTypes=*/true,
+                            pos))
+    , fPosition(pos) {}
+
 DSLType::DSLType(std::string_view name, Position pos)
     : fSkSLType(find_type(ThreadContext::Context(), pos, name))
     , fPosition(pos) {}
@@ -246,17 +253,6 @@ bool DSLType::isEffectChild() const {
     return this->skslType().isEffectChild();
 }
 
-const SkSL::Type& DSLType::skslType() const {
-    if (fSkSLType) {
-        return *fSkSLType;
-    }
-    const Context& context = ThreadContext::Context();
-    return *verify_type(context,
-                        get_type_from_type_constant(context, fTypeConstant),
-                        /*allowPrivateTypes=*/true,
-                        Position());
-}
-
 DSLExpression DSLType::Construct(DSLType type, SkSpan<DSLExpression> argArray) {
     SkSL::ExpressionArray skslArgs;
     skslArgs.reserve_back(argArray.size());
diff --git a/src/sksl/ir/SkSLType.cpp b/src/sksl/ir/SkSLType.cpp
index d54ce90212..78e9bdaf08 100644
--- a/src/sksl/ir/SkSLType.cpp
+++ b/src/sksl/ir/SkSLType.cpp
@@ -637,12 +637,12 @@ const Type* Type::applyPrecisionQualifiers(const Context& context,
         // We want to discourage precision modifiers internally. Instead, use the type that
         // corresponds to the precision you need. (e.g. half vs float, short vs int)
         context.fErrors->error(pos, "precision qualifiers are not allowed");
-        return nullptr;
+        return context.fTypes.fPoison.get();
     }
 
     if ((int(lowp) + int(mediump) + int(highp)) != 1) {
         context.fErrors->error(pos, "only one precision qualifier can be used");
-        return nullptr;
+        return context.fTypes.fPoison.get();
     }
 
     // We're going to return a whole new type, so the modifier bits can be cleared out.
@@ -673,7 +673,7 @@ const Type* Type::applyPrecisionQualifiers(const Context& context,
                 break;
 
             default:
-                mediumpType = nullptr;
+                mediumpType = context.fTypes.fPoison.get();
                 break;
         }
 
@@ -687,7 +687,7 @@ const Type* Type::applyPrecisionQualifiers(const Context& context,
 
     context.fErrors->error(pos, "type '" + this->displayName() +
                                  "' does not support precision qualifiers");
-    return nullptr;
+    return context.fTypes.fPoison.get();
 }
 
 const Type& Type::toCompound(const Context& context, int columns, int rows) const {