SkSL performance improvements

src/sksl/SkSLIRGenerator.cpp

Index: src/sksl/SkSLIRGenerator.cpp
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 2cc7eacb4d9ec92bd5e0e57c12c8fb6ecb1a7b64..98f9f24c81e0c5b23d8add899c632ebae94a71d6 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -68,12 +68,12 @@ public:
 
 IRGenerator::IRGenerator(std::shared_ptr<SymbolTable> symbolTable, 
                          ErrorReporter& errorReporter)
-: fSymbolTable(std::move(symbolTable))
-, fErrors(errorReporter) {
-}
+: fCurrentFunction(nullptr)
+, fSymbolTable(std::move(symbolTable))
+, fErrors(errorReporter) {}
 
-void IRGenerator::pushSymbolTable() {
-    fSymbolTable.reset(new SymbolTable(std::move(fSymbolTable), fErrors));
+void IRGenerator::pushSymbolTable(bool owner) {
+    fSymbolTable.reset(new SymbolTable(std::move(fSymbolTable), fErrors, owner));
 }
 
 void IRGenerator::popSymbolTable() {
@@ -141,16 +141,16 @@ Modifiers IRGenerator::convertModifiers(const ASTModifiers& modifiers) {
 
 std::unique_ptr<VarDeclaration> IRGenerator::convertVarDeclaration(const ASTVarDeclaration& decl,
                                                                    Variable::Storage storage) {
-    std::vector<std::shared_ptr<Variable>> variables;
+    std::vector<const Variable*> variables;
     std::vector<std::vector<std::unique_ptr<Expression>>> sizes;
     std::vector<std::unique_ptr<Expression>> values;
-    std::shared_ptr<Type> baseType = this->convertType(*decl.fType);
+    const Type* baseType = this->convertType(*decl.fType);
     if (!baseType) {
         return nullptr;
     }
     for (size_t i = 0; i < decl.fNames.size(); i++) {
         Modifiers modifiers = this->convertModifiers(decl.fModifiers);
-        std::shared_ptr<Type> type = baseType;
+        const Type* type = baseType;
         ASSERT(type->kind() != Type::kArray_Kind);
         std::vector<std::unique_ptr<Expression>> currentVarSizes;
         for (size_t j = 0; j < decl.fSizes[i].size(); j++) {
@@ -172,17 +172,18 @@ std::unique_ptr<VarDeclaration> IRGenerator::convertVarDeclaration(const ASTVarD
                     count = -1;
                     name += "[]";
                 }
-                type = std::shared_ptr<Type>(new Type(name, Type::kArray_Kind, type, (int) count));
+                type = new Type(name, Type::kArray_Kind, *type, (int) count);
+                fSymbolTable->takeOwnership((Type*) type);
                 currentVarSizes.push_back(std::move(size));
             } else {
-                type = std::shared_ptr<Type>(new Type(type->fName + "[]", Type::kArray_Kind, type, 
-                                                      -1));
+                type = new Type(type->fName + "[]", Type::kArray_Kind, *type, -1);
+                fSymbolTable->takeOwnership((Type*) type);
                 currentVarSizes.push_back(nullptr);
             }
         }
         sizes.push_back(std::move(currentVarSizes));
-        auto var = std::make_shared<Variable>(decl.fPosition, modifiers, decl.fNames[i], type, 
-                                              storage);
+        Variable* var = new Variable(decl.fPosition, modifiers, decl.fNames[i], *type, 
+                                     storage);
         variables.push_back(var);
         std::unique_ptr<Expression> value;
         if (decl.fValues[i]) {
@@ -190,7 +191,7 @@ std::unique_ptr<VarDeclaration> IRGenerator::convertVarDeclaration(const ASTVarD
             if (!value) {
                 return nullptr;
             }
-            value = this->coerce(std::move(value), type);
+            value = this->coerce(std::move(value), *type);
         }
         fSymbolTable->add(var->fName, var);
         values.push_back(std::move(value));
@@ -298,7 +299,7 @@ std::unique_ptr<Statement> IRGenerator::convertReturn(const ASTReturnStatement&
     } else {
         if (fCurrentFunction->fReturnType != kVoid_Type) {
             fErrors.error(r.fPosition, "expected function to return '" +
-                                       fCurrentFunction->fReturnType->description() + "'");
+                                       fCurrentFunction->fReturnType.description() + "'");
         }
         return std::unique_ptr<Statement>(new ReturnStatement(r.fPosition));
     }
@@ -316,80 +317,74 @@ std::unique_ptr<Statement> IRGenerator::convertDiscard(const ASTDiscardStatement
     return std::unique_ptr<Statement>(new DiscardStatement(d.fPosition));
 }
 
-static std::shared_ptr<Type> expand_generics(std::shared_ptr<Type> type, int i) {
-    if (type->kind() == Type::kGeneric_Kind) {
-        return type->coercibleTypes()[i];
+static const Type& expand_generics(const Type& type, int i) {
+    if (type.kind() == Type::kGeneric_Kind) {
+        return *type.coercibleTypes()[i];
     }
     return type;
 }
 
-static void expand_generics(FunctionDeclaration& decl, 
+static void expand_generics(const FunctionDeclaration& decl, 
                             SymbolTable& symbolTable) {
     for (int i = 0; i < 4; i++) {
-        std::shared_ptr<Type> returnType = expand_generics(decl.fReturnType, i);
-        std::vector<std::shared_ptr<Variable>> arguments;
+        const Type& returnType = expand_generics(decl.fReturnType, i);
+        std::vector<const Variable*> parameters;
         for (const auto& p : decl.fParameters) {
-            arguments.push_back(std::shared_ptr<Variable>(new Variable(
-                                                                    p->fPosition, 
-                                                                    Modifiers(p->fModifiers), 
-                                                                    p->fName,
-                                                                    expand_generics(p->fType, i),
-                                                                    Variable::kParameter_Storage)));
+            Variable* var = new Variable(p->fPosition, Modifiers(p->fModifiers), p->fName,
+                                         expand_generics(p->fType, i),
+                                         Variable::kParameter_Storage);
+            symbolTable.takeOwnership(var);
+            parameters.push_back(var);
         }
-        std::shared_ptr<FunctionDeclaration> expanded(new FunctionDeclaration(
-                                                                            decl.fPosition, 
-                                                                            decl.fName, 
-                                                                            std::move(arguments), 
-                                                                            std::move(returnType)));
+        FunctionDeclaration* expanded = new FunctionDeclaration(decl.fPosition,
+                                                                decl.fName,
+                                                                std::move(parameters),
+                                                                std::move(returnType));
         symbolTable.add(expanded->fName, expanded);
     }
 }
 
 std::unique_ptr<FunctionDefinition> IRGenerator::convertFunction(const ASTFunction& f) {
-    std::shared_ptr<SymbolTable> old = fSymbolTable;
-    AutoSymbolTable table(this);
     bool isGeneric;
-    std::shared_ptr<Type> returnType = this->convertType(*f.fReturnType);
+    const Type* returnType = this->convertType(*f.fReturnType);
     if (!returnType) {
         return nullptr;
     }
     isGeneric = returnType->kind() == Type::kGeneric_Kind;
-    std::vector<std::shared_ptr<Variable>> parameters;
+    std::vector<const Variable*> parameters;
     for (const auto& param : f.fParameters) {
-        std::shared_ptr<Type> type = this->convertType(*param->fType);
+        const Type* type = this->convertType(*param->fType);
         if (!type) {
             return nullptr;
         }
         for (int j = (int) param->fSizes.size() - 1; j >= 0; j--) {
             int size = param->fSizes[j];
             std::string name = type->name() + "[" + to_string(size) + "]";
-            type = std::shared_ptr<Type>(new Type(std::move(name), Type::kArray_Kind, 
-                                                  std::move(type), size));
+            Type* newType = new Type(std::move(name), Type::kArray_Kind, *type, size);
+            fSymbolTable->takeOwnership(newType);
+            type = newType;
         }
         std::string name = param->fName;
         Modifiers modifiers = this->convertModifiers(param->fModifiers);
         Position pos = param->fPosition;
-        std::shared_ptr<Variable> var = std::shared_ptr<Variable>(new Variable(
-                                                                     pos, 
-                                                                     modifiers, 
-                                                                     std::move(name), 
-                                                                     type,
-                                                                     Variable::kParameter_Storage));
-        parameters.push_back(std::move(var));
+        Variable* var = new Variable(pos, modifiers, std::move(name), *type,
+                                     Variable::kParameter_Storage);
+        fSymbolTable->takeOwnership(var);
+        parameters.push_back(var);
         isGeneric |= type->kind() == Type::kGeneric_Kind;
     }
 
     // find existing declaration
-    std::shared_ptr<FunctionDeclaration> decl;
-    auto entry = (*old)[f.fName];
+    const FunctionDeclaration* decl = nullptr;
+    auto entry = (*fSymbolTable)[f.fName];
     if (entry) {
-        std::vector<std::shared_ptr<FunctionDeclaration>> functions;
+        std::vector<const FunctionDeclaration*> functions;
         switch (entry->fKind) {
             case Symbol::kUnresolvedFunction_Kind:
-                functions = std::static_pointer_cast<UnresolvedFunction>(entry)->fFunctions;
+                functions = ((UnresolvedFunction*) entry)->fFunctions;
                 break;
             case Symbol::kFunctionDeclaration_Kind:
-                functions.push_back(std::static_pointer_cast<FunctionDeclaration>(entry));
+                functions.push_back((FunctionDeclaration*) entry);
                 break;
             default:
                 fErrors.error(f.fPosition, "symbol '" + f.fName + "' was already defined");
@@ -406,11 +401,8 @@ std::unique_ptr<FunctionDefinition> IRGenerator::convertFunction(const ASTFuncti
                     }
                 }
                 if (match) {
-                    if (returnType != other->fReturnType) {
-                        FunctionDeclaration newDecl = FunctionDeclaration(f.fPosition, 
-                                                                          f.fName, 
-                                                                          parameters, 
-                                                                          returnType);
+                    if (*returnType != other->fReturnType) {
+                        FunctionDeclaration newDecl(f.fPosition, f.fName, parameters, *returnType);
                         fErrors.error(f.fPosition, "functions '" + newDecl.description() +
                                                    "' and '" + other->description() + 
                                                    "' differ only in return type");
@@ -424,7 +416,6 @@ std::unique_ptr<FunctionDefinition> IRGenerator::convertFunction(const ASTFuncti
                                                        "declaration and definition");
                             return nullptr;
                         }
-                        fSymbolTable->add(parameters[i]->fName, decl->fParameters[i]);
                     }
                     if (other->fDefined) {
                         fErrors.error(f.fPosition, "duplicate definition of " + 
@@ -437,28 +428,31 @@ std::unique_ptr<FunctionDefinition> IRGenerator::convertFunction(const ASTFuncti
     }
     if (!decl) {
         // couldn't find an existing declaration
-        decl.reset(new FunctionDeclaration(f.fPosition, f.fName, parameters, returnType));
-        for (auto var : parameters) {
-            fSymbolTable->add(var->fName, var);
+        decl = new FunctionDeclaration(f.fPosition, f.fName, parameters, *returnType);
+        if (isGeneric) {
+            ASSERT(!f.fBody);
+            expand_generics(*decl, *fSymbolTable);
+            delete decl;

[dogben, 2016/07/08 19:56:40]

Missing return.

nit: maybe stack-allocate the FunctionDeclaration in 'then' block and only set
decl in 'else' block.

+        } else {
+            fSymbolTable->add(decl->fName, (FunctionDeclaration*) decl);
         }
     }
-    if (isGeneric) {
-        ASSERT(!f.fBody);
-        expand_generics(*decl, *old);
-    } else {
-        old->add(decl->fName, decl);
-        if (f.fBody) {
-            ASSERT(!fCurrentFunction);
-            fCurrentFunction = decl;
-            decl->fDefined = true;
-            std::unique_ptr<Block> body = this->convertBlock(*f.fBody);
-            fCurrentFunction = nullptr;
-            if (!body) {
-                return nullptr;
-            }
-            return std::unique_ptr<FunctionDefinition>(new FunctionDefinition(f.fPosition, decl, 
-                                                                              std::move(body)));
+    if (f.fBody) {
+        ASSERT(!fCurrentFunction);
+        fCurrentFunction = decl;
+        decl->fDefined = true;
+        std::shared_ptr<SymbolTable> old = fSymbolTable;
+        AutoSymbolTable table(this);
+        for (size_t i = 0; i < parameters.size(); i++) {
+            fSymbolTable->addWithoutOwnership(parameters[i]->fName, decl->fParameters[i]);
+        }
+        std::unique_ptr<Block> body = this->convertBlock(*f.fBody);
+        fCurrentFunction = nullptr;
+        if (!body) {
+            return nullptr;
         }
+        return std::unique_ptr<FunctionDefinition>(new FunctionDefinition(f.fPosition, *decl, 
+                                                                          std::move(body)));
     }
     return nullptr;
 }
@@ -488,28 +482,24 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte
             }
         }        
     }
-    std::shared_ptr<Type> type = std::shared_ptr<Type>(new Type(intf.fInterfaceName, fields));
+    Type* type = new Type(intf.fInterfaceName, fields);
+    fSymbolTable->takeOwnership(type);
     std::string name = intf.fValueName.length() > 0 ? intf.fValueName : intf.fInterfaceName;
-    std::shared_ptr<Variable> var = std::shared_ptr<Variable>(new Variable(intf.fPosition, mods, 
-                                                                          name, type,
-                                                                Variable::kGlobal_Storage));
+    Variable* var = new Variable(intf.fPosition, mods, name, *type, Variable::kGlobal_Storage);
     if (intf.fValueName.length()) {
         old->add(intf.fValueName, var);
-
     } else {
         for (size_t i = 0; i < fields.size(); i++) {
-            std::shared_ptr<Field> field = std::shared_ptr<Field>(new Field(intf.fPosition, var, 
-                                                                            (int) i));
-            old->add(fields[i].fName, field);
+            old->add(fields[i].fName, new Field(intf.fPosition, *var, (int) i));
         }
     }
-    return std::unique_ptr<InterfaceBlock>(new InterfaceBlock(intf.fPosition, var));
+    return std::unique_ptr<InterfaceBlock>(new InterfaceBlock(intf.fPosition, *var));
 }
 
-std::shared_ptr<Type> IRGenerator::convertType(const ASTType& type) {
-    std::shared_ptr<Symbol> result = (*fSymbolTable)[type.fName];
+const Type* IRGenerator::convertType(const ASTType& type) {
+    const Symbol* result = (*fSymbolTable)[type.fName];
     if (result && result->fKind == Symbol::kType_Kind) {
-        return std::static_pointer_cast<Type>(result);
+        return (const Type*) result;
     }
     fErrors.error(type.fPosition, "unknown type '" + type.fName + "'");
     return nullptr;
@@ -542,40 +532,40 @@ std::unique_ptr<Expression> IRGenerator::convertExpression(const ASTExpression&
 }
 
 std::unique_ptr<Expression> IRGenerator::convertIdentifier(const ASTIdentifier& identifier) {
-    std::shared_ptr<Symbol> result = (*fSymbolTable)[identifier.fText];
+    const Symbol* result = (*fSymbolTable)[identifier.fText];
     if (!result) {
         fErrors.error(identifier.fPosition, "unknown identifier '" + identifier.fText + "'");
         return nullptr;
     }
     switch (result->fKind) {
         case Symbol::kFunctionDeclaration_Kind: {
-            std::vector<std::shared_ptr<FunctionDeclaration>> f = {
-                std::static_pointer_cast<FunctionDeclaration>(result)
+            std::vector<const FunctionDeclaration*> f = {
+                (const FunctionDeclaration*) result
             };
             return std::unique_ptr<FunctionReference>(new FunctionReference(identifier.fPosition,
-                                                                            std::move(f)));
+                                                                            f));
         }
         case Symbol::kUnresolvedFunction_Kind: {
-            auto f = std::static_pointer_cast<UnresolvedFunction>(result);
+            const UnresolvedFunction* f = (const UnresolvedFunction*) result;
             return std::unique_ptr<FunctionReference>(new FunctionReference(identifier.fPosition,
                                                                             f->fFunctions));
         }
         case Symbol::kVariable_Kind: {
-            std::shared_ptr<Variable> var = std::static_pointer_cast<Variable>(result);
-            this->markReadFrom(var);
+            const Variable* var = (const Variable*) result;
+            this->markReadFrom(*var);
             return std::unique_ptr<VariableReference>(new VariableReference(identifier.fPosition,
-                                                                            std::move(var)));
+                                                                            *var));
         }
         case Symbol::kField_Kind: {
-            std::shared_ptr<Field> field = std::static_pointer_cast<Field>(result);
+            const Field* field = (const Field*) result;
             VariableReference* base = new VariableReference(identifier.fPosition, field->fOwner);
             return std::unique_ptr<Expression>(new FieldAccess(std::unique_ptr<Expression>(base),
                                                                field->fFieldIndex));
         }
         case Symbol::kType_Kind: {
-            auto t = std::static_pointer_cast<Type>(result);
+            const Type* t = (const Type*) result;
             return std::unique_ptr<TypeReference>(new TypeReference(identifier.fPosition, 
-                                                                    std::move(t)));
+                                                                    *t));
         }
         default:
             ABORT("unsupported symbol type %d\n", result->fKind);
@@ -584,43 +574,42 @@ std::unique_ptr<Expression> IRGenerator::convertIdentifier(const ASTIdentifier&
 }
 
 std::unique_ptr<Expression> IRGenerator::coerce(std::unique_ptr<Expression> expr, 
-                                                std::shared_ptr<Type> type) {
+                                                const Type& type) {
     if (!expr) {
         return nullptr;
     }
-    if (*expr->fType == *type) {
+    if (expr->fType == type) {
         return expr;
     }
     this->checkValid(*expr);
-    if (*expr->fType == *kInvalid_Type) {
+    if (expr->fType == kInvalid_Type) {
         return nullptr;
     }
-    if (!expr->fType->canCoerceTo(type)) {
-        fErrors.error(expr->fPosition, "expected '" + type->description() + "', but found '" + 
-                                        expr->fType->description() + "'");
+    if (!expr->fType.canCoerceTo(type)) {
+        fErrors.error(expr->fPosition, "expected '" + type.description() + "', but found '" + 
+                                        expr->fType.description() + "'");
         return nullptr;
     }
-    if (type->kind() == Type::kScalar_Kind) {
+    if (type.kind() == Type::kScalar_Kind) {
         std::vector<std::unique_ptr<Expression>> args;
         args.push_back(std::move(expr));
-        ASTIdentifier id(Position(), type->description());
+        ASTIdentifier id(Position(), type.description());
         std::unique_ptr<Expression> ctor = this->convertIdentifier(id);
         ASSERT(ctor);
         return this->call(Position(), std::move(ctor), std::move(args));
     }
-    ABORT("cannot coerce %s to %s", expr->fType->description().c_str(), 
-          type->description().c_str());
+    ABORT("cannot coerce %s to %s", expr->fType.description().c_str(), 
+          type.description().c_str());
 }
 
 /**
  * Determines the operand and result types of a binary expression. Returns true if the expression is
  * legal, false otherwise. If false, the values of the out parameters are undefined.
  */
-static bool determine_binary_type(Token::Kind op, std::shared_ptr<Type> left, 
-                                  std::shared_ptr<Type> right, 
-                                  std::shared_ptr<Type>* outLeftType,
-                                  std::shared_ptr<Type>* outRightType,
-                                  std::shared_ptr<Type>* outResultType,
+static bool determine_binary_type(Token::Kind op, const Type& left, const Type& right, 
+                                  const Type** outLeftType,
+                                  const Type** outRightType,
+                                  const Type** outResultType,
                                   bool tryFlipped) {
     bool isLogical;
     switch (op) {
@@ -638,24 +627,24 @@ static bool determine_binary_type(Token::Kind op, std::shared_ptr<Type> left,
         case Token::LOGICALOREQ: // fall through
         case Token::LOGICALANDEQ: // fall through
         case Token::LOGICALXOREQ:
-            *outLeftType = kBool_Type;
-            *outRightType = kBool_Type;
-            *outResultType = kBool_Type;
-            return left->canCoerceTo(kBool_Type) && right->canCoerceTo(kBool_Type);
+            *outLeftType = &kBool_Type;
+            *outRightType = &kBool_Type;
+            *outResultType = &kBool_Type;
+            return left.canCoerceTo(kBool_Type) && right.canCoerceTo(kBool_Type);
         case Token::STAR: // fall through
         case Token::STAREQ: 
             // FIXME need to handle non-square matrices
-            if (left->kind() == Type::kMatrix_Kind && right->kind() == Type::kVector_Kind) {
-                *outLeftType = left;
-                *outRightType = right;
-                *outResultType = right;
-                return left->rows() == right->columns();
+            if (left.kind() == Type::kMatrix_Kind && right.kind() == Type::kVector_Kind) {
+                *outLeftType = &left;
+                *outRightType = &right;
+                *outResultType = &right;
+                return left.rows() == right.columns();
             }  
-            if (left->kind() == Type::kVector_Kind && right->kind() == Type::kMatrix_Kind) {
-                *outLeftType = left;
-                *outRightType = right;
-                *outResultType = left;
-                return left->columns() == right->columns();
+            if (left.kind() == Type::kVector_Kind && right.kind() == Type::kMatrix_Kind) {
+                *outLeftType = &left;
+                *outRightType = &right;
+                *outResultType = &left;
+                return left.columns() == right.columns();
             }
             // fall through
         default:
@@ -664,33 +653,33 @@ static bool determine_binary_type(Token::Kind op, std::shared_ptr<Type> left,
     // FIXME: need to disallow illegal operations like vec3 > vec3. Also do not currently have
     // full support for numbers other than float.
     if (left == right) {
-        *outLeftType = left;
-        *outRightType = left;
+        *outLeftType = &left;
+        *outRightType = &left;
         if (isLogical) {
-            *outResultType = kBool_Type;
+            *outResultType = &kBool_Type;
         } else {
-            *outResultType = left;
+            *outResultType = &left;
         }
         return true;
     }
     // FIXME: incorrect for shift operations
-    if (left->canCoerceTo(right)) {
-        *outLeftType = right;
-        *outRightType = right;
+    if (left.canCoerceTo(right)) {
+        *outLeftType = &right;
+        *outRightType = &right;
         if (isLogical) {
-            *outResultType = kBool_Type;
+            *outResultType = &kBool_Type;
         } else {
-            *outResultType = right;
+            *outResultType = &right;
         }
         return true;
     }
-    if ((left->kind() == Type::kVector_Kind || left->kind() == Type::kMatrix_Kind) && 
-        (right->kind() == Type::kScalar_Kind)) {
-        if (determine_binary_type(op, left->componentType(), right, outLeftType, outRightType,
+    if ((left.kind() == Type::kVector_Kind || left.kind() == Type::kMatrix_Kind) && 
+        (right.kind() == Type::kScalar_Kind)) {
+        if (determine_binary_type(op, left.componentType(), right, outLeftType, outRightType,
                                   outResultType, false)) {
-            *outLeftType = (*outLeftType)->toCompound(left->columns(), left->rows());
+            *outLeftType = &(*outLeftType)->toCompound(left.columns(), left.rows());
             if (!isLogical) {
-                *outResultType = (*outResultType)->toCompound(left->columns(), left->rows());
+                *outResultType = &(*outResultType)->toCompound(left.columns(), left.rows());
             }
             return true;
         }
@@ -713,15 +702,15 @@ std::unique_ptr<Expression> IRGenerator::convertBinaryExpression(
     if (!right) {
         return nullptr;
     }
-    std::shared_ptr<Type> leftType;
-    std::shared_ptr<Type> rightType;
-    std::shared_ptr<Type> resultType;
+    const Type* leftType;
+    const Type* rightType;
+    const Type* resultType;
     if (!determine_binary_type(expression.fOperator, left->fType, right->fType, &leftType,
                                &rightType, &resultType, true)) {
         fErrors.error(expression.fPosition, "type mismatch: '" + 
                                             Token::OperatorName(expression.fOperator) + 
-                                            "' cannot operate on '" + left->fType->fName + 
-                                            "', '" + right->fType->fName + "'");
+                                            "' cannot operate on '" + left->fType.fName + 
+                                            "', '" + right->fType.fName + "'");
         return nullptr;
     }
     switch (expression.fOperator) {
@@ -744,11 +733,12 @@ std::unique_ptr<Expression> IRGenerator::convertBinaryExpression(
             break;
     }
     return std::unique_ptr<Expression>(new BinaryExpression(expression.fPosition, 
-                                                            this->coerce(std::move(left), leftType), 
+                                                            this->coerce(std::move(left), 
+                                                                         *leftType), 
                                                             expression.fOperator, 
                                                             this->coerce(std::move(right), 
-                                                                         rightType), 
-                                                            resultType));
+                                                                         *rightType), 
+                                                            *resultType));
 }
 
 std::unique_ptr<Expression> IRGenerator::convertTernaryExpression(  
@@ -766,34 +756,33 @@ std::unique_ptr<Expression> IRGenerator::convertTernaryExpression(
     if (!ifFalse) {
         return nullptr;
     }
-    std::shared_ptr<Type> trueType;
-    std::shared_ptr<Type> falseType;
-    std::shared_ptr<Type> resultType;
+    const Type* trueType;
+    const Type* falseType;
+    const Type* resultType;
     if (!determine_binary_type(Token::EQEQ, ifTrue->fType, ifFalse->fType, &trueType,
                                &falseType, &resultType, true)) {
         fErrors.error(expression.fPosition, "ternary operator result mismatch: '" + 
-                                            ifTrue->fType->fName + "', '" + 
-                                            ifFalse->fType->fName + "'");
+                                            ifTrue->fType.fName + "', '" + 
+                                            ifFalse->fType.fName + "'");
         return nullptr;
     }
     ASSERT(trueType == falseType);
-    ifTrue = this->coerce(std::move(ifTrue), trueType);
-    ifFalse = this->coerce(std::move(ifFalse), falseType);
+    ifTrue = this->coerce(std::move(ifTrue), *trueType);
+    ifFalse = this->coerce(std::move(ifFalse), *falseType);
     return std::unique_ptr<Expression>(new TernaryExpression(expression.fPosition, 
                                                              std::move(test),
                                                              std::move(ifTrue), 
                                                              std::move(ifFalse)));
 }
 
-std::unique_ptr<Expression> IRGenerator::call(
-                                         Position position, 
-                                         std::shared_ptr<FunctionDeclaration> function, 
-                                         std::vector<std::unique_ptr<Expression>> arguments) {
-    if (function->fParameters.size() != arguments.size()) {
-        std::string msg = "call to '" + function->fName + "' expected " + 
-                                 to_string(function->fParameters.size()) + 
+std::unique_ptr<Expression> IRGenerator::call(Position position, 
+                                              const FunctionDeclaration& function, 
+                                              std::vector<std::unique_ptr<Expression>> arguments) {
+    if (function.fParameters.size() != arguments.size()) {
+        std::string msg = "call to '" + function.fName + "' expected " + 
+                                 to_string(function.fParameters.size()) + 
                                  " argument";
-        if (function->fParameters.size() != 1) {
+        if (function.fParameters.size() != 1) {
             msg += "s";
         }
         msg += ", but found " + to_string(arguments.size());
@@ -801,12 +790,12 @@ std::unique_ptr<Expression> IRGenerator::call(
         return nullptr;
     }
     for (size_t i = 0; i < arguments.size(); i++) {
-        arguments[i] = this->coerce(std::move(arguments[i]), function->fParameters[i]->fType);
-        if (arguments[i] && (function->fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) {
+        arguments[i] = this->coerce(std::move(arguments[i]), function.fParameters[i]->fType);
+        if (arguments[i] && (function.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) {
             this->markWrittenTo(*arguments[i]);
         }
     }
-    return std::unique_ptr<FunctionCall>(new FunctionCall(position, std::move(function),
+    return std::unique_ptr<FunctionCall>(new FunctionCall(position, function,
                                                           std::move(arguments)));
 }
 
@@ -815,16 +804,16 @@ std::unique_ptr<Expression> IRGenerator::call(
  * if the cost could be computed, false if the call is not valid. Cost has no particular meaning 
  * other than "lower costs are preferred".
  */
-bool IRGenerator::determineCallCost(std::shared_ptr<FunctionDeclaration> function, 
+bool IRGenerator::determineCallCost(const FunctionDeclaration& function, 
                                     const std::vector<std::unique_ptr<Expression>>& arguments,
                                     int* outCost) {
-    if (function->fParameters.size() != arguments.size()) {
+    if (function.fParameters.size() != arguments.size()) {
         return false;
     }
     int total = 0;
     for (size_t i = 0; i < arguments.size(); i++) {
         int cost;
-        if (arguments[i]->fType->determineCoercionCost(function->fParameters[i]->fType, &cost)) {
+        if (arguments[i]->fType.determineCoercionCost(function.fParameters[i]->fType, &cost)) {
             total += cost;
         } else {
             return false;
@@ -848,40 +837,40 @@ std::unique_ptr<Expression> IRGenerator::call(Position position,
     }
     FunctionReference* ref = (FunctionReference*) functionValue.get();
     int bestCost = INT_MAX;
-    std::shared_ptr<FunctionDeclaration> best;
+    const FunctionDeclaration* best = nullptr;
     if (ref->fFunctions.size() > 1) {
         for (const auto& f : ref->fFunctions) {
             int cost;
-            if (this->determineCallCost(f, arguments, &cost) && cost < bestCost) {
+            if (this->determineCallCost(*f, arguments, &cost) && cost < bestCost) {
                 bestCost = cost;
                 best = f;
             }
         }
         if (best) {
-            return this->call(position, std::move(best), std::move(arguments));
+            return this->call(position, *best, std::move(arguments));
         }
         std::string msg = "no match for " + ref->fFunctions[0]->fName + "(";
         std::string separator = "";
         for (size_t i = 0; i < arguments.size(); i++) {
             msg += separator;
             separator = ", ";
-            msg += arguments[i]->fType->description();
+            msg += arguments[i]->fType.description();
         }
         msg += ")";
         fErrors.error(position, msg);
         return nullptr;
     }
-    return this->call(position, ref->fFunctions[0], std::move(arguments));
+    return this->call(position, *ref->fFunctions[0], std::move(arguments));
 }
 
 std::unique_ptr<Expression> IRGenerator::convertConstructor(
                                                     Position position, 
-                                                    std::shared_ptr<Type> type, 
+                                                    const Type& type, 
                                                     std::vector<std::unique_ptr<Expression>> args) {
     // FIXME: add support for structs and arrays
-    Type::Kind kind = type->kind();
-    if (!type->isNumber() && kind != Type::kVector_Kind && kind != Type::kMatrix_Kind) {
-        fErrors.error(position, "cannot construct '" + type->description() + "'");
+    Type::Kind kind = type.kind();
+    if (!type.isNumber() && kind != Type::kVector_Kind && kind != Type::kMatrix_Kind) {
+        fErrors.error(position, "cannot construct '" + type.description() + "'");
         return nullptr;
     }
     if (type == kFloat_Type && args.size() == 1 && 
@@ -893,9 +882,9 @@ std::unique_ptr<Expression> IRGenerator::convertConstructor(
         // argument is already the right type, just return it
         return std::move(args[0]);
     }
-    if (type->isNumber()) {
+    if (type.isNumber()) {
         if (args.size() != 1) {
-            fErrors.error(position, "invalid arguments to '" + type->description() + 
+            fErrors.error(position, "invalid arguments to '" + type.description() + 
                                     "' constructor, (expected exactly 1 argument, but found " +
                                     to_string(args.size()) + ")");
         }
@@ -907,38 +896,38 @@ std::unique_ptr<Expression> IRGenerator::convertConstructor(
                                                                this->coerce(std::move(one), type),
                                                                this->coerce(std::move(zero), 
                                                                             type)));
-        } else if (!args[0]->fType->isNumber()) {
-            fErrors.error(position, "invalid argument to '" + type->description() + 
+        } else if (!args[0]->fType.isNumber()) {
+            fErrors.error(position, "invalid argument to '" + type.description() + 
                                     "' constructor (expected a number or bool, but found '" +
-                                    args[0]->fType->description() + "')");
+                                    args[0]->fType.description() + "')");
         }
     } else {
         ASSERT(kind == Type::kVector_Kind || kind == Type::kMatrix_Kind);
         int actual = 0;
         for (size_t i = 0; i < args.size(); i++) {
-            if (args[i]->fType->kind() == Type::kVector_Kind || 
-                args[i]->fType->kind() == Type::kMatrix_Kind) {
-                int columns = args[i]->fType->columns();
-                int rows = args[i]->fType->rows();
+            if (args[i]->fType.kind() == Type::kVector_Kind || 
+                args[i]->fType.kind() == Type::kMatrix_Kind) {
+                int columns = args[i]->fType.columns();
+                int rows = args[i]->fType.rows();
                 args[i] = this->coerce(std::move(args[i]), 
-                                       type->componentType()->toCompound(columns, rows));
-                actual += args[i]->fType->rows() * args[i]->fType->columns();
-            } else if (args[i]->fType->kind() == Type::kScalar_Kind) {
+                                       type.componentType().toCompound(columns, rows));
+                actual += args[i]->fType.rows() * args[i]->fType.columns();
+            } else if (args[i]->fType.kind() == Type::kScalar_Kind) {
                 actual += 1;
-                if (type->kind() != Type::kScalar_Kind) {
-                    args[i] = this->coerce(std::move(args[i]), type->componentType());
+                if (type.kind() != Type::kScalar_Kind) {
+                    args[i] = this->coerce(std::move(args[i]), type.componentType());
                 }
             } else {
-                fErrors.error(position, "'" + args[i]->fType->description() + "' is not a valid "
-                                        "parameter to '" + type->description() + "' constructor");
+                fErrors.error(position, "'" + args[i]->fType.description() + "' is not a valid "
+                                        "parameter to '" + type.description() + "' constructor");
                 return nullptr;
             }
         }
-        int min = type->rows() * type->columns();
-        int max = type->columns() > 1 ? INT_MAX : min;
+        int min = type.rows() * type.columns();
+        int max = type.columns() > 1 ? INT_MAX : min;
         if ((actual < min || actual > max) &&
             !((kind == Type::kVector_Kind || kind == Type::kMatrix_Kind) && (actual == 1))) {
-            fErrors.error(position, "invalid arguments to '" + type->description() + 
+            fErrors.error(position, "invalid arguments to '" + type.description() + 
                                     "' constructor (expected " + to_string(min) + " scalar" + 
                                     (min == 1 ? "" : "s") + ", but found " + to_string(actual) + 
                                     ")");
@@ -956,16 +945,16 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(
     }
     switch (expression.fOperator) {
         case Token::PLUS:
-            if (!base->fType->isNumber() && base->fType->kind() != Type::kVector_Kind) {
+            if (!base->fType.isNumber() && base->fType.kind() != Type::kVector_Kind) {
                 fErrors.error(expression.fPosition, 
-                              "'+' cannot operate on '" + base->fType->description() + "'");
+                              "'+' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             return base;
         case Token::MINUS:
-            if (!base->fType->isNumber() && base->fType->kind() != Type::kVector_Kind) {
+            if (!base->fType.isNumber() && base->fType.kind() != Type::kVector_Kind) {
                 fErrors.error(expression.fPosition, 
-                              "'-' cannot operate on '" + base->fType->description() + "'");
+                              "'-' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             if (base->fKind == Expression::kIntLiteral_Kind) {
@@ -978,19 +967,19 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(
             }
             return std::unique_ptr<Expression>(new PrefixExpression(Token::MINUS, std::move(base)));
         case Token::PLUSPLUS:
-            if (!base->fType->isNumber()) {
+            if (!base->fType.isNumber()) {
                 fErrors.error(expression.fPosition, 
                               "'" + Token::OperatorName(expression.fOperator) + 
-                              "' cannot operate on '" + base->fType->description() + "'");
+                              "' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             this->markWrittenTo(*base);
             break;
         case Token::MINUSMINUS: 
-            if (!base->fType->isNumber()) {
+            if (!base->fType.isNumber()) {
                 fErrors.error(expression.fPosition, 
                               "'" + Token::OperatorName(expression.fOperator) + 
-                              "' cannot operate on '" + base->fType->description() + "'");
+                              "' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             this->markWrittenTo(*base);
@@ -999,7 +988,7 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(
             if (base->fType != kBool_Type) {
                 fErrors.error(expression.fPosition, 
                               "'" + Token::OperatorName(expression.fOperator) + 
-                              "' cannot operate on '" + base->fType->description() + "'");
+                              "' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             break;
@@ -1012,8 +1001,8 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(
 
 std::unique_ptr<Expression> IRGenerator::convertIndex(std::unique_ptr<Expression> base,
                                                       const ASTExpression& index) {
-    if (base->fType->kind() != Type::kArray_Kind && base->fType->kind() != Type::kMatrix_Kind) {
-        fErrors.error(base->fPosition, "expected array, but found '" + base->fType->description() + 
+    if (base->fType.kind() != Type::kArray_Kind && base->fType.kind() != Type::kMatrix_Kind) {
+        fErrors.error(base->fPosition, "expected array, but found '" + base->fType.description() + 
                                        "'");
         return nullptr;
     }
@@ -1030,21 +1019,21 @@ std::unique_ptr<Expression> IRGenerator::convertIndex(std::unique_ptr<Expression
 
 std::unique_ptr<Expression> IRGenerator::convertField(std::unique_ptr<Expression> base,
                                                       const std::string& field) {
-    auto fields = base->fType->fields();
+    auto fields = base->fType.fields();
     for (size_t i = 0; i < fields.size(); i++) {
         if (fields[i].fName == field) {
             return std::unique_ptr<Expression>(new FieldAccess(std::move(base), (int) i));
         }
     }
-    fErrors.error(base->fPosition, "type '" + base->fType->description() + "' does not have a "
+    fErrors.error(base->fPosition, "type '" + base->fType.description() + "' does not have a "
                                    "field named '" + field + "");
     return nullptr;
 }
 
 std::unique_ptr<Expression> IRGenerator::convertSwizzle(std::unique_ptr<Expression> base,
                                                         const std::string& fields) {
-    if (base->fType->kind() != Type::kVector_Kind) {
-        fErrors.error(base->fPosition, "cannot swizzle type '" + base->fType->description() + "'");
+    if (base->fType.kind() != Type::kVector_Kind) {
+        fErrors.error(base->fPosition, "cannot swizzle type '" + base->fType.description() + "'");
         return nullptr;
     }
     std::vector<int> swizzleComponents;
@@ -1058,7 +1047,7 @@ std::unique_ptr<Expression> IRGenerator::convertSwizzle(std::unique_ptr<Expressi
             case 'y': // fall through
             case 'g': // fall through
             case 't':
-                if (base->fType->columns() >= 2) {
+                if (base->fType.columns() >= 2) {
                     swizzleComponents.push_back(1);
                     break;
                 }
@@ -1066,7 +1055,7 @@ std::unique_ptr<Expression> IRGenerator::convertSwizzle(std::unique_ptr<Expressi
             case 'z': // fall through
             case 'b': // fall through
             case 'p': 
-                if (base->fType->columns() >= 3) {
+                if (base->fType.columns() >= 3) {
                     swizzleComponents.push_back(2);
                     break;
                 }
@@ -1074,7 +1063,7 @@ std::unique_ptr<Expression> IRGenerator::convertSwizzle(std::unique_ptr<Expressi
             case 'w': // fall through
             case 'a': // fall through
             case 'q':
-                if (base->fType->columns() >= 4) {
+                if (base->fType.columns() >= 4) {
                     swizzleComponents.push_back(3);
                     break;
                 }
@@ -1117,7 +1106,7 @@ std::unique_ptr<Expression> IRGenerator::convertSuffixExpression(
             return this->call(expression.fPosition, std::move(base), std::move(arguments));
         }
         case ASTSuffix::kField_Kind: {
-            switch (base->fType->kind()) {
+            switch (base->fType.kind()) {
                 case Type::kVector_Kind:
                     return this->convertSwizzle(std::move(base), 
                                                 ((ASTFieldSuffix&) *expression.fSuffix).fField);
@@ -1126,23 +1115,23 @@ std::unique_ptr<Expression> IRGenerator::convertSuffixExpression(
                                               ((ASTFieldSuffix&) *expression.fSuffix).fField);
                 default:
                     fErrors.error(base->fPosition, "cannot swizzle value of type '" + 
-                                                   base->fType->description() + "'");
+                                                   base->fType.description() + "'");
                     return nullptr;
             }
         }
         case ASTSuffix::kPostIncrement_Kind:
-            if (!base->fType->isNumber()) {
+            if (!base->fType.isNumber()) {
                 fErrors.error(expression.fPosition, 
-                              "'++' cannot operate on '" + base->fType->description() + "'");
+                              "'++' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             this->markWrittenTo(*base);
             return std::unique_ptr<Expression>(new PostfixExpression(std::move(base), 
                                                                      Token::PLUSPLUS));
         case ASTSuffix::kPostDecrement_Kind:
-            if (!base->fType->isNumber()) {
+            if (!base->fType.isNumber()) {
                 fErrors.error(expression.fPosition, 
-                              "'--' cannot operate on '" + base->fType->description() + "'");
+                              "'--' cannot operate on '" + base->fType.description() + "'");
                 return nullptr;
             }
             this->markWrittenTo(*base);
@@ -1167,8 +1156,8 @@ void IRGenerator::checkValid(const Expression& expr) {
     }
 }
 
-void IRGenerator::markReadFrom(std::shared_ptr<Variable> var) {
-    var->fIsReadFrom = true;
+void IRGenerator::markReadFrom(const Variable& var) {
+    var.fIsReadFrom = true;
 }
 
 static bool has_duplicates(const Swizzle& swizzle) {
@@ -1187,7 +1176,7 @@ static bool has_duplicates(const Swizzle& swizzle) {
 void IRGenerator::markWrittenTo(const Expression& expr) {
     switch (expr.fKind) {
         case Expression::kVariableReference_Kind: {
-            const Variable& var = *((VariableReference&) expr).fVariable;
+            const Variable& var = ((VariableReference&) expr).fVariable;
             if (var.fModifiers.fFlags & (Modifiers::kConst_Flag | Modifiers::kUniform_Flag)) {
                 fErrors.error(expr.fPosition, 
                               "cannot modify immutable variable '" + var.fName + "'");