Only track compile-time expressions in definition maps.

This avoids an error case where the definition map points to an
expression that is later eliminated, leaving the definition map holding
a dangling pointer.

In practice we only use values from the definition map if they are
compile-time constants, so this generates the same code during
optimization.

Change-Id: I314ce8e1f9b4e0c90fbfe7c97e2a3950b65a5819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368059
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>

src/sksl/SkSLDefinitionMap.cpp

@@ -41,6 +41,27 @@ void DefinitionMap::computeStartState(const CFG& cfg) {
     }
 }
 
+Expression* DefinitionMap::getKnownDefinition(const Variable* var) const {
+    // Find this variable in the definition map.
+    std::unique_ptr<Expression>** exprPtr = fDefinitions.find(var);
+    if (!exprPtr || !*exprPtr) {
+        return nullptr;
+    }
+    // Return known expressions only; return "null" for defined-but-unknown expressions.
+    Expression* expr = (*exprPtr)->get();
+    return (expr->kind() != Expression::Kind::kDefined) ? expr : nullptr;
+}
+
+// Add compile-time constants to the definition map. Non-compile-time constant expressions are saved
+// as just "defined" since we can't guarantee that the expression will remain alive until the end of
+// optimization.
+void DefinitionMap::addDefinition(const Context& context, const Variable* var,
+                                  std::unique_ptr<Expression>* expr) {
+    fDefinitions.set(var, (*expr)->isCompileTimeConstant()
+                                  ? expr
+                                  : (std::unique_ptr<Expression>*)&context.fDefined_Expression);
+}
+
 // Add the definition created by assigning to the lvalue to the definition map.
 void DefinitionMap::addDefinition(const Context& context, const Expression* lvalue,
                                   std::unique_ptr<Expression>* expr) {
@@ -48,7 +69,7 @@ void DefinitionMap::addDefinition(const Context& context, const Expression* lval
         case Expression::Kind::kVariableReference: {
             const Variable& var = *lvalue->as<VariableReference>().variable();
             if (var.storage() == Variable::Storage::kLocal) {
-                fDefinitions.set(&var, expr);
+                this->addDefinition(context, &var, expr);
             }
             break;
         }
@@ -156,7 +177,7 @@ void DefinitionMap::addDefinitions(const Context& context, const BasicBlock::Nod
         if (stmt->is<VarDeclaration>()) {
             VarDeclaration& vd = stmt->as<VarDeclaration>();
             if (vd.value()) {
-                fDefinitions.set(&vd.var(), &vd.value());
+                this->addDefinition(context, &vd.var(), &vd.value());
             }
         }
     }

src/sksl/SkSLDefinitionMap.h

@@ -24,20 +24,38 @@ class DefinitionMap {
 public:
     using MapType = SkTHashMap<const Variable*, std::unique_ptr<Expression>*>;
 
-    // Allow callers to set, search and iterate directly, like a THashMap.
+    /**
+     * These accessors allow callers to set, search and iterate directly, like a THashMap.
+     */
     void set(const Variable* v, std::unique_ptr<Expression>* e) { fDefinitions.set(v, e); }
     std::unique_ptr<Expression>** find(const Variable* v) const { return fDefinitions.find(v); }
     std::unique_ptr<Expression>* get(const Variable* v) { return fDefinitions[v]; }
     MapType::Iter begin() const { return fDefinitions.begin(); }
     MapType::Iter end() const { return fDefinitions.end(); }
 
-    // These methods populate the definition map from the CFG.
+    /**
+     * Retrieves the compile-time-constant definition of this variable. Returns null if unknown, or
+     * if the definition is not a compile-time constant.
+     */
+    Expression* getKnownDefinition(const Variable* v) const;
+
+    /**
+     * Maps all local variables in the function to null, indicating that their value is initially
+     * unknown.
+     */
     void computeStartState(const CFG& cfg);
-    void addDefinition(const Context& context, const Expression* lvalue,
-                       std::unique_ptr<Expression>* expr);
+
+    /**
+     * Walks the CFG and populates the definition map with compile-time lvalues.
+     */
     void addDefinitions(const Context& context, const BasicBlockNode& node);
 
 private:
+    void addDefinition(const Context& context, const Variable* var,
+                       std::unique_ptr<Expression>* expr);
+    void addDefinition(const Context& context, const Expression* lvalue,
+                       std::unique_ptr<Expression>* expr);
+
     SkTHashMap<const Variable*, std::unique_ptr<Expression>*> fDefinitions;
 };
 

src/sksl/ir/SkSLVariableReference.cpp

@@ -59,11 +59,8 @@ std::unique_ptr<Expression> VariableReference::constantPropagate(const IRGenerat
         !this->type().isArray()) {
         return initialValue->clone();
     }
-    std::unique_ptr<Expression>** exprPPtr = definitions.find(this->variable());
-    if (exprPPtr && *exprPPtr && (**exprPPtr)->isCompileTimeConstant()) {
-        return (**exprPPtr)->clone();
-    }
-    return nullptr;
+    Expression* expr = definitions.getKnownDefinition(this->variable());
+    return expr ? expr->clone() : nullptr;
 }
 
 }  // namespace SkSL