Roll Skia from 2564767d24e5 to 7b1620f36648 (8 revisions)

https://skia.googlesource.com/skia.git/+log/2564767d24e5..7b1620f36648

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://skia-autoroll.corp.goog/r/android-master-autoroll
Please CC djsollen@google.com on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/master/autoroll/README.md

Test: Presubmit checks will test this change.
Exempt-From-Owner-Approval: The autoroll bot does not require owner approval.
Change-Id: I6dc1789dac984002d3e59a9ecc603ca671fae16a
diff --git a/Android.bp b/Android.bp
index ff44b93..a86dfbb 100644
--- a/Android.bp
+++ b/Android.bp
@@ -435,6 +435,7 @@
         "src/sksl/SkSLSectionAndParameterHelper.cpp",
         "src/sksl/SkSLString.cpp",
         "src/sksl/SkSLUtil.cpp",
+        "src/sksl/ir/SkSLIRNode.cpp",
         "src/sksl/ir/SkSLSetting.cpp",
         "src/sksl/ir/SkSLSymbolTable.cpp",
         "src/sksl/ir/SkSLType.cpp",
@@ -635,6 +636,7 @@
           "src/gpu/GrTextureProxy.cpp",
           "src/gpu/GrTextureRenderTargetProxy.cpp",
           "src/gpu/GrTextureResolveRenderTask.cpp",
+          "src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.cpp",
           "src/gpu/GrTransferFromRenderTask.cpp",
           "src/gpu/GrTriangulator.cpp",
           "src/gpu/GrUniformDataManager.cpp",
@@ -1638,6 +1640,7 @@
         "tests/GrTBlockListTest.cpp",
         "tests/GrTestingBackendTextureUploadTest.cpp",
         "tests/GrTextureMipMapInvalidationTest.cpp",
+        "tests/GrThreadSafeViewCacheTest.cpp",
         "tests/GrUploadPixelsTests.cpp",
         "tests/GradientTest.cpp",
         "tests/HSVRoundTripTest.cpp",
diff --git a/METADATA b/METADATA
index 14889d2..e05e42a 100644
--- a/METADATA
+++ b/METADATA
@@ -9,7 +9,7 @@
     type: GIT
     value: "https://skia.googlesource.com/skia"
   }
-  version: "2564767d24e558f48f9a91e0b20d20a2d6a8bbd6"
+  version: "7b1620f36648c8c31a2e03dc4f5df18e6a9c700b"
   license_type: RECIPROCAL
   last_upgrade_date {
     year: 2020
diff --git a/experimental/wasm-skp-debugger/debugger_bindings.cpp b/experimental/wasm-skp-debugger/debugger_bindings.cpp
index be92d3d..2f751c6 100644
--- a/experimental/wasm-skp-debugger/debugger_bindings.cpp
+++ b/experimental/wasm-skp-debugger/debugger_bindings.cpp
@@ -27,6 +27,7 @@
 #include <string>
 #include <string_view>
 #include <vector>
+#include <map>
 #include <emscripten.h>
 #include <emscripten/bind.h>
 
@@ -275,6 +276,29 @@
       return toSimpleImageInfo(fImages[index]->imageInfo());
     }
 
+    // returns a JSON string representing commands where each image is referenced.
+    std::string imageUseInfoForFrame(int framenumber) {
+      std::map<int, std::vector<int>> m = frames[framenumber]->getImageIdToCommandMap(udm);
+
+      SkDynamicMemoryWStream stream;
+      SkJSONWriter writer(&stream, SkJSONWriter::Mode::kFast);
+      writer.beginObject(); // root
+
+      for (auto it = m.begin(); it != m.end(); ++it) {
+        writer.beginArray(std::to_string(it->first).c_str());
+        for (const int commandId : it->second) {
+          writer.appendU64((uint64_t)commandId);
+        }
+        writer.endArray();
+      }
+
+      writer.endObject(); // root
+      writer.flush();
+      auto skdata = stream.detachAsData();
+      std::string_view data_view(reinterpret_cast<const char*>(skdata->data()), skdata->size());
+      return std::string(data_view);
+    }
+
     // return a list of layer draw events that happened at the beginning of this frame.
     std::vector<DebugLayerManager::LayerSummary> getLayerSummaries() {
       return fLayerManager->summarizeLayers(fp);
@@ -480,6 +504,7 @@
     .function("getImageInfo",         &SkpDebugPlayer::getImageInfo)
     .function("getLayerSummaries",    &SkpDebugPlayer::getLayerSummaries)
     .function("getSize",              &SkpDebugPlayer::getSize)
+    .function("imageUseInfoForFrame", &SkpDebugPlayer::imageUseInfoForFrame)
     .function("jsonCommandList",      &SkpDebugPlayer::jsonCommandList, allow_raw_pointers())
     .function("lastCommandInfo",      &SkpDebugPlayer::lastCommandInfo)
     .function("loadSkp",              &SkpDebugPlayer::loadSkp, allow_raw_pointers())
diff --git a/experimental/wasm-skp-debugger/externs.js b/experimental/wasm-skp-debugger/externs.js
index 0446d15..c417c52 100644
--- a/experimental/wasm-skp-debugger/externs.js
+++ b/experimental/wasm-skp-debugger/externs.js
@@ -45,6 +45,7 @@
 		setCommandVisibility: function() {},
 		jsonCommandList: function() {},
 		lastCommandInfo: function() {},
+		imageUseInfoForFrame: function() {},
 	},
 
 	/**
diff --git a/gn/gpu.gni b/gn/gpu.gni
index 46d22e5..0690900 100644
--- a/gn/gpu.gni
+++ b/gn/gpu.gni
@@ -248,6 +248,8 @@
   "$_src/gpu/GrTextureResolveManager.h",
   "$_src/gpu/GrTextureResolveRenderTask.cpp",
   "$_src/gpu/GrTextureResolveRenderTask.h",
+  "$_src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.cpp",
+  "$_src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.h",
   "$_src/gpu/GrTracing.h",
   "$_src/gpu/GrTransferFromRenderTask.cpp",
   "$_src/gpu/GrTransferFromRenderTask.h",
diff --git a/gn/sksl.gni b/gn/sksl.gni
index 3bfa0c6..9a6c286 100644
--- a/gn/sksl.gni
+++ b/gn/sksl.gni
@@ -68,6 +68,7 @@
   "$_src/sksl/ir/SkSLFunctionCall.h",
   "$_src/sksl/ir/SkSLFunctionDefinition.h",
   "$_src/sksl/ir/SkSLFunctionReference.h",
+  "$_src/sksl/ir/SkSLIRNode.cpp",
   "$_src/sksl/ir/SkSLIRNode.h",
   "$_src/sksl/ir/SkSLIfStatement.h",
   "$_src/sksl/ir/SkSLIndexExpression.h",
diff --git a/gn/tests.gni b/gn/tests.gni
index b12e794..60d0802 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -120,6 +120,7 @@
   "$_tests/GrTBlockListTest.cpp",
   "$_tests/GrTestingBackendTextureUploadTest.cpp",
   "$_tests/GrTextureMipMapInvalidationTest.cpp",
+  "$_tests/GrThreadSafeViewCacheTest.cpp",
   "$_tests/GrUploadPixelsTests.cpp",
   "$_tests/GradientTest.cpp",
   "$_tests/HSVRoundTripTest.cpp",
diff --git a/include/private/SkVx.h b/include/private/SkVx.h
index 0455550..19ff470 100644
--- a/include/private/SkVx.h
+++ b/include/private/SkVx.h
@@ -327,11 +327,6 @@
 SIT bool any(const Vec<1,T>& x) { return x.val != 0; }
 SIT bool all(const Vec<1,T>& x) { return x.val != 0; }
 
-SIT T min(const Vec<1,T>& x) { return x.val; }
-SIT T max(const Vec<1,T>& x) { return x.val; }
-
-SIT Vec<1,T> min(const Vec<1,T>& x, const Vec<1,T>& y) { return std::min(x.val, y.val); }
-SIT Vec<1,T> max(const Vec<1,T>& x, const Vec<1,T>& y) { return std::max(x.val, y.val); }
 SIT Vec<1,T> pow(const Vec<1,T>& x, const Vec<1,T>& y) { return std::pow(x.val, y.val); }
 
 SIT Vec<1,T>  atan(const Vec<1,T>& x) { return std:: atan(x.val); }
@@ -351,6 +346,15 @@
 SIT Vec<1,T> rsqrt(const Vec<1,T>& x) { return rcp(sqrt(x)); }
 
 // All default N != 1 implementations just recurse on lo and hi halves.
+
+// Clang can reason about naive_if_then_else() and optimize through it better
+// than if_then_else(), so it's sometimes useful to call it directly when we
+// think an entire expression should optimize away, e.g. min()/max().
+SINT Vec<N,T> naive_if_then_else(const Vec<N,M<T>>& cond, const Vec<N,T>& t, const Vec<N,T>& e) {
+    return bit_pun<Vec<N,T>>(( cond & bit_pun<Vec<N, M<T>>>(t)) |
+                             (~cond & bit_pun<Vec<N, M<T>>>(e)) );
+}
+
 SINT Vec<N,T> if_then_else(const Vec<N,M<T>>& cond, const Vec<N,T>& t, const Vec<N,T>& e) {
     // Specializations inline here so they can generalize what types the apply to.
     // (This header is used in C++14 contexts, so we have to kind of fake constexpr if.)
@@ -381,22 +385,12 @@
                     if_then_else(cond.hi, t.hi, e.hi));
     }
     // This default can lead to better code than the recursing onto scalars.
-    return bit_pun<Vec<N,T>>(( cond & bit_pun<Vec<N, M<T>>>(t)) |
-                             (~cond & bit_pun<Vec<N, M<T>>>(e)) );
+    return naive_if_then_else(cond, t, e);
 }
 
 SINT bool any(const Vec<N,T>& x) { return any(x.lo) || any(x.hi); }
 SINT bool all(const Vec<N,T>& x) { return all(x.lo) && all(x.hi); }
 
-SINT T min(const Vec<N,T>& x) { return std::min(min(x.lo), min(x.hi)); }
-SINT T max(const Vec<N,T>& x) { return std::max(max(x.lo), max(x.hi)); }
-
-SINT Vec<N,T> min(const Vec<N,T>& x, const Vec<N,T>& y) {
-    return join(min(x.lo, y.lo), min(x.hi, y.hi));
-}
-SINT Vec<N,T> max(const Vec<N,T>& x, const Vec<N,T>& y) {
-    return join(max(x.lo, y.lo), max(x.hi, y.hi));
-}
 SINT Vec<N,T> pow(const Vec<N,T>& x, const Vec<N,T>& y) {
     return join(pow(x.lo, y.lo), pow(x.hi, y.hi));
 }
@@ -432,8 +426,6 @@
 SINTU Vec<N,M<T>> operator>=(U x, const Vec<N,T>& y) { return Vec<N,T>(x) >= y; }
 SINTU Vec<N,M<T>> operator< (U x, const Vec<N,T>& y) { return Vec<N,T>(x) <  y; }
 SINTU Vec<N,M<T>> operator> (U x, const Vec<N,T>& y) { return Vec<N,T>(x) >  y; }
-SINTU Vec<N,T>           min(U x, const Vec<N,T>& y) { return min(Vec<N,T>(x), y); }
-SINTU Vec<N,T>           max(U x, const Vec<N,T>& y) { return max(Vec<N,T>(x), y); }
 SINTU Vec<N,T>           pow(U x, const Vec<N,T>& y) { return pow(Vec<N,T>(x), y); }
 
 // ... and same deal for vector/scalar operations.
@@ -450,8 +442,6 @@
 SINTU Vec<N,M<T>> operator>=(const Vec<N,T>& x, U y) { return x >= Vec<N,T>(y); }
 SINTU Vec<N,M<T>> operator< (const Vec<N,T>& x, U y) { return x <  Vec<N,T>(y); }
 SINTU Vec<N,M<T>> operator> (const Vec<N,T>& x, U y) { return x >  Vec<N,T>(y); }
-SINTU Vec<N,T>           min(const Vec<N,T>& x, U y) { return min(x, Vec<N,T>(y)); }
-SINTU Vec<N,T>           max(const Vec<N,T>& x, U y) { return max(x, Vec<N,T>(y)); }
 SINTU Vec<N,T>           pow(const Vec<N,T>& x, U y) { return pow(x, Vec<N,T>(y)); }
 
 // The various op= operators, for vectors...
@@ -489,6 +479,21 @@
 #endif
 }
 
+// min/max match logic of std::min/std::max, which is important when NaN is involved.
+SIT  T min(const Vec<1,T>& x) { return x.val; }
+SIT  T max(const Vec<1,T>& x) { return x.val; }
+SINT T min(const Vec<N,T>& x) { return std::min(min(x.lo), min(x.hi)); }
+SINT T max(const Vec<N,T>& x) { return std::max(max(x.lo), max(x.hi)); }
+
+SINT Vec<N,T> min(const Vec<N,T>& x, const Vec<N,T>& y) { return naive_if_then_else(y < x, y, x); }
+SINT Vec<N,T> max(const Vec<N,T>& x, const Vec<N,T>& y) { return naive_if_then_else(x < y, y, x); }
+
+SINTU Vec<N,T> min(const Vec<N,T>& x, U y) { return min(x, Vec<N,T>(y)); }
+SINTU Vec<N,T> max(const Vec<N,T>& x, U y) { return max(x, Vec<N,T>(y)); }
+SINTU Vec<N,T> min(U x, const Vec<N,T>& y) { return min(Vec<N,T>(x), y); }
+SINTU Vec<N,T> max(U x, const Vec<N,T>& y) { return max(Vec<N,T>(x), y); }
+
+
 // Shuffle values from a vector pretty arbitrarily:
 //    skvx::Vec<4,float> rgba = {R,G,B,A};
 //    shuffle<2,1,0,3>        (rgba) ~> {B,G,R,A}
@@ -704,12 +709,6 @@
         SI Vec<4, float> rsqrt(const Vec<4, float>& x) { return 1.0f / sqrt(x); }
         SI Vec<2,double> rsqrt(const Vec<2,double>& x) { return 1.0f / sqrt(x); }
 
-        SI Vec<4,float> min(const Vec<4,float>& x, const Vec<4,float>& y) {
-            return to_vec<4,float>(wasm_f32x4_min(to_vext(x), to_vext(y)));
-        }
-        SI Vec<4,float> max(const Vec<4,float>& x, const Vec<4,float>& y) {
-            return to_vec<4,float>(wasm_f32x4_max(to_vext(x), to_vext(y)));
-        }
         SI Vec<4,float> sqrt(const Vec<4,float>& x) {
             return to_vec<4,float>(wasm_f32x4_sqrt(to_vext(x)));
         }
@@ -717,12 +716,6 @@
             return to_vec<4,float>(wasm_f32x4_abs(to_vext(x)));
         }
 
-        SI Vec<2,double> min(const Vec<2,double>& x, const Vec<2,double>& y) {
-            return to_vec<2,double>(wasm_f64x2_min(to_vext(x), to_vext(y)));
-        }
-        SI Vec<2,double> max(const Vec<2,double>& x, const Vec<2,double>& y) {
-            return to_vec<2,double>(wasm_f64x2_max(to_vext(x), to_vext(y)));
-        }
         SI Vec<2,double> sqrt(const Vec<2,double>& x) {
             return to_vec<2,double>(wasm_f64x2_sqrt(to_vext(x)));
         }
@@ -735,22 +728,9 @@
         SI bool all(const Vec<4, int32_t>& x) { return wasm_i32x4_all_true(to_vext(x)); }
         SI bool all(const Vec<4,uint32_t>& x) { return wasm_i32x4_all_true(to_vext(x)); }
 
-        SI Vec<4,int32_t> min(const Vec<4,int32_t>& x, const Vec<4,int32_t>& y) {
-            return to_vec<4,int32_t>(wasm_i32x4_min(to_vext(x), to_vext(y)));
-        }
-        SI Vec<4,int32_t> max(const Vec<4,int32_t>& x, const Vec<4,int32_t>& y) {
-            return to_vec<4,int32_t>(wasm_i32x4_max(to_vext(x), to_vext(y)));
-        }
         SI Vec<4,int32_t> abs(const Vec<4,int32_t>& x) {
             return to_vec<4,int32_t>(wasm_i32x4_abs(to_vext(x)));
         }
-
-        SI Vec<4,uint32_t> min(const Vec<4,uint32_t>& x, const Vec<4,uint32_t>& y) {
-            return to_vec<4,uint32_t>(wasm_u32x4_min(to_vext(x), to_vext(y)));
-        }
-        SI Vec<4,uint32_t> max(const Vec<4,uint32_t>& x, const Vec<4,uint32_t>& y) {
-            return to_vec<4,uint32_t>(wasm_u32x4_max(to_vext(x), to_vext(y)));
-        }
     #endif
 
 #endif // !defined(SKNX_NO_SIMD)
diff --git a/src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.cpp b/src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.cpp
new file mode 100644
index 0000000..ee02b75
--- /dev/null
+++ b/src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.cpp
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.h"
+
+GrThreadSafeUniquelyKeyedProxyViewCache::GrThreadSafeUniquelyKeyedProxyViewCache() {}
+
+GrThreadSafeUniquelyKeyedProxyViewCache::~GrThreadSafeUniquelyKeyedProxyViewCache() {
+    fUniquelyKeyedProxyViews.foreach([](const GrUniqueKey&k, Entry** v) { delete *v; });
+}
+
+#if GR_TEST_UTILS
+int GrThreadSafeUniquelyKeyedProxyViewCache::numEntries() const {
+    SkAutoSpinlock lock{fSpinLock};
+
+    return fUniquelyKeyedProxyViews.count();
+}
+
+size_t GrThreadSafeUniquelyKeyedProxyViewCache::approxBytesUsed() const {
+    SkAutoSpinlock lock{fSpinLock};
+
+    return fUniquelyKeyedProxyViews.approxBytesUsed();
+}
+#endif
+
+void GrThreadSafeUniquelyKeyedProxyViewCache::dropAllRefs() {
+    SkAutoSpinlock lock{fSpinLock};
+
+    fUniquelyKeyedProxyViews.reset();
+}
+
+GrSurfaceProxyView GrThreadSafeUniquelyKeyedProxyViewCache::find(const GrUniqueKey& key) {
+    SkAutoSpinlock lock{fSpinLock};
+
+    Entry** tmp = fUniquelyKeyedProxyViews.find(const_cast<GrUniqueKey&>(key));
+    if (tmp) {
+        return (*tmp)->fView;
+    }
+
+    return {};
+}
+
+GrSurfaceProxyView GrThreadSafeUniquelyKeyedProxyViewCache::internalAdd(
+                                                                const GrUniqueKey& key,
+                                                                const GrSurfaceProxyView& view) {
+    Entry** tmp = fUniquelyKeyedProxyViews.find(const_cast<GrUniqueKey&>(key));
+    if (!tmp) {
+        // TODO: block allocate here?
+        Entry* newT = new Entry(key, view);
+        tmp = fUniquelyKeyedProxyViews.set(newT->fKey, newT);
+    }
+
+    return (*tmp)->fView;
+}
+
+GrSurfaceProxyView GrThreadSafeUniquelyKeyedProxyViewCache::add(const GrUniqueKey& key,
+                                                                const GrSurfaceProxyView& view) {
+    SkAutoSpinlock lock{fSpinLock};
+
+    return this->internalAdd(key, view);
+}
diff --git a/src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.h b/src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.h
new file mode 100644
index 0000000..1b2c078
--- /dev/null
+++ b/src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrThreadSafeUniquelyKeyedProxyViewCache_DEFINED
+#define GrThreadSafeUniquelyKeyedProxyViewCache_DEFINED
+
+#include "include/private/SkSpinlock.h"
+#include "include/private/SkTHash.h"
+#include "src/gpu/GrSurfaceProxyView.h"
+
+// Ganesh creates a lot of utility textures (e.g., blurred-rrect masks) that need to be shared
+// between the direct context and all the DDL recording contexts. This thread-safe cache
+// allows this sharing.
+//
+// In operation, each thread will first check if the threaded cache possesses the required texture.
+//
+// If a DDL thread doesn't find a needed texture it will go off and create it on the cpu and then
+// attempt to add it to the cache. If another thread had added it in the interim, the losing thread
+// will discard its work and use the texture the winning thread had created.
+//
+// If the thread in possession of the direct context doesn't find the needed texture it should
+// add a place holder view and then queue up the draw calls to complete it. In this way the
+// gpu-thread has precedence over the recording threads.
+//
+// The invariants for this cache differ a bit from those of the proxy and resource caches.
+// For this cache:
+//
+//   only this cache knows the unique key - neither the proxy nor backing resource should
+//              be discoverable in any other cache by the unique key
+//   if a backing resource resides in the resource cache then there should be an entry in this
+//              cache
+//   an entry in this cache, however, doesn't guarantee that there is a corresponding entry in
+//              the resource cache - although the entry here should be able to generate that entry
+//              (i.e., be a lazy proxy)
+class GrThreadSafeUniquelyKeyedProxyViewCache {
+public:
+    GrThreadSafeUniquelyKeyedProxyViewCache();
+    ~GrThreadSafeUniquelyKeyedProxyViewCache();
+
+#if GR_TEST_UTILS
+    int numEntries() const  SK_EXCLUDES(fSpinLock);
+    size_t approxBytesUsed() const   SK_EXCLUDES(fSpinLock);
+#endif
+
+    void dropAllRefs() SK_EXCLUDES(fSpinLock);
+
+    GrSurfaceProxyView find(const GrUniqueKey&)  SK_EXCLUDES(fSpinLock);
+
+    GrSurfaceProxyView add(const GrUniqueKey&, const GrSurfaceProxyView&)  SK_EXCLUDES(fSpinLock);
+
+private:
+    struct Entry {
+        Entry(const GrUniqueKey& key, const GrSurfaceProxyView& view) : fKey(key), fView(view) {}
+
+        // Note: the unique key is stored here bc it is never attached to a proxy or a GrTexture
+        GrUniqueKey        fKey;
+        GrSurfaceProxyView fView;
+    };
+
+    GrSurfaceProxyView internalAdd(const GrUniqueKey&,
+                                   const GrSurfaceProxyView&)  SK_REQUIRES(fSpinLock);
+
+    mutable SkSpinlock fSpinLock;
+
+    struct KeyHash {
+        uint32_t operator()(const GrUniqueKey& key) { return key.hash(); }
+    };
+
+    // TODO: it sure would be cool if the key could be a const& to the version stored in 'Entry'
+    SkTHashMap<GrUniqueKey, Entry*, KeyHash> fUniquelyKeyedProxyViews  SK_GUARDED_BY(fSpinLock);
+};
+
+#endif // GrThreadSafeUniquelyKeyedProxyViewCache_DEFINED
diff --git a/src/opts/SkVM_opts.h b/src/opts/SkVM_opts.h
index 9b4c6ce..2839e32 100644
--- a/src/opts/SkVM_opts.h
+++ b/src/opts/SkVM_opts.h
@@ -266,8 +266,6 @@
                         r[d].f32 = skvx::from_half(skvx::cast<uint16_t>(r[x].i32));
                         break;
 
-                    // TODO: make sure these q14x2 ops have good codegen
-
                     CASE(Op::add_q14x2): r[d].i16x2 = r[x].i16x2 + r[y].i16x2; break;
                     CASE(Op::sub_q14x2): r[d].i16x2 = r[x].i16x2 - r[y].i16x2; break;
 
@@ -291,8 +289,8 @@
                                                            0x4000)>>15 )<<1;
                         break;
 
-                    // TODO: this op exists only so it can be implemented as pavgw / vrhadd.u16
-                    // without expanding up to 32-bit.
+                    // Happily, Clang can see through this one and generates perfect code
+                    // using vpavgw without any help from us!
                     CASE(Op::uavg_q14x2):
                         r[d].u16x2 = skvx::cast<uint16_t>( (skvx::cast<int>(r[x].u16x2) +
                                                             skvx::cast<int>(r[y].u16x2) + 1)>>1 );
diff --git a/src/sksl/SkSLASTNode.h b/src/sksl/SkSLASTNode.h
index c65429c..795acbb 100644
--- a/src/sksl/SkSLASTNode.h
+++ b/src/sksl/SkSLASTNode.h
@@ -16,11 +16,6 @@
 
 namespace SkSL {
 
-// std::max isn't constexpr in some compilers
-static constexpr size_t Max(size_t a, size_t b) {
-    return a > b ? a : b;
-}
-
 /**
  * Represents a node in the abstract syntax tree (AST). The AST is based directly on the parse tree;
  * it is a parsed-but-not-yet-analyzed version of the program.
@@ -267,18 +262,18 @@
     };
 
     struct NodeData {
-        char fBytes[Max(sizeof(Token),
-                    Max(sizeof(StringFragment),
-                    Max(sizeof(bool),
-                    Max(sizeof(SKSL_INT),
-                    Max(sizeof(SKSL_FLOAT),
-                    Max(sizeof(Modifiers),
-                    Max(sizeof(TypeData),
-                    Max(sizeof(FunctionData),
-                    Max(sizeof(ParameterData),
-                    Max(sizeof(VarData),
-                    Max(sizeof(InterfaceBlockData),
-                        sizeof(SectionData))))))))))))];
+        char fBytes[std::max({sizeof(Token),
+                              sizeof(StringFragment),
+                              sizeof(bool),
+                              sizeof(SKSL_INT),
+                              sizeof(SKSL_FLOAT),
+                              sizeof(Modifiers),
+                              sizeof(TypeData),
+                              sizeof(FunctionData),
+                              sizeof(ParameterData),
+                              sizeof(VarData),
+                              sizeof(InterfaceBlockData),
+                              sizeof(SectionData)})];
 
         enum class Kind {
             kToken,
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
index 6496965..82ac423 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -266,7 +266,7 @@
             return false;
         case Expression::Kind::kBinary: {
             const BinaryExpression& b = e.as<BinaryExpression>();
-            return this->visitExpression(*b.fLeft) || this->visitExpression(*b.fRight); }
+            return this->visitExpression(b.left()) || this->visitExpression(b.right()); }
         case Expression::Kind::kConstructor: {
             const Constructor& c = e.as<Constructor>();
             for (const auto& arg : c.fArguments) {
diff --git a/src/sksl/SkSLByteCodeGenerator.cpp b/src/sksl/SkSLByteCodeGenerator.cpp
index a213ca4..1e9dab0 100644
--- a/src/sksl/SkSLByteCodeGenerator.cpp
+++ b/src/sksl/SkSLByteCodeGenerator.cpp
@@ -681,28 +681,29 @@
 }
 
 bool ByteCodeGenerator::writeBinaryExpression(const BinaryExpression& b, bool discard) {
-    if (b.fOperator == Token::Kind::TK_EQ) {
-        std::unique_ptr<LValue> lvalue = this->getLValue(*b.fLeft);
-        this->writeExpression(*b.fRight);
+    const Expression& left = b.left();
+    const Expression& right = b.right();
+    Token::Kind op = b.getOperator();
+    if (op == Token::Kind::TK_EQ) {
+        std::unique_ptr<LValue> lvalue = this->getLValue(left);
+        this->writeExpression(right);
         lvalue->store(discard);
         discard = false;
         return discard;
     }
-    const Type& lType = b.fLeft->type();
-    const Type& rType = b.fRight->type();
+    const Type& lType = left.type();
+    const Type& rType = right.type();
     bool lVecOrMtx = (lType.typeKind() == Type::TypeKind::kVector ||
                       lType.typeKind() == Type::TypeKind::kMatrix);
     bool rVecOrMtx = (rType.typeKind() == Type::TypeKind::kVector ||
                       rType.typeKind() == Type::TypeKind::kMatrix);
-    Token::Kind op;
     std::unique_ptr<LValue> lvalue;
-    if (Compiler::IsAssignment(b.fOperator)) {
-        lvalue = this->getLValue(*b.fLeft);
+    if (Compiler::IsAssignment(op)) {
+        lvalue = this->getLValue(left);
         lvalue->load();
-        op = Compiler::RemoveAssignment(b.fOperator);
+        op = Compiler::RemoveAssignment(op);
     } else {
-        this->writeExpression(*b.fLeft);
-        op = b.fOperator;
+        this->writeExpression(left);
         if (!lVecOrMtx && rVecOrMtx) {
             for (int i = SlotCount(rType); i > 1; --i) {
                 this->write(ByteCodeInstruction::kDup, 1);
@@ -718,7 +719,7 @@
             this->write(ByteCodeInstruction::kMaskPush);
             this->write(ByteCodeInstruction::kBranchIfAllFalse);
             DeferredLocation falseLocation(this);
-            this->writeExpression(*b.fRight);
+            this->writeExpression(right);
             this->write(ByteCodeInstruction::kAndB, 1);
             falseLocation.set();
             this->write(ByteCodeInstruction::kMaskPop);
@@ -731,7 +732,7 @@
             this->write(ByteCodeInstruction::kMaskPush);
             this->write(ByteCodeInstruction::kBranchIfAllFalse);
             DeferredLocation falseLocation(this);
-            this->writeExpression(*b.fRight);
+            this->writeExpression(right);
             this->write(ByteCodeInstruction::kOrB, 1);
             falseLocation.set();
             this->write(ByteCodeInstruction::kMaskPop);
@@ -741,13 +742,13 @@
         case Token::Kind::TK_SHR: {
             SkASSERT(count == 1 && (tc == SkSL::TypeCategory::kSigned ||
                                     tc == SkSL::TypeCategory::kUnsigned));
-            if (!b.fRight->isCompileTimeConstant()) {
-                fErrors.error(b.fRight->fOffset, "Shift amounts must be constant");
+            if (!right.isCompileTimeConstant()) {
+                fErrors.error(right.fOffset, "Shift amounts must be constant");
                 return false;
             }
-            int64_t shift = b.fRight->getConstantInt();
+            int64_t shift = right.getConstantInt();
             if (shift < 0 || shift > 31) {
-                fErrors.error(b.fRight->fOffset, "Shift amount out of range");
+                fErrors.error(right.fOffset, "Shift amount out of range");
                 return false;
             }
 
@@ -765,7 +766,7 @@
         default:
             break;
     }
-    this->writeExpression(*b.fRight);
+    this->writeExpression(right);
     if (lVecOrMtx && !rVecOrMtx) {
         for (int i = SlotCount(lType); i > 1; --i) {
             this->write(ByteCodeInstruction::kDup, 1);
diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index d9a9ab1..281e380 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -166,14 +166,14 @@
     switch (expr->kind()) {
         case Expression::Kind::kBinary: {
             BinaryExpression& b = expr->as<BinaryExpression>();
-            if (b.fOperator == Token::Kind::TK_EQ) {
-                if (!this->tryRemoveLValueBefore(iter, b.fLeft.get())) {
+            if (b.getOperator() == Token::Kind::TK_EQ) {
+                if (!this->tryRemoveLValueBefore(iter, &b.left())) {
                     return false;
                 }
-            } else if (!this->tryRemoveExpressionBefore(iter, b.fLeft.get())) {
+            } else if (!this->tryRemoveExpressionBefore(iter, &b.left())) {
                 return false;
             }
-            if (!this->tryRemoveExpressionBefore(iter, b.fRight.get())) {
+            if (!this->tryRemoveExpressionBefore(iter, &b.right())) {
                 return false;
             }
             SkASSERT((*iter)->expression()->get() == expr);
@@ -267,11 +267,12 @@
     switch ((*expr)->kind()) {
         case Expression::Kind::kBinary: {
             BinaryExpression& b = expr->get()->as<BinaryExpression>();
-            if (!this->tryInsertExpression(iter, &b.fRight)) {
+            if (!this->tryInsertExpression(iter, &b.rightPointer())) {
                 return false;
             }
+
             ++(*iter);
-            if (!this->tryInsertExpression(iter, &b.fLeft)) {
+            if (!this->tryInsertExpression(iter, &b.leftPointer())) {
                 return false;
             }
             ++(*iter);
@@ -319,16 +320,17 @@
     switch ((*e)->kind()) {
         case Expression::Kind::kBinary: {
             BinaryExpression& b = e->get()->as<BinaryExpression>();
-            switch (b.fOperator) {
+            Token::Kind op = b.getOperator();
+            switch (op) {
                 case Token::Kind::TK_LOGICALAND: // fall through
                 case Token::Kind::TK_LOGICALOR: {
                     // this isn't as precise as it could be -- we don't bother to track that if we
                     // early exit from a logical and/or, we know which branch of an 'if' we're going
                     // to hit -- but it won't make much difference in practice.
-                    this->addExpression(cfg, &b.fLeft, constantPropagate);
+                    this->addExpression(cfg, &b.leftPointer(), constantPropagate);
                     BlockId start = cfg.fCurrent;
                     cfg.newBlock();
-                    this->addExpression(cfg, &b.fRight, constantPropagate);
+                    this->addExpression(cfg, &b.rightPointer(), constantPropagate);
                     cfg.newBlock();
                     cfg.addExit(start, cfg.fCurrent);
                     cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
@@ -340,8 +342,8 @@
                     break;
                 }
                 case Token::Kind::TK_EQ: {
-                    this->addExpression(cfg, &b.fRight, constantPropagate);
-                    this->addLValue(cfg, &b.fLeft);
+                    this->addExpression(cfg, &b.rightPointer(), constantPropagate);
+                    this->addLValue(cfg, &b.leftPointer());
                     cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
                         BasicBlock::Node::kExpression_Kind,
                         constantPropagate,
@@ -351,8 +353,8 @@
                     break;
                 }
                 default:
-                    this->addExpression(cfg, &b.fLeft, !Compiler::IsAssignment(b.fOperator));
-                    this->addExpression(cfg, &b.fRight, constantPropagate);
+                    this->addExpression(cfg, &b.leftPointer(), !Compiler::IsAssignment(op));
+                    this->addExpression(cfg, &b.rightPointer(), constantPropagate);
                     cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
                         BasicBlock::Node::kExpression_Kind,
                         constantPropagate,
diff --git a/src/sksl/SkSLCPPCodeGenerator.cpp b/src/sksl/SkSLCPPCodeGenerator.cpp
index 3eacb80..a62374e 100644
--- a/src/sksl/SkSLCPPCodeGenerator.cpp
+++ b/src/sksl/SkSLCPPCodeGenerator.cpp
@@ -70,42 +70,45 @@
 
 void CPPCodeGenerator::writeBinaryExpression(const BinaryExpression& b,
                                              Precedence parentPrecedence) {
-    if (b.fOperator == Token::Kind::TK_PERCENT) {
+    const Expression& left = b.left();
+    const Expression& right = b.right();
+    Token::Kind op = b.getOperator();
+    if (op == Token::Kind::TK_PERCENT) {
         // need to use "%%" instead of "%" b/c the code will be inside of a printf
-        Precedence precedence = GetBinaryPrecedence(b.fOperator);
+        Precedence precedence = GetBinaryPrecedence(op);
         if (precedence >= parentPrecedence) {
             this->write("(");
         }
-        this->writeExpression(*b.fLeft, precedence);
+        this->writeExpression(left, precedence);
         this->write(" %% ");
-        this->writeExpression(*b.fRight, precedence);
+        this->writeExpression(right, precedence);
         if (precedence >= parentPrecedence) {
             this->write(")");
         }
-    } else if (b.fLeft->kind() == Expression::Kind::kNullLiteral ||
-               b.fRight->kind() == Expression::Kind::kNullLiteral) {
+    } else if (left.kind() == Expression::Kind::kNullLiteral ||
+               right.kind() == Expression::Kind::kNullLiteral) {
         const Variable* var;
-        if (b.fLeft->kind() != Expression::Kind::kNullLiteral) {
-            var = &b.fLeft->as<VariableReference>().fVariable;
+        if (left.kind() != Expression::Kind::kNullLiteral) {
+            var = &left.as<VariableReference>().fVariable;
         } else {
-            var = &b.fRight->as<VariableReference>().fVariable;
+            var = &right.as<VariableReference>().fVariable;
         }
         SkASSERT(var->type().typeKind() == Type::TypeKind::kNullable &&
                  var->type().componentType() == *fContext.fFragmentProcessor_Type);
         this->write("%s");
-        const char* op = "";
-        switch (b.fOperator) {
+        const char* prefix = "";
+        switch (op) {
             case Token::Kind::TK_EQEQ:
-                op = "!";
+                prefix = "!";
                 break;
             case Token::Kind::TK_NEQ:
-                op = "";
+                prefix = "";
                 break;
             default:
                 SkASSERT(false);
         }
         int childIndex = this->getChildFPIndex(*var);
-        fFormatArgs.push_back(String(op) + "_outer.childProcessor(" + to_string(childIndex) +
+        fFormatArgs.push_back(String(prefix) + "_outer.childProcessor(" + to_string(childIndex) +
                               ") ? \"true\" : \"false\"");
     } else {
         INHERITED::writeBinaryExpression(b, parentPrecedence);
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 0558688..6bd8f74 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -436,11 +436,11 @@
             switch (expr->kind()) {
                 case Expression::Kind::kBinary: {
                     BinaryExpression* b = &expr->as<BinaryExpression>();
-                    if (b->fOperator == Token::Kind::TK_EQ) {
-                        this->addDefinition(b->fLeft.get(), &b->fRight, definitions);
-                    } else if (Compiler::IsAssignment(b->fOperator)) {
+                    if (b->getOperator() == Token::Kind::TK_EQ) {
+                        this->addDefinition(&b->left(), &b->rightPointer(), definitions);
+                    } else if (Compiler::IsAssignment(b->getOperator())) {
                         this->addDefinition(
-                                      b->fLeft.get(),
+                                      &b->left(),
                                       (std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
                                       definitions);
 
@@ -607,10 +607,10 @@
  * to a dead target and lack of side effects on the left hand side.
  */
 static bool dead_assignment(const BinaryExpression& b) {
-    if (!Compiler::IsAssignment(b.fOperator)) {
+    if (!Compiler::IsAssignment(b.getOperator())) {
         return false;
     }
-    return is_dead(*b.fLeft);
+    return is_dead(b.left());
 }
 
 void Compiler::computeDataFlow(CFG* cfg) {
@@ -705,14 +705,16 @@
     *outUpdated = true;
     std::unique_ptr<Expression>* target = (*iter)->expression();
     BinaryExpression& bin = (*target)->as<BinaryExpression>();
-    SkASSERT(!bin.fLeft->hasSideEffects());
+    Expression& left = bin.left();
+    std::unique_ptr<Expression>& rightPointer = bin.rightPointer();
+    SkASSERT(!left.hasSideEffects());
     bool result;
-    if (bin.fOperator == Token::Kind::TK_EQ) {
-        result = b->tryRemoveLValueBefore(iter, bin.fLeft.get());
+    if (bin.getOperator() == Token::Kind::TK_EQ) {
+        result = b->tryRemoveLValueBefore(iter, &left);
     } else {
-        result = b->tryRemoveExpressionBefore(iter, bin.fLeft.get());
+        result = b->tryRemoveExpressionBefore(iter, &left);
     }
-    *target = std::move(bin.fRight);
+    *target = std::move(rightPointer);
     if (!result) {
         *outNeedsRescan = true;
         return;
@@ -723,7 +725,7 @@
     }
     --(*iter);
     if ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
-        (*iter)->expression() != &bin.fRight) {
+        (*iter)->expression() != &rightPointer) {
         *outNeedsRescan = true;
         return;
     }
@@ -742,20 +744,22 @@
     *outUpdated = true;
     std::unique_ptr<Expression>* target = (*iter)->expression();
     BinaryExpression& bin = (*target)->as<BinaryExpression>();
-    SkASSERT(!bin.fRight->hasSideEffects());
-    if (!b->tryRemoveExpressionBefore(iter, bin.fRight.get())) {
-        *target = std::move(bin.fLeft);
+    std::unique_ptr<Expression>& leftPointer = bin.leftPointer();
+    Expression& right = bin.right();
+    SkASSERT(!right.hasSideEffects());
+    if (!b->tryRemoveExpressionBefore(iter, &right)) {
+        *target = std::move(leftPointer);
         *outNeedsRescan = true;
         return;
     }
-    *target = std::move(bin.fLeft);
+    *target = std::move(leftPointer);
     if (*iter == b->fNodes.begin()) {
         *outNeedsRescan = true;
         return;
     }
     --(*iter);
     if (((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
-        (*iter)->expression() != &bin.fLeft)) {
+        (*iter)->expression() != &leftPointer)) {
         *outNeedsRescan = true;
         return;
     }
@@ -808,7 +812,7 @@
                            bool* outUpdated,
                            bool* outNeedsRescan) {
     BinaryExpression& bin = (*(*iter)->expression())->as<BinaryExpression>();
-    vectorize(b, iter, bin.fRight->type(), &bin.fLeft, outUpdated, outNeedsRescan);
+    vectorize(b, iter, bin.right().type(), &bin.leftPointer(), outUpdated, outNeedsRescan);
 }
 
 /**
@@ -820,7 +824,7 @@
                             bool* outUpdated,
                             bool* outNeedsRescan) {
     BinaryExpression& bin = (*(*iter)->expression())->as<BinaryExpression>();
-    vectorize(b, iter, bin.fLeft->type(), &bin.fRight, outUpdated, outNeedsRescan);
+    vectorize(b, iter, bin.left().type(), &bin.rightPointer(), outUpdated, outNeedsRescan);
 }
 
 // Mark that an expression which we were writing to is no longer being written to
@@ -902,8 +906,10 @@
                 delete_left(&b, iter, outUpdated, outNeedsRescan);
                 break;
             }
-            const Type& leftType = bin->fLeft->type();
-            const Type& rightType = bin->fRight->type();
+            Expression& left = bin->left();
+            Expression& right = bin->right();
+            const Type& leftType = left.type();
+            const Type& rightType = right.type();
             // collapse useless expressions like x * 1 or x + 0
             if (((leftType.typeKind() != Type::TypeKind::kScalar) &&
                  (leftType.typeKind() != Type::TypeKind::kVector)) ||
@@ -911,9 +917,9 @@
                  (rightType.typeKind() != Type::TypeKind::kVector))) {
                 break;
             }
-            switch (bin->fOperator) {
+            switch (bin->getOperator()) {
                 case Token::Kind::TK_STAR:
-                    if (is_constant(*bin->fLeft, 1)) {
+                    if (is_constant(left, 1)) {
                         if (leftType.typeKind() == Type::TypeKind::kVector &&
                             rightType.typeKind() == Type::TypeKind::kScalar) {
                             // float4(1) * x -> float4(x)
@@ -925,22 +931,22 @@
                             delete_left(&b, iter, outUpdated, outNeedsRescan);
                         }
                     }
-                    else if (is_constant(*bin->fLeft, 0)) {
+                    else if (is_constant(left, 0)) {
                         if (leftType.typeKind() == Type::TypeKind::kScalar &&
                             rightType.typeKind() == Type::TypeKind::kVector &&
-                            !bin->fRight->hasSideEffects()) {
+                            !right.hasSideEffects()) {
                             // 0 * float4(x) -> float4(0)
                             vectorize_left(&b, iter, outUpdated, outNeedsRescan);
                         } else {
                             // 0 * x -> 0
                             // float4(0) * x -> float4(0)
                             // float4(0) * float4(x) -> float4(0)
-                            if (!bin->fRight->hasSideEffects()) {
+                            if (!right.hasSideEffects()) {
                                 delete_right(&b, iter, outUpdated, outNeedsRescan);
                             }
                         }
                     }
-                    else if (is_constant(*bin->fRight, 1)) {
+                    else if (is_constant(right, 1)) {
                         if (leftType.typeKind() == Type::TypeKind::kScalar &&
                             rightType.typeKind() == Type::TypeKind::kVector) {
                             // x * float4(1) -> float4(x)
@@ -952,24 +958,24 @@
                             delete_right(&b, iter, outUpdated, outNeedsRescan);
                         }
                     }
-                    else if (is_constant(*bin->fRight, 0)) {
+                    else if (is_constant(right, 0)) {
                         if (leftType.typeKind() == Type::TypeKind::kVector &&
                             rightType.typeKind() == Type::TypeKind::kScalar &&
-                            !bin->fLeft->hasSideEffects()) {
+                            !left.hasSideEffects()) {
                             // float4(x) * 0 -> float4(0)
                             vectorize_right(&b, iter, outUpdated, outNeedsRescan);
                         } else {
                             // x * 0 -> 0
                             // x * float4(0) -> float4(0)
                             // float4(x) * float4(0) -> float4(0)
-                            if (!bin->fLeft->hasSideEffects()) {
+                            if (!left.hasSideEffects()) {
                                 delete_left(&b, iter, outUpdated, outNeedsRescan);
                             }
                         }
                     }
                     break;
                 case Token::Kind::TK_PLUS:
-                    if (is_constant(*bin->fLeft, 0)) {
+                    if (is_constant(left, 0)) {
                         if (leftType.typeKind() == Type::TypeKind::kVector &&
                             rightType.typeKind() == Type::TypeKind::kScalar) {
                             // float4(0) + x -> float4(x)
@@ -980,7 +986,7 @@
                             // float4(0) + float4(x) -> float4(x)
                             delete_left(&b, iter, outUpdated, outNeedsRescan);
                         }
-                    } else if (is_constant(*bin->fRight, 0)) {
+                    } else if (is_constant(right, 0)) {
                         if (leftType.typeKind() == Type::TypeKind::kScalar &&
                             rightType.typeKind() == Type::TypeKind::kVector) {
                             // x + float4(0) -> float4(x)
@@ -994,7 +1000,7 @@
                     }
                     break;
                 case Token::Kind::TK_MINUS:
-                    if (is_constant(*bin->fRight, 0)) {
+                    if (is_constant(right, 0)) {
                         if (leftType.typeKind() == Type::TypeKind::kScalar &&
                             rightType.typeKind() == Type::TypeKind::kVector) {
                             // x - float4(0) -> float4(x)
@@ -1008,7 +1014,7 @@
                     }
                     break;
                 case Token::Kind::TK_SLASH:
-                    if (is_constant(*bin->fRight, 1)) {
+                    if (is_constant(right, 1)) {
                         if (leftType.typeKind() == Type::TypeKind::kScalar &&
                             rightType.typeKind() == Type::TypeKind::kVector) {
                             // x / float4(1) -> float4(x)
@@ -1019,43 +1025,43 @@
                             // float4(x) / float4(1) -> float4(x)
                             delete_right(&b, iter, outUpdated, outNeedsRescan);
                         }
-                    } else if (is_constant(*bin->fLeft, 0)) {
+                    } else if (is_constant(left, 0)) {
                         if (leftType.typeKind() == Type::TypeKind::kScalar &&
                             rightType.typeKind() == Type::TypeKind::kVector &&
-                            !bin->fRight->hasSideEffects()) {
+                            !right.hasSideEffects()) {
                             // 0 / float4(x) -> float4(0)
                             vectorize_left(&b, iter, outUpdated, outNeedsRescan);
                         } else {
                             // 0 / x -> 0
                             // float4(0) / x -> float4(0)
                             // float4(0) / float4(x) -> float4(0)
-                            if (!bin->fRight->hasSideEffects()) {
+                            if (!right.hasSideEffects()) {
                                 delete_right(&b, iter, outUpdated, outNeedsRescan);
                             }
                         }
                     }
                     break;
                 case Token::Kind::TK_PLUSEQ:
-                    if (is_constant(*bin->fRight, 0)) {
-                        clear_write(*bin->fLeft);
+                    if (is_constant(right, 0)) {
+                        clear_write(left);
                         delete_right(&b, iter, outUpdated, outNeedsRescan);
                     }
                     break;
                 case Token::Kind::TK_MINUSEQ:
-                    if (is_constant(*bin->fRight, 0)) {
-                        clear_write(*bin->fLeft);
+                    if (is_constant(right, 0)) {
+                        clear_write(left);
                         delete_right(&b, iter, outUpdated, outNeedsRescan);
                     }
                     break;
                 case Token::Kind::TK_STAREQ:
-                    if (is_constant(*bin->fRight, 1)) {
-                        clear_write(*bin->fLeft);
+                    if (is_constant(right, 1)) {
+                        clear_write(left);
                         delete_right(&b, iter, outUpdated, outNeedsRescan);
                     }
                     break;
                 case Token::Kind::TK_SLASHEQ:
-                    if (is_constant(*bin->fRight, 1)) {
-                        clear_write(*bin->fLeft);
+                    if (is_constant(right, 1)) {
+                        clear_write(left);
                         delete_right(&b, iter, outUpdated, outNeedsRescan);
                     }
                     break;
diff --git a/src/sksl/SkSLDehydrator.cpp b/src/sksl/SkSLDehydrator.cpp
index 4f93647..b320343 100644
--- a/src/sksl/SkSLDehydrator.cpp
+++ b/src/sksl/SkSLDehydrator.cpp
@@ -258,9 +258,9 @@
             case Expression::Kind::kBinary: {
                 const BinaryExpression& b = e->as<BinaryExpression>();
                 this->writeU8(Rehydrator::kBinary_Command);
-                this->write(b.fLeft.get());
-                this->writeU8((int) b.fOperator);
-                this->write(b.fRight.get());
+                this->write(&b.left());
+                this->writeU8((int) b.getOperator());
+                this->write(&b.right());
                 this->write(b.type());
                 break;
             }
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 433b11b..6ef3b9b 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -913,31 +913,33 @@
 
 void GLSLCodeGenerator::writeBinaryExpression(const BinaryExpression& b,
                                               Precedence parentPrecedence) {
+    const Expression& left = b.left();
+    const Expression& right = b.right();
+    Token::Kind op = b.getOperator();
     if (fProgram.fSettings.fCaps->unfoldShortCircuitAsTernary() &&
-            (b.fOperator == Token::Kind::TK_LOGICALAND ||
-             b.fOperator == Token::Kind::TK_LOGICALOR)) {
+            (op == Token::Kind::TK_LOGICALAND || op == Token::Kind::TK_LOGICALOR)) {
         this->writeShortCircuitWorkaroundExpression(b, parentPrecedence);
         return;
     }
 
-    Precedence precedence = GetBinaryPrecedence(b.fOperator);
+    Precedence precedence = GetBinaryPrecedence(op);
     if (precedence >= parentPrecedence) {
         this->write("(");
     }
     bool positionWorkaround = fProgramKind == Program::Kind::kVertex_Kind &&
-                              Compiler::IsAssignment(b.fOperator) &&
-                              b.fLeft->kind() == Expression::Kind::kFieldAccess &&
-                              is_sk_position((FieldAccess&) *b.fLeft) &&
-                              !b.fRight->containsRTAdjust() &&
+                              Compiler::IsAssignment(op) &&
+                              left.kind() == Expression::Kind::kFieldAccess &&
+                              is_sk_position((FieldAccess&) left) &&
+                              !right.containsRTAdjust() &&
                               !fProgram.fSettings.fCaps->canUseFragCoord();
     if (positionWorkaround) {
         this->write("sk_FragCoord_Workaround = (");
     }
-    this->writeExpression(*b.fLeft, precedence);
+    this->writeExpression(left, precedence);
     this->write(" ");
-    this->write(Compiler::OperatorName(b.fOperator));
+    this->write(Compiler::OperatorName(op));
     this->write(" ");
-    this->writeExpression(*b.fRight, precedence);
+    this->writeExpression(right, precedence);
     if (positionWorkaround) {
         this->write(")");
     }
@@ -955,20 +957,20 @@
     // Transform:
     // a && b  =>   a ? b : false
     // a || b  =>   a ? true : b
-    this->writeExpression(*b.fLeft, kTernary_Precedence);
+    this->writeExpression(b.left(), kTernary_Precedence);
     this->write(" ? ");
-    if (b.fOperator == Token::Kind::TK_LOGICALAND) {
-        this->writeExpression(*b.fRight, kTernary_Precedence);
+    if (b.getOperator() == Token::Kind::TK_LOGICALAND) {
+        this->writeExpression(b.right(), kTernary_Precedence);
     } else {
         BoolLiteral boolTrue(fContext, -1, true);
         this->writeBoolLiteral(boolTrue);
     }
     this->write(" : ");
-    if (b.fOperator == Token::Kind::TK_LOGICALAND) {
+    if (b.getOperator() == Token::Kind::TK_LOGICALAND) {
         BoolLiteral boolFalse(fContext, -1, false);
         this->writeBoolLiteral(boolFalse);
     } else {
-        this->writeExpression(*b.fRight, kTernary_Precedence);
+        this->writeExpression(b.right(), kTernary_Precedence);
     }
     if (kTernary_Precedence >= parentPrecedence) {
         this->write(")");
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index 5ede14d..4761329 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -308,9 +308,9 @@
         case Expression::Kind::kBinary: {
             const BinaryExpression& b = expression.as<BinaryExpression>();
             return std::make_unique<BinaryExpression>(offset,
-                                                      expr(b.fLeft),
-                                                      b.fOperator,
-                                                      expr(b.fRight),
+                                                      expr(b.leftPointer()),
+                                                      b.getOperator(),
+                                                      expr(b.rightPointer()),
                                                       &b.type());
         }
         case Expression::Kind::kBoolLiteral:
@@ -938,7 +938,7 @@
 
                 case Expression::Kind::kBinary: {
                     BinaryExpression& binaryExpr = (*expr)->as<BinaryExpression>();
-                    this->visitExpression(&binaryExpr.fLeft);
+                    this->visitExpression(&binaryExpr.leftPointer());
 
                     // Logical-and and logical-or binary expressions do not inline the right side,
                     // because that would invalidate short-circuiting. That is, when evaluating
@@ -948,10 +948,11 @@
                     // It is illegal for side-effects from x() or y() to occur. The simplest way to
                     // enforce that rule is to avoid inlining the right side entirely. However, it
                     // is safe for other types of binary expression to inline both sides.
-                    bool shortCircuitable = (binaryExpr.fOperator == Token::Kind::TK_LOGICALAND ||
-                                             binaryExpr.fOperator == Token::Kind::TK_LOGICALOR);
+                    Token::Kind op = binaryExpr.getOperator();
+                    bool shortCircuitable = (op == Token::Kind::TK_LOGICALAND ||
+                                             op == Token::Kind::TK_LOGICALOR);
                     if (!shortCircuitable) {
-                        this->visitExpression(&binaryExpr.fRight);
+                        this->visitExpression(&binaryExpr.rightPointer());
                     }
                     break;
                 }
diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp
index d573c38..1ee05a4 100644
--- a/src/sksl/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -808,11 +808,14 @@
 
 void MetalCodeGenerator::writeBinaryExpression(const BinaryExpression& b,
                                                Precedence parentPrecedence) {
-    const Type& leftType = b.fLeft->type();
-    const Type& rightType = b.fRight->type();
-    Precedence precedence = GetBinaryPrecedence(b.fOperator);
+    const Expression& left = b.left();
+    const Expression& right = b.right();
+    const Type& leftType = left.type();
+    const Type& rightType = right.type();
+    Token::Kind op = b.getOperator();
+    Precedence precedence = GetBinaryPrecedence(b.getOperator());
     bool needParens = precedence >= parentPrecedence;
-    switch (b.fOperator) {
+    switch (op) {
         case Token::Kind::TK_EQEQ:
             if (leftType.typeKind() == Type::TypeKind::kVector) {
                 this->write("all");
@@ -831,22 +834,21 @@
     if (needParens) {
         this->write("(");
     }
-    if (Compiler::IsAssignment(b.fOperator) &&
-        Expression::Kind::kVariableReference == b.fLeft->kind() &&
-        Variable::kParameter_Storage == ((VariableReference&) *b.fLeft).fVariable.fStorage &&
-        (((VariableReference&) *b.fLeft).fVariable.fModifiers.fFlags & Modifiers::kOut_Flag)) {
+    if (Compiler::IsAssignment(op) &&
+        Expression::Kind::kVariableReference == left.kind() &&
+        Variable::kParameter_Storage == left.as<VariableReference>().fVariable.fStorage &&
+        left.as<VariableReference>().fVariable.fModifiers.fFlags & Modifiers::kOut_Flag) {
         // writing to an out parameter. Since we have to turn those into pointers, we have to
         // dereference it here.
         this->write("*");
     }
-    if (b.fOperator == Token::Kind::TK_STAREQ &&
-        leftType.typeKind() == Type::TypeKind::kMatrix &&
+    if (op == Token::Kind::TK_STAREQ && leftType.typeKind() == Type::TypeKind::kMatrix &&
         rightType.typeKind() == Type::TypeKind::kMatrix) {
         this->writeMatrixTimesEqualHelper(leftType, rightType, b.type());
     }
-    this->writeExpression(*b.fLeft, precedence);
-    if (b.fOperator != Token::Kind::TK_EQ && Compiler::IsAssignment(b.fOperator) &&
-        b.fLeft->kind() == Expression::Kind::kSwizzle && !b.fLeft->hasSideEffects()) {
+    this->writeExpression(left, precedence);
+    if (op != Token::Kind::TK_EQ && Compiler::IsAssignment(op) &&
+        left.kind() == Expression::Kind::kSwizzle && !left.hasSideEffects()) {
         // This doesn't compile in Metal:
         // float4 x = float4(1);
         // x.xy *= float2x2(...);
@@ -854,16 +856,16 @@
         // but switching it to x.xy = x.xy * float2x2(...) fixes it. We perform this tranformation
         // as long as the LHS has no side effects, and hope for the best otherwise.
         this->write(" = ");
-        this->writeExpression(*b.fLeft, kAssignment_Precedence);
+        this->writeExpression(left, kAssignment_Precedence);
         this->write(" ");
-        String op = Compiler::OperatorName(b.fOperator);
-        SkASSERT(op.endsWith("="));
-        this->write(op.substr(0, op.size() - 1).c_str());
+        String opName = Compiler::OperatorName(op);
+        SkASSERT(opName.endsWith("="));
+        this->write(opName.substr(0, opName.size() - 1).c_str());
         this->write(" ");
     } else {
-        this->write(String(" ") + Compiler::OperatorName(b.fOperator) + " ");
+        this->write(String(" ") + Compiler::OperatorName(op) + " ");
     }
-    this->writeExpression(*b.fRight, precedence);
+    this->writeExpression(right, precedence);
     if (needParens) {
         this->write(")");
     }
@@ -1730,8 +1732,8 @@
             return this->requirements(e->as<Swizzle>().fBase.get());
         case Expression::Kind::kBinary: {
             const BinaryExpression& bin = e->as<BinaryExpression>();
-            return this->requirements(bin.fLeft.get()) |
-                   this->requirements(bin.fRight.get());
+            return this->requirements(&bin.left()) |
+                   this->requirements(&bin.right());
         }
         case Expression::Kind::kIndex: {
             const IndexExpression& idx = e->as<IndexExpression>();
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index ab3d709..6e07dfb 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2302,11 +2302,14 @@
 }
 
 SpvId SPIRVCodeGenerator::writeBinaryExpression(const BinaryExpression& b, OutputStream& out) {
+    const Expression& left = b.left();
+    const Expression& right = b.right();
+    Token::Kind op = b.getOperator();
     // handle cases where we don't necessarily evaluate both LHS and RHS
-    switch (b.fOperator) {
+    switch (op) {
         case Token::Kind::TK_EQ: {
-            SpvId rhs = this->writeExpression(*b.fRight, out);
-            this->getLValue(*b.fLeft, out)->store(rhs, out);
+            SpvId rhs = this->writeExpression(right, out);
+            this->getLValue(left, out)->store(rhs, out);
             return rhs;
         }
         case Token::Kind::TK_LOGICALAND:
@@ -2319,17 +2322,16 @@
 
     std::unique_ptr<LValue> lvalue;
     SpvId lhs;
-    if (Compiler::IsAssignment(b.fOperator)) {
-        lvalue = this->getLValue(*b.fLeft, out);
+    if (Compiler::IsAssignment(op)) {
+        lvalue = this->getLValue(left, out);
         lhs = lvalue->load(out);
     } else {
         lvalue = nullptr;
-        lhs = this->writeExpression(*b.fLeft, out);
+        lhs = this->writeExpression(left, out);
     }
-    SpvId rhs = this->writeExpression(*b.fRight, out);
-    SpvId result = this->writeBinaryExpression(b.fLeft->type(), lhs,
-                                               Compiler::RemoveAssignment(b.fOperator),
-                                               b.fRight->type(), rhs, b.type(), out);
+    SpvId rhs = this->writeExpression(right, out);
+    SpvId result = this->writeBinaryExpression(left.type(), lhs, Compiler::RemoveAssignment(op),
+                                               right.type(), rhs, b.type(), out);
     if (lvalue) {
         lvalue->store(result, out);
     }
@@ -2337,17 +2339,17 @@
 }
 
 SpvId SPIRVCodeGenerator::writeLogicalAnd(const BinaryExpression& a, OutputStream& out) {
-    SkASSERT(a.fOperator == Token::Kind::TK_LOGICALAND);
+    SkASSERT(a.getOperator() == Token::Kind::TK_LOGICALAND);
     BoolLiteral falseLiteral(fContext, -1, false);
     SpvId falseConstant = this->writeBoolLiteral(falseLiteral);
-    SpvId lhs = this->writeExpression(*a.fLeft, out);
+    SpvId lhs = this->writeExpression(a.left(), out);
     SpvId rhsLabel = this->nextId();
     SpvId end = this->nextId();
     SpvId lhsBlock = fCurrentBlock;
     this->writeInstruction(SpvOpSelectionMerge, end, SpvSelectionControlMaskNone, out);
     this->writeInstruction(SpvOpBranchConditional, lhs, rhsLabel, end, out);
     this->writeLabel(rhsLabel, out);
-    SpvId rhs = this->writeExpression(*a.fRight, out);
+    SpvId rhs = this->writeExpression(a.right(), out);
     SpvId rhsBlock = fCurrentBlock;
     this->writeInstruction(SpvOpBranch, end, out);
     this->writeLabel(end, out);
@@ -2358,17 +2360,17 @@
 }
 
 SpvId SPIRVCodeGenerator::writeLogicalOr(const BinaryExpression& o, OutputStream& out) {
-    SkASSERT(o.fOperator == Token::Kind::TK_LOGICALOR);
+    SkASSERT(o.getOperator() == Token::Kind::TK_LOGICALOR);
     BoolLiteral trueLiteral(fContext, -1, true);
     SpvId trueConstant = this->writeBoolLiteral(trueLiteral);
-    SpvId lhs = this->writeExpression(*o.fLeft, out);
+    SpvId lhs = this->writeExpression(o.left(), out);
     SpvId rhsLabel = this->nextId();
     SpvId end = this->nextId();
     SpvId lhsBlock = fCurrentBlock;
     this->writeInstruction(SpvOpSelectionMerge, end, SpvSelectionControlMaskNone, out);
     this->writeInstruction(SpvOpBranchConditional, lhs, end, rhsLabel, out);
     this->writeLabel(rhsLabel, out);
-    SpvId rhs = this->writeExpression(*o.fRight, out);
+    SpvId rhs = this->writeExpression(o.right(), out);
     SpvId rhsBlock = fCurrentBlock;
     this->writeInstruction(SpvOpBranch, end, out);
     this->writeLabel(end, out);
diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h
index 18ce2b7..47ccd7a 100644
--- a/src/sksl/ir/SkSLBinaryExpression.h
+++ b/src/sksl/ir/SkSLBinaryExpression.h
@@ -19,24 +19,24 @@
 
 namespace SkSL {
 
-static inline bool check_ref(Expression* expr) {
-    switch (expr->kind()) {
+static inline bool check_ref(const Expression& expr) {
+    switch (expr.kind()) {
         case Expression::Kind::kExternalValue:
             return true;
         case Expression::Kind::kFieldAccess:
-            return check_ref(((FieldAccess*) expr)->fBase.get());
+            return check_ref(*expr.as<FieldAccess>().fBase);
         case Expression::Kind::kIndex:
-            return check_ref(((IndexExpression*) expr)->fBase.get());
+            return check_ref(*expr.as<IndexExpression>().fBase);
         case Expression::Kind::kSwizzle:
-            return check_ref(((Swizzle*) expr)->fBase.get());
+            return check_ref(*expr.as<Swizzle>().fBase);
         case Expression::Kind::kTernary: {
-            TernaryExpression* t = (TernaryExpression*) expr;
-            return check_ref(t->fIfTrue.get()) && check_ref(t->fIfFalse.get());
+            const TernaryExpression& t = expr.as<TernaryExpression>();
+            return check_ref(*t.fIfTrue) && check_ref(*t.fIfFalse);
         }
         case Expression::Kind::kVariableReference: {
-            VariableReference* ref = (VariableReference*) expr;
-            return ref->fRefKind == VariableReference::kWrite_RefKind ||
-                   ref->fRefKind == VariableReference::kReadWrite_RefKind;
+            const VariableReference& ref = expr.as<VariableReference>();
+            return ref.fRefKind == VariableReference::kWrite_RefKind ||
+                   ref.fRefKind == VariableReference::kReadWrite_RefKind;
         }
         default:
             return false;
@@ -51,46 +51,75 @@
 
     BinaryExpression(int offset, std::unique_ptr<Expression> left, Token::Kind op,
                      std::unique_ptr<Expression> right, const Type* type)
-    : INHERITED(offset, kExpressionKind, type)
-    , fLeft(std::move(left))
-    , fOperator(op)
-    , fRight(std::move(right)) {
+    : INHERITED(offset, kExpressionKind, { type, op }) {
+        fExpressionChildren.reserve(2);
+        fExpressionChildren.push_back(std::move(left));
+        fExpressionChildren.push_back(std::move(right));
         // If we are assigning to a VariableReference, ensure that it is set to Write or ReadWrite
-        SkASSERT(!Compiler::IsAssignment(op) || check_ref(fLeft.get()));
+        SkASSERT(!Compiler::IsAssignment(op) || check_ref(this->left()));
+    }
+
+    Expression& left() const {
+        return this->expressionChild(0);
+    }
+
+    std::unique_ptr<Expression>& leftPointer() {
+        return this->expressionPointer(0);
+    }
+
+    const std::unique_ptr<Expression>& leftPointer() const {
+        return this->expressionPointer(0);
+    }
+
+    Expression& right() const {
+        return this->expressionChild(1);
+    }
+
+    std::unique_ptr<Expression>& rightPointer() {
+        return this->expressionPointer(1);
+    }
+
+    const std::unique_ptr<Expression>& rightPointer() const {
+        return this->expressionPointer(1);
+    }
+
+    Token::Kind getOperator() const {
+        return this->typeTokenData().fToken;
     }
 
     bool isConstantOrUniform() const override {
-        return fLeft->isConstantOrUniform() && fRight->isConstantOrUniform();
+        return this->left().isConstantOrUniform() && this->right().isConstantOrUniform();
     }
 
     std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
                                                   const DefinitionMap& definitions) override {
-        return irGenerator.constantFold(*fLeft,
-                                        fOperator,
-                                        *fRight);
+        return irGenerator.constantFold(this->left(),
+                                        this->getOperator(),
+                                        this->right());
     }
 
     bool hasProperty(Property property) const override {
-        if (property == Property::kSideEffects && Compiler::IsAssignment(fOperator)) {
+        if (property == Property::kSideEffects && Compiler::IsAssignment(this->getOperator())) {
             return true;
         }
-        return fLeft->hasProperty(property) || fRight->hasProperty(property);
+        return this->left().hasProperty(property) || this->right().hasProperty(property);
     }
 
     std::unique_ptr<Expression> clone() const override {
-        return std::unique_ptr<Expression>(new BinaryExpression(fOffset, fLeft->clone(), fOperator,
-                                                                fRight->clone(), &this->type()));
+        return std::unique_ptr<Expression>(new BinaryExpression(fOffset,
+                                                                this->left().clone(),
+                                                                this->getOperator(),
+                                                                this->right().clone(),
+                                                                &this->type()));
     }
 
     String description() const override {
-        return "(" + fLeft->description() + " " + Compiler::OperatorName(fOperator) + " " +
-               fRight->description() + ")";
+        return "(" + this->left().description() + " " +
+               Compiler::OperatorName(this->getOperator()) + " " + this->right().description() +
+               ")";
     }
 
-    std::unique_ptr<Expression> fLeft;
-    const Token::Kind fOperator;
-    std::unique_ptr<Expression> fRight;
-
+private:
     using INHERITED = Expression;
 };
 
diff --git a/src/sksl/ir/SkSLExpression.h b/src/sksl/ir/SkSLExpression.h
index 7beb341..14c9708 100644
--- a/src/sksl/ir/SkSLExpression.h
+++ b/src/sksl/ir/SkSLExpression.h
@@ -57,7 +57,12 @@
     };
 
     Expression(int offset, Kind kind, const Type* type)
-    : INHERITED(offset, (int) kind, type) {
+        : INHERITED(offset, (int) kind, type) {
+        SkASSERT(kind >= Kind::kFirst && kind <= Kind::kLast);
+    }
+
+    Expression(int offset, Kind kind, TypeTokenData data)
+        : INHERITED(offset, (int) kind, data) {
         SkASSERT(kind >= Kind::kFirst && kind <= Kind::kLast);
     }
 
diff --git a/src/sksl/ir/SkSLIRNode.cpp b/src/sksl/ir/SkSLIRNode.cpp
new file mode 100644
index 0000000..19eaf8b
--- /dev/null
+++ b/src/sksl/ir/SkSLIRNode.cpp
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2020 Google LLC.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/sksl/ir/SkSLIRNode.h"
+
+#include "src/sksl/ir/SkSLExpression.h"
+
+namespace SkSL {
+
+IRNode::IRNode(int offset, int kind, const Type* data)
+: fOffset(offset)
+, fKind(kind)
+, fData(data) {}
+
+IRNode::IRNode(int offset, int kind, TypeTokenData data)
+: fOffset(offset)
+, fKind(kind)
+, fData(data) {}
+
+IRNode::IRNode(const IRNode& other)
+    : fOffset(other.fOffset)
+    , fKind(other.fKind)
+    , fData(other.fData) {
+    // For now, we can't use a default copy constructor because of the std::unique_ptr children.
+    // Since we never copy nodes containing children, it's easiest just to assert we don't have any
+    // than bother with cloning them.
+    SkASSERT(other.fExpressionChildren.empty());
+}
+
+IRNode::~IRNode() {}
+
+} // namespace SkSL
diff --git a/src/sksl/ir/SkSLIRNode.h b/src/sksl/ir/SkSLIRNode.h
index 4a6aa90..d50afbe 100644
--- a/src/sksl/ir/SkSLIRNode.h
+++ b/src/sksl/ir/SkSLIRNode.h
@@ -8,11 +8,15 @@
 #ifndef SKSL_IRNODE
 #define SKSL_IRNODE
 
+#include "src/sksl/SkSLASTNode.h"
 #include "src/sksl/SkSLLexer.h"
 #include "src/sksl/SkSLString.h"
 
+#include <vector>
+
 namespace SkSL {
 
+struct Expression;
 class Type;
 
 /**
@@ -21,12 +25,19 @@
  */
 class IRNode {
 public:
-    IRNode(int offset, int kind, const Type* type = nullptr)
-    : fOffset(offset)
-    , fKind(kind)
-    , fType(type) {}
+    virtual ~IRNode();
 
-    virtual ~IRNode() {}
+    IRNode& operator=(const IRNode& other) {
+        // Need to have a copy assignment operator because Type requires it, but can't use the
+        // default version until we finish migrating away from std::unique_ptr children. For now,
+        // just assert that there are no children (we could theoretically clone them, but we never
+        // actually copy nodes containing children).
+        SkASSERT(other.fExpressionChildren.empty());
+        fKind = other.fKind;
+        fOffset = other.fOffset;
+        fData = other.fData;
+        return *this;
+    }
 
     virtual String description() const = 0;
 
@@ -35,15 +46,81 @@
     int fOffset;
 
     const Type& type() const {
-        SkASSERT(fType);
-        return *fType;
+        switch (fData.fKind) {
+            case NodeData::Kind::kType:
+                return *this->typeData();
+            case NodeData::Kind::kTypeToken:
+                return *this->typeTokenData().fType;
+            default:
+                SkUNREACHABLE;
+        }
     }
 
 protected:
+    struct TypeTokenData {
+        const Type* fType;
+        Token::Kind fToken;
+    };
+
+    struct NodeData {
+        char fBytes[std::max(sizeof(Type*),
+                             sizeof(TypeTokenData))];
+
+        enum class Kind {
+            kType,
+            kTypeToken,
+        } fKind;
+
+        NodeData() = default;
+
+        NodeData(const Type* data)
+            : fKind(Kind::kType) {
+            memcpy(fBytes, &data, sizeof(data));
+        }
+
+        NodeData(TypeTokenData data)
+            : fKind(Kind::kTypeToken) {
+            memcpy(fBytes, &data, sizeof(data));
+        }
+    };
+
+    IRNode(int offset, int kind, const Type* data = nullptr);
+
+    IRNode(int offset, int kind, TypeTokenData data);
+
+    IRNode(const IRNode& other);
+
+    Expression& expressionChild(int index) const {
+        return *fExpressionChildren[index];
+    }
+
+    std::unique_ptr<Expression>& expressionPointer(int index) {
+        return fExpressionChildren[index];
+    }
+
+    const std::unique_ptr<Expression>& expressionPointer(int index) const {
+        return fExpressionChildren[index];
+    }
+
+    Type* typeData() const {
+        SkASSERT(fData.fKind == NodeData::Kind::kType);
+        Type* result;
+        memcpy(&result, fData.fBytes, sizeof(result));
+        return result;
+    }
+
+    TypeTokenData typeTokenData() const {
+        SkASSERT(fData.fKind == NodeData::Kind::kTypeToken);
+        TypeTokenData result;
+        memcpy(&result, fData.fBytes, sizeof(result));
+        return result;
+    }
+
     int fKind;
+    std::vector<std::unique_ptr<Expression>> fExpressionChildren;
 
 private:
-    const Type* fType;
+    NodeData fData;
 };
 
 }  // namespace SkSL
diff --git a/tests/GrThreadSafeViewCacheTest.cpp b/tests/GrThreadSafeViewCacheTest.cpp
new file mode 100644
index 0000000..934e18c
--- /dev/null
+++ b/tests/GrThreadSafeViewCacheTest.cpp
@@ -0,0 +1,339 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/core/SkCanvas.h"
+#include "include/core/SkDeferredDisplayListRecorder.h"
+#include "include/core/SkSurfaceCharacterization.h"
+#include "src/gpu/GrContextPriv.h"
+#include "src/gpu/GrProxyProvider.h"
+#include "src/gpu/GrRecordingContextPriv.h"
+#include "src/gpu/GrRenderTargetContext.h"
+#include "src/gpu/GrThreadSafeUniquelyKeyedProxyViewCache.h"
+#include "tests/Test.h"
+#include "tests/TestUtils.h"
+
+static constexpr int kImageWH = 32;
+static constexpr auto kImageOrigin = kBottomLeft_GrSurfaceOrigin;
+
+static SkImageInfo default_ii(int wh) {
+    return SkImageInfo::Make(wh, wh, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+}
+
+static void create_key(GrUniqueKey* key, int wh) {
+    static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain();
+    GrUniqueKey::Builder builder(key, kDomain, 1);
+    builder[0] = wh;
+    builder.finish();
+};
+
+SkBitmap create_up_arrow_bitmap(int wh) {
+    SkBitmap bitmap;
+
+    bitmap.allocPixels(default_ii(wh));
+
+    SkCanvas tmp(bitmap);
+    tmp.clear(SK_ColorWHITE);
+
+    SkPaint blue;
+    blue.setColor(SK_ColorBLUE);
+    blue.setAntiAlias(true);
+
+    float halfW = wh / 2.0f;
+    float halfH = wh / 2.0f;
+    float thirdW = wh / 3.0f;
+
+    SkPath path;
+    path.moveTo(0, halfH);
+    path.lineTo(thirdW, halfH);
+    path.lineTo(thirdW, wh);
+    path.lineTo(2*thirdW, wh);
+    path.lineTo(2*thirdW, halfH);
+    path.lineTo(wh, halfH);
+    path.lineTo(halfW, 0);
+    path.close();
+
+    tmp.drawPath(path, blue);
+
+    return bitmap;
+}
+
+class TestHelper {
+public:
+    struct Stats {
+        int fCacheHits = 0;
+        int fCacheMisses = 0;
+
+        int fNumSWCreations = 0;
+        int fNumHWCreations = 0;
+    };
+
+    TestHelper(GrDirectContext* dContext) : fDContext(dContext) {
+
+        fDst = SkSurface::MakeRenderTarget(dContext, SkBudgeted::kNo, default_ii(kImageWH));
+        SkAssertResult(fDst);
+
+        SkSurfaceCharacterization characterization;
+        SkAssertResult(fDst->characterize(&characterization));
+
+        fRecorder1 = std::make_unique<SkDeferredDisplayListRecorder>(characterization);
+
+        fRecorder2 = std::make_unique<SkDeferredDisplayListRecorder>(characterization);
+
+        SkBitmap tmp = create_up_arrow_bitmap(kImageWH);
+        SkAssertResult(CreateBackendTexture(fDContext, &fBETex, tmp));
+
+        fThreadSafeViewCache = std::make_unique<GrThreadSafeUniquelyKeyedProxyViewCache>();
+    }
+
+    ~TestHelper() {
+        DeleteBackendTexture(fDContext, fBETex);
+    }
+
+    Stats* stats() { return &fStats; }
+
+    int numCacheEntries() const {
+        return fThreadSafeViewCache->numEntries();
+    }
+
+    GrDirectContext* dContext() { return fDContext; }
+
+    SkCanvas* liveCanvas() { return fDst ? fDst->getCanvas() : nullptr; }
+    SkCanvas* ddlCanvas1() { return fRecorder1 ? fRecorder1->getCanvas() : nullptr; }
+    SkCanvas* ddlCanvas2() { return fRecorder2 ? fRecorder2->getCanvas() : nullptr; }
+
+    GrThreadSafeUniquelyKeyedProxyViewCache* threadSafeViewCache() {
+        return fThreadSafeViewCache.get();
+    }
+
+    // Add a draw on 'canvas' that will introduce a ref on the 'wh' view
+    void accessCachedView(SkCanvas* canvas,
+                          int wh,
+                          bool failLookup = false) {
+        GrRecordingContext* rContext = canvas->recordingContext();
+
+        auto view = AccessCachedView(rContext, this->threadSafeViewCache(),
+                                     fBETex, wh, failLookup, &fStats);
+        SkASSERT(view);
+
+        auto rtc = canvas->internal_private_accessTopLayerRenderTargetContext();
+
+        rtc->drawTexture(nullptr,
+                         view,
+                         kPremul_SkAlphaType,
+                         GrSamplerState::Filter::kNearest,
+                         GrSamplerState::MipmapMode::kNone,
+                         SkBlendMode::kSrcOver,
+                         SkPMColor4f(),
+                         SkRect::MakeWH(wh, wh),
+                         SkRect::MakeWH(wh, wh),
+                         GrAA::kNo,
+                         GrQuadAAFlags::kNone,
+                         SkCanvas::kFast_SrcRectConstraint,
+                         SkMatrix::I(),
+                         nullptr);
+    }
+
+    bool checkView(SkCanvas* canvas, int wh, int hits, int misses, int numRefs) {
+        if (fStats.fCacheHits != hits || fStats.fCacheMisses != misses) {
+            return false;
+        }
+
+        GrUniqueKey key;
+        create_key(&key, wh);
+
+        GrRecordingContext* rContext = canvas->recordingContext();
+        auto threadSafeViewCache = this->threadSafeViewCache();
+
+        GrSurfaceProxyView view = threadSafeViewCache->find(key);
+
+        if (!view.proxy()->refCntGreaterThan(numRefs+1) ||  // +1 for 'view's ref
+            view.proxy()->refCntGreaterThan(numRefs+2)) {
+            return false;
+        }
+
+        {
+            GrProxyProvider* recordingProxyProvider = rContext->priv().proxyProvider();
+            sk_sp<GrTextureProxy> result = recordingProxyProvider->findProxyByUniqueKey(key);
+            if (result) {
+                // views in this cache should never appear in the recorder's cache
+                return false;
+            }
+        }
+
+        {
+            GrProxyProvider* directProxyProvider = fDContext->priv().proxyProvider();
+            sk_sp<GrTextureProxy> result = directProxyProvider->findProxyByUniqueKey(key);
+            if (result) {
+                // views in this cache should never appear in the main proxy cache
+                return false;
+            }
+        }
+
+        {
+            auto resourceProvider = fDContext->priv().resourceProvider();
+            sk_sp<GrSurface> surf = resourceProvider->findByUniqueKey<GrSurface>(key);
+            if (surf) {
+                // the textures backing the views in this cache should never be discoverable in the
+                // resource cache
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+private:
+    static GrSurfaceProxyView AccessCachedView(GrRecordingContext*,
+                                               GrThreadSafeUniquelyKeyedProxyViewCache*,
+                                               GrBackendTexture, int wh,
+                                               bool failLookup, Stats*);
+    static GrSurfaceProxyView CreateViewOnCpu(GrRecordingContext*, int wh, Stats*);
+    static GrSurfaceProxyView CreateViewOnGpu(GrDirectContext*, GrBackendTexture, int wh, Stats*);
+
+    Stats fStats;
+    GrDirectContext* fDContext = nullptr;
+
+    std::unique_ptr<GrThreadSafeUniquelyKeyedProxyViewCache> fThreadSafeViewCache;
+
+    sk_sp<SkSurface> fDst;
+    std::unique_ptr<SkDeferredDisplayListRecorder> fRecorder1;
+    std::unique_ptr<SkDeferredDisplayListRecorder> fRecorder2;
+
+    GrBackendTexture fBETex;
+};
+
+GrSurfaceProxyView TestHelper::CreateViewOnCpu(GrRecordingContext* rContext,
+                                               int wh,
+                                               Stats* stats) {
+    GrProxyProvider* proxyProvider = rContext->priv().proxyProvider();
+
+    sk_sp<GrTextureProxy> proxy = proxyProvider->createProxyFromBitmap(create_up_arrow_bitmap(wh),
+                                                                       GrMipmapped::kNo,
+                                                                       SkBackingFit::kExact,
+                                                                       SkBudgeted::kYes);
+    if (!proxy) {
+        return {};
+    }
+
+    GrSwizzle swizzle = rContext->priv().caps()->getReadSwizzle(proxy->backendFormat(),
+                                                                GrColorType::kRGBA_8888);
+    ++stats->fNumSWCreations;
+    return {std::move(proxy), kImageOrigin, swizzle};
+}
+
+GrSurfaceProxyView TestHelper::CreateViewOnGpu(GrDirectContext* dContext,
+                                               GrBackendTexture beTex,
+                                               int wh,
+                                               Stats* stats) {
+    SkASSERT(!stats->fNumHWCreations); // we had better not do this more than once
+
+    GrProxyProvider* proxyProvider = dContext->priv().proxyProvider();
+
+    sk_sp<GrTextureProxy> proxy = proxyProvider->wrapBackendTexture(beTex,
+                                                                    kBorrow_GrWrapOwnership,
+                                                                    GrWrapCacheable::kNo,
+                                                                    kRead_GrIOType);
+    GrSwizzle swizzle = dContext->priv().caps()->getReadSwizzle(proxy->backendFormat(),
+                                                                GrColorType::kRGBA_8888);
+    ++stats->fNumHWCreations;
+    return {std::move(proxy), kImageOrigin, swizzle};
+}
+
+// TODO: this doesn't actually implement the correct behavior for the gpu-thread. It needs to
+// add a view to the cache and then queue up the calls to draw the content.
+GrSurfaceProxyView TestHelper::AccessCachedView(
+                                    GrRecordingContext* rContext,
+                                    GrThreadSafeUniquelyKeyedProxyViewCache* threadSafeViewCache,
+                                    GrBackendTexture beTex,
+                                    int wh,
+                                    bool failLookup,
+                                    Stats* stats) {
+    GrUniqueKey key;
+    create_key(&key, wh);
+
+    // We can "fail the lookup" to simulate a threaded race condition
+    if (auto view = threadSafeViewCache->find(key); !failLookup && view) {
+        ++stats->fCacheHits;
+        return view;
+    }
+
+    ++stats->fCacheMisses;
+
+    GrSurfaceProxyView view;
+    if (GrDirectContext* dContext = rContext->asDirectContext()) {
+        view = CreateViewOnGpu(dContext, beTex, wh, stats);
+    } else {
+        view = CreateViewOnCpu(rContext, wh, stats);
+    }
+    SkASSERT(view);
+
+    return threadSafeViewCache->add(key, view);
+}
+
+// Case 1: ensure two DDL recorders share the view
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GrThreadSafeViewCache1, reporter, ctxInfo) {
+    TestHelper helper(ctxInfo.directContext());
+
+    helper.accessCachedView(helper.ddlCanvas1(), kImageWH);
+    helper.checkView(helper.ddlCanvas1(), kImageWH, /*hits*/ 0, /*misses*/ 1, /*refs*/ 1);
+
+    helper.accessCachedView(helper.ddlCanvas2(), kImageWH);
+    helper.checkView(helper.ddlCanvas2(), kImageWH, /*hits*/ 1, /*misses*/ 1, /*refs*/ 2);
+
+    REPORTER_ASSERT(reporter, helper.numCacheEntries() == 1);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumHWCreations == 0);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumSWCreations == 1);
+}
+
+// Case 2: ensure that, if the direct context version wins, it is reused by the DDL recorders
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GrThreadSafeViewCache2, reporter, ctxInfo) {
+    TestHelper helper(ctxInfo.directContext());
+
+    helper.accessCachedView(helper.liveCanvas(), kImageWH);
+    helper.checkView(helper.liveCanvas(), kImageWH, /*hits*/ 0, /*misses*/ 1, /*refs*/ 1);
+
+    helper.accessCachedView(helper.ddlCanvas1(), kImageWH);
+    helper.checkView(helper.ddlCanvas1(), kImageWH, /*hits*/ 1, /*misses*/ 1, /*refs*/ 2);
+
+    helper.accessCachedView(helper.ddlCanvas2(), kImageWH);
+    helper.checkView(helper.ddlCanvas2(), kImageWH, /*hits*/ 2, /*misses*/ 1, /*refs*/ 3);
+
+    REPORTER_ASSERT(reporter, helper.numCacheEntries() == 1);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumHWCreations == 1);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumSWCreations == 0);
+}
+
+// Case 3: ensure that, if the cpu-version wins, it is reused by the direct context
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GrThreadSafeViewCache3, reporter, ctxInfo) {
+    TestHelper helper(ctxInfo.directContext());
+
+    helper.accessCachedView(helper.ddlCanvas1(), kImageWH);
+    helper.checkView(helper.ddlCanvas1(), kImageWH, /*hits*/ 0, /*misses*/ 1, /*refs*/ 1);
+
+    helper.accessCachedView(helper.liveCanvas(), kImageWH);
+    helper.checkView(helper.liveCanvas(), kImageWH, /*hits*/ 1, /*misses*/ 1, /*refs*/ 2);
+
+    REPORTER_ASSERT(reporter, helper.numCacheEntries() == 1);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumHWCreations == 0);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumSWCreations == 1);
+}
+
+// Case 4: ensure that, if two DDL recorders get in a race, they still end up sharing a single view
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GrThreadSafeViewCache4, reporter, ctxInfo) {
+    TestHelper helper(ctxInfo.directContext());
+
+    helper.accessCachedView(helper.ddlCanvas1(), kImageWH);
+    helper.checkView(helper.ddlCanvas1(), kImageWH, /*hits*/ 0, /*misses*/ 1, /*refs*/ 1);
+
+    static const bool kFailLookup = true;
+    helper.accessCachedView(helper.ddlCanvas2(), kImageWH, kFailLookup);
+    helper.checkView(helper.ddlCanvas2(), kImageWH, /*hits*/ 0, /*misses*/ 2, /*refs*/ 2);
+
+    REPORTER_ASSERT(reporter, helper.numCacheEntries() == 1);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumHWCreations == 0);
+    REPORTER_ASSERT(reporter, helper.stats()->fNumSWCreations == 2);
+}
diff --git a/tests/SkVMTest.cpp b/tests/SkVMTest.cpp
index 4208bad..34ba94a 100644
--- a/tests/SkVMTest.cpp
+++ b/tests/SkVMTest.cpp
@@ -2406,6 +2406,12 @@
           0x10001000, 0x14401440, 0x19001900, 0x1e401e40,
           0x24002400, 0x2a402a40, 0x31003100, 0x38403840, 0x40004000}},
 
+        {[](skvm::Q14x2 x) { return -(x*-x); }, // square, version B
+         {0x00000000, 0x00400040, 0x01000100, 0x02400240,
+          0x04000400, 0x06400640, 0x09000900, 0x0c400c40,
+          0x10001000, 0x14401440, 0x19001900, 0x1e401e40,
+          0x24002400, 0x2a402a40, 0x31003100, 0x38403840, 0x40004000}},
+
         {[](skvm::Q14x2 x) { return x>>1; },  // divide by 2
          {0x00000000, 0xfe000200, 0xfc000400, 0xfa000600,
           0xf8000800, 0xf6000a00, 0xf4000c00, 0xf2000e00,
diff --git a/tests/TestUtils.cpp b/tests/TestUtils.cpp
index d2b9479..3499376 100644
--- a/tests/TestUtils.cpp
+++ b/tests/TestUtils.cpp
@@ -144,6 +144,25 @@
     return backendTex->isValid();
 }
 
+bool CreateBackendTexture(GrDirectContext* dContext,
+                          GrBackendTexture* backendTex,
+                          const SkBitmap& bm) {
+    bool finishedBECreate = false;
+    auto markFinished = [](void* context) {
+        *(bool*)context = true;
+    };
+
+    *backendTex = dContext->createBackendTexture(bm.pixmap(), GrRenderable::kNo, GrProtected::kNo,
+                                                 markFinished, &finishedBECreate);
+    if (backendTex->isValid()) {
+        dContext->submit();
+        while (!finishedBECreate) {
+            dContext->checkAsyncWorkCompletion();
+        }
+    }
+    return backendTex->isValid();
+}
+
 void DeleteBackendTexture(GrDirectContext* dContext, const GrBackendTexture& backendTex) {
     dContext->flush();
     dContext->submit(true);
diff --git a/tests/TestUtils.h b/tests/TestUtils.h
index 605aff0..90052ff 100644
--- a/tests/TestUtils.h
+++ b/tests/TestUtils.h
@@ -49,6 +49,10 @@
                           GrRenderable,
                           GrProtected = GrProtected::kNo);
 
+bool CreateBackendTexture(GrDirectContext*,
+                          GrBackendTexture* backendTex,
+                          const SkBitmap& bm);
+
 void DeleteBackendTexture(GrDirectContext*, const GrBackendTexture& backendTex);
 
 // Checks srcBuffer and dstBuffer contain the same colors
diff --git a/tools/debugger/DebugCanvas.cpp b/tools/debugger/DebugCanvas.cpp
index 5335e0f..68b373e 100644
--- a/tools/debugger/DebugCanvas.cpp
+++ b/tools/debugger/DebugCanvas.cpp
@@ -251,7 +251,7 @@
     fCommandVector.remove(index);
 }
 
-DrawCommand* DebugCanvas::getDrawCommandAt(int index) {
+DrawCommand* DebugCanvas::getDrawCommandAt(int index) const {
     SkASSERT(index < fCommandVector.count());
     return fCommandVector[index];
 }
@@ -596,3 +596,37 @@
     SkASSERT(index < fCommandVector.count());
     fCommandVector[index]->setVisible(toggle);
 }
+
+std::map<int, std::vector<int>> DebugCanvas::getImageIdToCommandMap(UrlDataManager& udm) const {
+    // map from image ids to list of commands that reference them.
+    std::map<int, std::vector<int>> m;
+
+    for (int i = 0; i < this->getSize(); i++) {
+        const DrawCommand* command = this->getDrawCommandAt(i);
+        int imageIndex = -1;
+        // this is not an exaustive list of where images can be used, they show up in paints too.
+        switch (command->getOpType()) {
+            case DrawCommand::OpType::kDrawImage_OpType: {
+                imageIndex = static_cast<const DrawImageCommand*>(command)->imageId(udm);
+                break;
+            }
+            case DrawCommand::OpType::kDrawImageRect_OpType: {
+                imageIndex = static_cast<const DrawImageRectCommand*>(command)->imageId(udm);
+                break;
+            }
+            case DrawCommand::OpType::kDrawImageNine_OpType: {
+                imageIndex = static_cast<const DrawImageNineCommand*>(command)->imageId(udm);
+                break;
+            }
+            case DrawCommand::OpType::kDrawImageLattice_OpType: {
+                imageIndex = static_cast<const DrawImageLatticeCommand*>(command)->imageId(udm);
+                break;
+            }
+            default: break;
+        }
+        if (imageIndex >= 0) {
+            m[imageIndex].push_back(i);
+        }
+    }
+    return m;
+}
diff --git a/tools/debugger/DebugCanvas.h b/tools/debugger/DebugCanvas.h
index fe1d32d..d966bf8 100644
--- a/tools/debugger/DebugCanvas.h
+++ b/tools/debugger/DebugCanvas.h
@@ -19,6 +19,9 @@
 #include "tools/debugger/DebugLayerManager.h"
 #include "tools/debugger/DrawCommand.h"
 
+#include <map>
+#include <vector>
+
 class GrAuditTrail;
 class SkNWayCanvas;
 class SkPicture;
@@ -103,7 +106,7 @@
         Returns the draw command at the given index.
         @param index  The index of the command
      */
-    DrawCommand* getDrawCommandAt(int index);
+    DrawCommand* getDrawCommandAt(int index) const;
 
     /**
         Returns length of draw command vector.
@@ -127,6 +130,11 @@
 
     void detachCommands(SkTDArray<DrawCommand*>* dst) { fCommandVector.swap(*dst); }
 
+    /**
+        Returns a map from image IDs to command indices where they are used.
+     */
+    std::map<int, std::vector<int>> getImageIdToCommandMap(UrlDataManager& udm) const;
+
 protected:
     void              willSave() override;
     SaveLayerStrategy getSaveLayerStrategy(const SaveLayerRec&) override;
diff --git a/tools/debugger/DrawCommand.cpp b/tools/debugger/DrawCommand.cpp
index 6811d43..6dbcc5c 100644
--- a/tools/debugger/DrawCommand.cpp
+++ b/tools/debugger/DrawCommand.cpp
@@ -1198,13 +1198,15 @@
     return true;
 }
 
+uint64_t DrawImageCommand::imageId(UrlDataManager& udm) const {
+    return udm.lookupImage(fImage.get());
+}
+
 void DrawImageCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const {
     INHERITED::toJSON(writer, urlDataManager);
-
-
     if (urlDataManager.hasImageIndex()) {
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_IMAGE_INDEX);
-        writer.appendU64((uint64_t)urlDataManager.lookupImage(fImage.get()));
+        writer.appendU64(imageId(urlDataManager));
     } else {
         writer.beginObject(DEBUGCANVAS_ATTRIBUTE_IMAGE);
         flatten(*fImage, writer, urlDataManager);
@@ -1261,11 +1263,15 @@
     return true;
 }
 
+uint64_t DrawImageLatticeCommand::imageId(UrlDataManager& udm) const {
+    return udm.lookupImage(fImage.get());
+}
+
 void DrawImageLatticeCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const {
     INHERITED::toJSON(writer, urlDataManager);
     if (urlDataManager.hasImageIndex()) {
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_IMAGE_INDEX);
-        writer.appendU64((uint64_t)urlDataManager.lookupImage(fImage.get()));
+        writer.appendU64(imageId(urlDataManager));
     } else {
         writer.beginObject(DEBUGCANVAS_ATTRIBUTE_IMAGE);
         flatten(*fImage, writer, urlDataManager);
@@ -1312,11 +1318,15 @@
     return true;
 }
 
+uint64_t DrawImageRectCommand::imageId(UrlDataManager& udm) const {
+    return udm.lookupImage(fImage.get());
+}
+
 void DrawImageRectCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const {
     INHERITED::toJSON(writer, urlDataManager);
     if (urlDataManager.hasImageIndex()) {
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_IMAGE_INDEX);
-        writer.appendU64((uint64_t)urlDataManager.lookupImage(fImage.get()));
+        writer.appendU64(imageId(urlDataManager));
     } else {
         writer.beginObject(DEBUGCANVAS_ATTRIBUTE_IMAGE);
         flatten(*fImage, writer, urlDataManager);
@@ -1424,11 +1434,15 @@
     return true;
 }
 
+uint64_t DrawImageNineCommand::imageId(UrlDataManager& udm) const {
+    return udm.lookupImage(fImage.get());
+}
+
 void DrawImageNineCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const {
     INHERITED::toJSON(writer, urlDataManager);
     if (urlDataManager.hasImageIndex()) {
         writer.appendName(DEBUGCANVAS_ATTRIBUTE_IMAGE_INDEX);
-        writer.appendU64((uint64_t)urlDataManager.lookupImage(fImage.get()));
+        writer.appendU64(imageId(urlDataManager));
     } else {
         writer.beginObject(DEBUGCANVAS_ATTRIBUTE_IMAGE);
         flatten(*fImage, writer, urlDataManager);
diff --git a/tools/debugger/DrawCommand.h b/tools/debugger/DrawCommand.h
index 4171ea3..e15d02f 100644
--- a/tools/debugger/DrawCommand.h
+++ b/tools/debugger/DrawCommand.h
@@ -113,6 +113,7 @@
     static bool flatten(const SkBitmap& bitmap,
                         SkJSONWriter&   writer,
                         UrlDataManager& urlDataManager);
+    OpType getOpType() const { return fOpType; }
 
 private:
     OpType fOpType;
@@ -255,6 +256,7 @@
     void execute(SkCanvas* canvas) const override;
     bool render(SkCanvas* canvas) const override;
     void toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const override;
+    uint64_t imageId(UrlDataManager& udb) const;
 
 private:
     sk_sp<const SkImage> fImage;
@@ -274,6 +276,7 @@
     void execute(SkCanvas* canvas) const override;
     bool render(SkCanvas* canvas) const override;
     void toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const override;
+    uint64_t imageId(UrlDataManager& udb) const;
 
 private:
     sk_sp<const SkImage> fImage;
@@ -293,6 +296,7 @@
     void execute(SkCanvas* canvas) const override;
     bool render(SkCanvas* canvas) const override;
     void toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const override;
+    uint64_t imageId(UrlDataManager& udb) const;
 
 private:
     sk_sp<const SkImage> fImage;
@@ -313,6 +317,7 @@
     void execute(SkCanvas* canvas) const override;
     bool render(SkCanvas* canvas) const override;
     void toJSON(SkJSONWriter& writer, UrlDataManager& urlDataManager) const override;
+    uint64_t imageId(UrlDataManager& udm) const;
 
 private:
     sk_sp<const SkImage>        fImage;
diff --git a/tools/fm/fm_bot/fm_bot.go b/tools/fm/fm_bot/fm_bot.go
index eb781f9..f077b8a 100644
--- a/tools/fm/fm_bot/fm_bot.go
+++ b/tools/fm/fm_bot/fm_bot.go
@@ -71,15 +71,6 @@
 func parseWork(args []string, gms []string, tests []string) (*work, error) {
 	w := &work{}
 	for _, arg := range args {
-		// I wish we could parse flags here too, but it's too late.
-		if strings.HasPrefix(arg, "-") {
-			msg := "Is '%s' an fm flag? If so please pass it using flag=value syntax."
-			if flag.Lookup(arg[1:]) != nil {
-				msg = "Please pass fm_bot flags like '%s' on the command line before the FM binary."
-			}
-			return nil, fmt.Errorf(msg, arg)
-		}
-
 		// Everything after a # is a comment.
 		if strings.HasPrefix(arg, "#") {
 			break
@@ -96,6 +87,17 @@
 			continue
 		}
 
+		// -foo to remove foo if already added (e.g. by gms, tests).
+		if strings.HasPrefix(arg, "-") {
+			for i, s := range w.Sources {
+				if s == arg[1:] {
+					w.Sources = append(w.Sources[:i], w.Sources[i+1:]...)
+					break
+				}
+			}
+			continue
+		}
+
 		// Is this an option to pass through to fm?
 		if parts := strings.Split(arg, "="); len(parts) == 2 {
 			f := "-"