SkSL function inlining

This first pass adds "one size fits all" function inlining, without any
allowances for simple functions that don't need all of this machinery
in place. Followup CLs will simplify common cases by e.g. not storing
arguments in variables if they're already variables and not wrapping
the function body in a loop when it isn't necessary.

Change-Id: I4fd8c1655ff48b9e4708bcca03a506df34ceadd9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/290127
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
diff --git a/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp b/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp
index 94d81a0..ceb17fd 100644
--- a/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp
+++ b/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp
@@ -39,9 +39,13 @@
         vVar = args.fUniformHandler->addUniform(&_outer, kFragment_GrShaderFlag, kHalf4_GrSLType,
                                                 "v");
         fragBuilder->codeAppendf(
-                "half4 inputColor = %s;\n@if (%s) {\n    inputColor = unpremul(inputColor);\n}\n%s "
-                "= %s * inputColor + %s;\n@if (%s) {\n    %s = clamp(%s, 0.0, 1.0);\n} else {\n    "
-                "%s.w = clamp(%s.w, 0.0, 1.0);\n}\n@if (%s) {\n    %s.xyz *= %s.w;\n}\n",
+                "half4 inputColor = %s;\n@if (%s) {\n    half4 inlineResult530;\n    half4 "
+                "inlineArg530_0 = inputColor;\n    do {\n        {\n            inlineResult530 = "
+                "half4(inlineArg530_0.xyz / max(inlineArg530_0.w, 9.9999997473787516e-05), "
+                "inlineArg530_0.w);\n            break;\n        }\n    } while (false);\n    "
+                "inputColor = inlineResult530;\n\n}\n%s = %s * inputColor + %s;\n@if (%s) {\n    "
+                "%s = clamp(%s, 0.0, 1.0);\n} else {\n    %s.w = clamp(%s.w, 0.0, 1.0);\n}\n@if "
+                "(%s) {\n    %s.xyz *= %s.w;\n}\n",
                 args.fInputColor, (_outer.unpremulInput ? "true" : "false"), args.fOutputColor,
                 args.fUniformHandler->getUniformCStr(mVar),
                 args.fUniformHandler->getUniformCStr(vVar),
diff --git a/src/sksl/SkSLASTNode.cpp b/src/sksl/SkSLASTNode.cpp
index 7c5a203..a74d5ab 100644
--- a/src/sksl/SkSLASTNode.cpp
+++ b/src/sksl/SkSLASTNode.cpp
@@ -34,7 +34,7 @@
             return "break";
         case Kind::kCall: {
             auto iter = this->begin();
-            String result = iter->description();
+            String result = (iter++)->description();
             result += "(";
             const char* separator = "";
             while (iter != this->end()) {
@@ -230,6 +230,11 @@
             }
             return result;
         }
+        case Kind::kWhile: {
+            return "while (" + this->begin()->description() + ") " +
+                   (this->begin() + 1)->description();
+
+        }
         default:
             SkASSERT(false);
             return "<error>";
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index fe75fe7..85f4bb6 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -1269,6 +1269,14 @@
                     break;
                 case BasicBlock::Node::kExpression_Kind:
                     offset = (*cfg.fBlocks[i].fNodes[0].expression())->fOffset;
+                    if ((*cfg.fBlocks[i].fNodes[0].expression())->fKind ==
+                        Expression::kBoolLiteral_Kind) {
+                        // Function inlining can generate do { ... } while(false) loops which always
+                        // break, so the boolean condition is considered unreachable. Since not
+                        // being able to reach a literal is a non-issue in the first place, we
+                        // don't report an error in this case.
+                        continue;
+                    }
                     break;
             }
             this->error(offset, String("unreachable"));
diff --git a/src/sksl/SkSLContext.h b/src/sksl/SkSLContext.h
index eb7659a..39968d4 100644
--- a/src/sksl/SkSLContext.h
+++ b/src/sksl/SkSLContext.h
@@ -400,6 +400,11 @@
             return "<defined>";
         }
 
+        int nodeCount() const override {
+            SkASSERT(false);
+            return 1;
+        }
+
         std::unique_ptr<Expression> clone() const override {
             return std::unique_ptr<Expression>(new Defined(fType));
         }
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index e21c32c..0aa7bbc 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -1521,11 +1521,15 @@
 }
 
 void GLSLCodeGenerator::writeBlock(const Block& b) {
-    this->writeLine("{");
-    fIndentation++;
+    if (b.fIsScope) {
+        this->writeLine("{");
+        fIndentation++;
+    }
     this->writeStatements(b.fStatements);
-    fIndentation--;
-    this->write("}");
+    if (b.fIsScope) {
+        fIndentation--;
+        this->write("}");
+    }
 }
 
 void GLSLCodeGenerator::writeIfStatement(const IfStatement& stmt) {
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 8f2b1c5..bdecf7e 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -182,7 +182,7 @@
     fSettings = nullptr;
 }
 
-std::unique_ptr<Statement> IRGenerator::convertStatement(const ASTNode& statement) {
+std::unique_ptr<Statement> IRGenerator::convertSingleStatement(const ASTNode& statement) {
     switch (statement.fKind) {
         case ASTNode::Kind::kBlock:
             return this->convertBlock(statement);
@@ -228,6 +228,24 @@
     }
 }
 
+std::unique_ptr<Statement> IRGenerator::convertStatement(const ASTNode& statement) {
+    std::vector<std::unique_ptr<Statement>> oldExtraStatements = std::move(fExtraStatements);
+    std::unique_ptr<Statement> result = this->convertSingleStatement(statement);
+    if (!result) {
+        fExtraStatements = std::move(oldExtraStatements);
+        return nullptr;
+    }
+    if (fExtraStatements.size()) {
+        fExtraStatements.push_back(std::move(result));
+        std::unique_ptr<Statement> block(new Block(-1, std::move(fExtraStatements), nullptr,
+                                                   false));
+        fExtraStatements = std::move(oldExtraStatements);
+        return block;
+    }
+    fExtraStatements = std::move(oldExtraStatements);
+    return result;
+}
+
 std::unique_ptr<Block> IRGenerator::convertBlock(const ASTNode& block) {
     SkASSERT(block.fKind == ASTNode::Kind::kBlock);
     AutoSymbolTable table(this);
@@ -496,15 +514,22 @@
     ++iter;
     std::unique_ptr<Expression> test;
     if (*iter) {
+        bool oldCanInline = fCanInline;
+        fCanInline = false;
         test = this->coerce(this->convertExpression(*iter), *fContext.fBool_Type);
+        fCanInline = oldCanInline;
         if (!test) {
             return nullptr;
         }
+
     }
     ++iter;
     std::unique_ptr<Expression> next;
     if (*iter) {
+        bool oldCanInline = fCanInline;
+        fCanInline = false;
         next = this->convertExpression(*iter);
+        fCanInline = oldCanInline;
         if (!next) {
             return nullptr;
         }
@@ -524,8 +549,11 @@
     SkASSERT(w.fKind == ASTNode::Kind::kWhile);
     AutoLoopLevel level(this);
     auto iter = w.begin();
+    bool oldCanInline = fCanInline;
+    fCanInline = false;
     std::unique_ptr<Expression> test = this->coerce(this->convertExpression(*(iter++)),
                                                     *fContext.fBool_Type);
+    fCanInline = oldCanInline;
     if (!test) {
         return nullptr;
     }
@@ -545,8 +573,11 @@
     if (!statement) {
         return nullptr;
     }
+    bool oldCanInline = fCanInline;
+    fCanInline = false;
     std::unique_ptr<Expression> test = this->coerce(this->convertExpression(*(iter++)),
                                                     *fContext.fBool_Type);
+    fCanInline = oldCanInline;
     if (!test) {
         return nullptr;
     }
@@ -884,7 +915,7 @@
                             return;
                         }
                     }
-                    if (other->fDefined && !other->fBuiltin) {
+                    if (other->fDefinition && !other->fBuiltin) {
                         fErrors.error(f.fOffset, "duplicate definition of " +
                                                  other->declaration());
                     }
@@ -907,7 +938,6 @@
         // compile body
         SkASSERT(!fCurrentFunction);
         fCurrentFunction = decl;
-        decl->fDefined = true;
         std::shared_ptr<SymbolTable> old = fSymbolTable;
         AutoSymbolTable table(this);
         if (fd.fName == "main" && fKind == Program::kPipelineStage_Kind) {
@@ -940,6 +970,7 @@
         }
         std::unique_ptr<FunctionDefinition> result(new FunctionDefinition(f.fOffset, *decl,
                                                                           std::move(body)));
+        decl->fDefinition = result.get();
         result->fSource = &f;
         fProgramElements->push_back(std::move(result));
     }
@@ -1715,7 +1746,15 @@
     if (!left) {
         return nullptr;
     }
+    Token::Kind op = expression.getToken().fKind;
+    bool oldCanInline = fCanInline;
+    if (op == Token::Kind::TK_LOGICALAND || op == Token::Kind::TK_LOGICALOR) {
+        // can't inline the right side of a short-circuiting boolean, because our inlining
+        // approach runs things out of order
+        fCanInline = false;
+    }
     std::unique_ptr<Expression> right = this->convertExpression(*(iter++));
+    fCanInline = oldCanInline;
     if (!right) {
         return nullptr;
     }
@@ -1734,7 +1773,6 @@
     } else {
         rawRightType = &right->fType;
     }
-    Token::Kind op = expression.getToken().fKind;
     if (!determine_binary_type(fContext, op, *rawLeftType, *rawRightType, &leftType, &rightType,
                                &resultType, !Compiler::IsAssignment(op))) {
         fErrors.error(expression.fOffset, String("type mismatch: '") +
@@ -1811,6 +1849,324 @@
                                                              std::move(ifFalse)));
 }
 
+std::unique_ptr<Expression> IRGenerator::inlineExpression(int offset,
+                                                          std::map<const Variable*,
+                                                                   const Variable*>* varMap,
+                                                          const Expression& expression) {
+    auto expr = [&](const std::unique_ptr<Expression>& e) {
+        if (e) {
+            return this->inlineExpression(offset, varMap, *e);
+        }
+        return std::unique_ptr<Expression>(nullptr);
+    };
+    switch (expression.fKind) {
+        case Expression::kBinary_Kind: {
+            const BinaryExpression& b = (const BinaryExpression&) expression;
+            return std::unique_ptr<Expression>(new BinaryExpression(offset,
+                                                                    expr(b.fLeft),
+                                                                    b.fOperator,
+                                                                    expr(b.fRight),
+                                                                    b.fType));
+        }
+        case Expression::kBoolLiteral_Kind:
+        case Expression::kIntLiteral_Kind:
+        case Expression::kFloatLiteral_Kind:
+        case Expression::kNullLiteral_Kind:
+            return expression.clone();
+        case Expression::kConstructor_Kind: {
+            const Constructor& c = (const Constructor&) expression;
+            std::vector<std::unique_ptr<Expression>> args;
+            for (const auto& arg : c.fArguments) {
+                args.push_back(expr(arg));
+            }
+            return std::unique_ptr<Expression>(new Constructor(offset, c.fType, std::move(args)));
+        }
+        case Expression::kExternalFunctionCall_Kind: {
+            const ExternalFunctionCall& e = (const ExternalFunctionCall&) expression;
+            std::vector<std::unique_ptr<Expression>> args;
+            for (const auto& arg : e.fArguments) {
+                args.push_back(expr(arg));
+            }
+            return std::unique_ptr<Expression>(new ExternalFunctionCall(offset, e.fType,
+                                                                        e.fFunction,
+                                                                        std::move(args)));
+        }
+        case Expression::kExternalValue_Kind:
+            return expression.clone();
+        case Expression::kFieldAccess_Kind: {
+            const FieldAccess& f = (const FieldAccess&) expression;
+            return std::unique_ptr<Expression>(new FieldAccess(expr(f.fBase), f.fFieldIndex,
+                                                               f.fOwnerKind));
+        }
+        case Expression::kFunctionCall_Kind: {
+            const FunctionCall& c = (const FunctionCall&) expression;
+            std::vector<std::unique_ptr<Expression>> args;
+            for (const auto& arg : c.fArguments) {
+                args.push_back(expr(arg));
+            }
+            return std::unique_ptr<Expression>(new FunctionCall(offset, c.fType, c.fFunction,
+                                                                std::move(args)));
+        }
+        case Expression::kIndex_Kind: {
+            const IndexExpression& idx = (const IndexExpression&) expression;
+            return std::unique_ptr<Expression>(new IndexExpression(fContext, expr(idx.fBase),
+                                                                   expr(idx.fIndex)));
+        }
+        case Expression::kPrefix_Kind: {
+            const PrefixExpression& p = (const PrefixExpression&) expression;
+            return std::unique_ptr<Expression>(new PrefixExpression(p.fOperator, expr(p.fOperand)));
+        }
+        case Expression::kPostfix_Kind: {
+            const PostfixExpression& p = (const PostfixExpression&) expression;
+            return std::unique_ptr<Expression>(new PostfixExpression(expr(p.fOperand),
+                                                                     p.fOperator));
+        }
+        case Expression::kSetting_Kind:
+            return expression.clone();
+        case Expression::kSwizzle_Kind: {
+            const Swizzle& s = (const Swizzle&) expression;
+            return std::unique_ptr<Expression>(new Swizzle(fContext, expr(s.fBase), s.fComponents));
+        }
+        case Expression::kTernary_Kind: {
+            const TernaryExpression& t = (const TernaryExpression&) expression;
+            return std::unique_ptr<Expression>(new TernaryExpression(offset, expr(t.fTest),
+                                                                     expr(t.fIfTrue),
+                                                                     expr(t.fIfFalse)));
+        }
+        case Expression::kVariableReference_Kind: {
+            const VariableReference& v = (const VariableReference&) expression;
+            auto found = varMap->find(&v.fVariable);
+            if (found != varMap->end()) {
+                return std::unique_ptr<Expression>(new VariableReference(offset,
+                                                                         *found->second,
+                                                                         v.fRefKind));
+            }
+            return v.clone();
+        }
+        default:
+            SkASSERT(false);
+            return nullptr;
+    }
+}
+
+std::unique_ptr<Statement> IRGenerator::inlineStatement(int offset,
+                                                        std::map<const Variable*,
+                                                                 const Variable*>* varMap,
+                                                        const Variable* returnVar,
+                                                        const Statement& statement) {
+    auto stmt = [&](const std::unique_ptr<Statement>& s) {
+        if (s) {
+            return this->inlineStatement(offset, varMap, returnVar, *s);
+        }
+        return std::unique_ptr<Statement>(nullptr);
+    };
+    auto stmts = [&](const std::vector<std::unique_ptr<Statement>>& ss) {
+        std::vector<std::unique_ptr<Statement>> result;
+        for (const auto& s : ss) {
+            result.push_back(stmt(s));
+        }
+        return result;
+    };
+    auto expr = [&](const std::unique_ptr<Expression>& e) {
+        if (e) {
+            return this->inlineExpression(offset, varMap, *e);
+        }
+        return std::unique_ptr<Expression>(nullptr);
+    };
+    switch (statement.fKind) {
+        case Statement::kBlock_Kind: {
+            const Block& b = (const Block&) statement;
+            return std::unique_ptr<Statement>(new Block(offset, stmts(b.fStatements), b.fSymbols,
+                                                        b.fIsScope));
+        }
+
+        case Statement::kBreak_Kind:
+        case Statement::kContinue_Kind:
+        case Statement::kDiscard_Kind:
+            return statement.clone();
+
+        case Statement::kDo_Kind: {
+            const DoStatement& d = (const DoStatement&) statement;
+            return std::unique_ptr<Statement>(new DoStatement(offset,
+                                                              stmt(d.fStatement),
+                                                              expr(d.fTest)));
+        }
+        case Statement::kExpression_Kind: {
+            const ExpressionStatement& e = (const ExpressionStatement&) statement;
+            return std::unique_ptr<Statement>(new ExpressionStatement(expr(e.fExpression)));
+        }
+        case Statement::kFor_Kind: {
+            const ForStatement& f = (const ForStatement&) statement;
+            // need to ensure initializer is evaluated first so that we've already remapped its
+            // declarations by the time we evaluate test & next
+            std::unique_ptr<Statement> initializer = stmt(f.fInitializer);
+            return std::unique_ptr<Statement>(new ForStatement(offset, std::move(initializer),
+                                                               expr(f.fTest), expr(f.fNext),
+                                                               stmt(f.fStatement), f.fSymbols));
+        }
+        case Statement::kIf_Kind: {
+            const IfStatement& i = (const IfStatement&) statement;
+            return std::unique_ptr<Statement>(new IfStatement(offset, i.fIsStatic, expr(i.fTest),
+                                                               stmt(i.fIfTrue), stmt(i.fIfFalse)));
+        }
+        case Statement::kNop_Kind:
+            return statement.clone();
+        case Statement::kReturn_Kind: {
+            const ReturnStatement& r = (const ReturnStatement&) statement;
+            if (r.fExpression) {
+                std::vector<std::unique_ptr<Statement>> block;
+                block.emplace_back(new ExpressionStatement(
+                                    std::unique_ptr<Expression>(new BinaryExpression(offset,
+                                        std::unique_ptr<Expression>(new VariableReference(
+                                                                offset,
+                                                                *returnVar,
+                                                                VariableReference::kWrite_RefKind)),
+                                        Token::Kind::TK_EQ,
+                                        expr(r.fExpression),
+                                        returnVar->fType))));
+                block.emplace_back(new BreakStatement(offset));
+                return std::unique_ptr<Statement>(new Block(offset, std::move(block)));
+            } else {
+                return std::unique_ptr<Statement>(new BreakStatement(offset));
+            }
+        }
+        case Statement::kSwitch_Kind: {
+            const SwitchStatement& ss = (const SwitchStatement&) statement;
+            std::vector<std::unique_ptr<SwitchCase>> cases;
+            for (const auto& sc : ss.fCases) {
+                cases.emplace_back(new SwitchCase(offset, expr(sc->fValue),
+                                                  stmts(sc->fStatements)));
+            }
+            return std::unique_ptr<Statement>(new SwitchStatement(offset, ss.fIsStatic,
+                                                                  expr(ss.fValue),
+                                                                  std::move(cases),
+                                                                  ss.fSymbols));
+        }
+        case Statement::kVarDeclaration_Kind: {
+            const VarDeclaration& decl = (const VarDeclaration&) statement;
+            std::vector<std::unique_ptr<Expression>> sizes;
+            for (const auto& size : decl.fSizes) {
+                sizes.push_back(expr(size));
+            }
+            std::unique_ptr<Expression> initialValue = expr(decl.fValue);
+            const Variable* old = decl.fVar;
+            Variable* clone = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
+                                                   new Variable(offset, old->fModifiers, old->fName,
+                                                                  old->fType, old->fStorage,
+                                                                  initialValue.get())));
+            (*varMap)[old] = clone;
+            return std::unique_ptr<Statement>(new VarDeclaration(clone, std::move(sizes),
+                                                                 std::move(initialValue)));
+        }
+        case Statement::kVarDeclarations_Kind: {
+            const VarDeclarations& decls = *((VarDeclarationsStatement&) statement).fDeclaration;
+            std::vector<std::unique_ptr<VarDeclaration>> vars;
+            for (const auto& var : decls.fVars) {
+                vars.emplace_back((VarDeclaration*) stmt(var).release());
+            }
+            return std::unique_ptr<Statement>(new VarDeclarationsStatement(
+                    std::unique_ptr<VarDeclarations>(new VarDeclarations(offset, &decls.fBaseType,
+                                                                         std::move(vars)))));
+        }
+        case Statement::kWhile_Kind: {
+            const WhileStatement& w = (const WhileStatement&) statement;
+            return std::unique_ptr<Statement>(new WhileStatement(offset,
+                                                              expr(w.fTest),
+                                                              stmt(w.fStatement)));
+        }
+        default:
+            SkASSERT(false);
+            return nullptr;
+    }
+}
+
+std::unique_ptr<Expression> IRGenerator::inlineCall(
+                                               int offset,
+                                               const FunctionDefinition& function,
+                                               std::vector<std::unique_ptr<Expression>> arguments) {
+    // Inlining is more complicated here than in a typical compiler, because we have to have a
+    // high-level IR and can't just drop statements into the middle of an expression or even use
+    // gotos.
+    //
+    // Since we can't insert statements into an expression, we run the inline function as extra
+    // statements before the statement we're currently processing, relying on a lack of execution
+    // order guarantees. Since we can't use gotos (which are normally used to replace return
+    // statements), we wrap the whole function in a loop and use break statements to jump to the
+    // end.
+    std::vector<std::unique_ptr<Statement>> body;
+    Variable* resultVar;
+    if (function.fDeclaration.fReturnType != *fContext.fVoid_Type) {
+        std::unique_ptr<String> name(new String());
+        name->appendf("inlineResult%d", offset);
+        String* namePtr = (String*) fSymbolTable->takeOwnership(std::move(name));
+        resultVar = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
+                                                     new Variable(-1, Modifiers(), namePtr->c_str(),
+                                                                  function.fDeclaration.fReturnType,
+                                                                  Variable::kLocal_Storage,
+                                                                  nullptr)));
+        std::vector<std::unique_ptr<VarDeclaration>> variables;
+        variables.emplace_back(new VarDeclaration(resultVar, {}, nullptr));
+        fExtraStatements.emplace_back(new VarDeclarationsStatement(
+                        std::unique_ptr<VarDeclarations>(new VarDeclarations(offset,
+                                                                           &resultVar->fType,
+                                                                           std::move(variables)))));
+
+    } else {
+        resultVar = nullptr;
+    }
+    std::map<const Variable*, const Variable*> varMap;
+    // create variables to hold the arguments and assign the arguments to them
+    for (int i = 0; i < (int) arguments.size(); ++i) {
+        std::unique_ptr<String> argName(new String());
+        argName->appendf("inlineArg%d_%d", offset, i);
+        String* argNamePtr = (String*) fSymbolTable->takeOwnership(std::move(argName));
+        Variable* argVar = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
+                                                      new Variable(-1, Modifiers(),
+                                                                   argNamePtr->c_str(),
+                                                                   arguments[i]->fType,
+                                                                   Variable::kLocal_Storage,
+                                                                   arguments[i].get())));
+        varMap[function.fDeclaration.fParameters[i]] = argVar;
+        std::vector<std::unique_ptr<VarDeclaration>> vars;
+        if (function.fDeclaration.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag) {
+            vars.emplace_back(new VarDeclaration(argVar, {}, arguments[i]->clone()));
+        } else {
+            vars.emplace_back(new VarDeclaration(argVar, {}, std::move(arguments[i])));
+        }
+        fExtraStatements.emplace_back(new VarDeclarationsStatement(
+                           std::unique_ptr<VarDeclarations>(new VarDeclarations(offset,
+                                                                                &argVar->fType,
+                                                                                std::move(vars)))));
+    }
+    for (const auto& s : ((Block&) *function.fBody).fStatements) {
+        body.push_back(this->inlineStatement(offset, &varMap, resultVar, *s));
+    }
+    fExtraStatements.emplace_back(new DoStatement(-1,
+                                std::unique_ptr<Statement>(new Block(-1, std::move(body))),
+                                std::unique_ptr<Expression>(new BoolLiteral(fContext, -1, false))));
+    // copy the values of out parameters into their destinations
+    for (size_t i = 0; i < arguments.size(); ++i) {
+        const Variable* p = function.fDeclaration.fParameters[i];
+        if (p->fModifiers.fFlags & Modifiers::kOut_Flag) {
+            std::unique_ptr<Expression> varRef(new VariableReference(offset, *varMap[p]));
+            fExtraStatements.emplace_back(new ExpressionStatement(
+                    std::unique_ptr<Expression>(new BinaryExpression(offset,
+                                                                     arguments[i]->clone(),
+                                                                     Token::Kind::TK_EQ,
+                                                                     std::move(varRef),
+                                                                     arguments[i]->fType))));
+        }
+    }
+    if (function.fDeclaration.fReturnType != *fContext.fVoid_Type) {
+        return std::unique_ptr<Expression>(new VariableReference(-1, *resultVar));
+    } else {
+        // it's a void function, so it doesn't actually result in anything, but we have to return
+        // something non-null as a standin
+        return std::unique_ptr<Expression>(new BoolLiteral(fContext, -1, false));
+    }
+}
+
+
 std::unique_ptr<Expression> IRGenerator::call(int offset,
                                               const FunctionDeclaration& function,
                                               std::vector<std::unique_ptr<Expression>> arguments) {
@@ -1835,7 +2191,7 @@
         fErrors.error(offset, msg);
         return nullptr;
     }
-    if (fKind == Program::kPipelineStage_Kind && !function.fDefined && !function.fBuiltin) {
+    if (fKind == Program::kPipelineStage_Kind && !function.fDefinition && !function.fBuiltin) {
         String msg = "call to undefined function '" + function.fName + "'";
         fErrors.error(offset, msg);
         return nullptr;
@@ -1866,6 +2222,9 @@
                              VariableReference::kPointer_RefKind);
         }
     }
+    if (fCanInline && function.fDefinition && function.fDefinition->canBeInlined()) {
+        return this->inlineCall(offset, *function.fDefinition, std::move(arguments));
+    }
     return std::unique_ptr<FunctionCall>(new FunctionCall(offset, *returnType, function,
                                                           std::move(arguments)));
 }
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index 2895a34..4861fcf 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -83,11 +83,21 @@
     std::unique_ptr<VarDeclarations> convertVarDeclarations(const ASTNode& decl,
                                                             Variable::Storage storage);
     void convertFunction(const ASTNode& f);
+    std::unique_ptr<Statement> convertSingleStatement(const ASTNode& statement);
     std::unique_ptr<Statement> convertStatement(const ASTNode& statement);
     std::unique_ptr<Expression> convertExpression(const ASTNode& expression);
     std::unique_ptr<ModifiersDeclaration> convertModifiersDeclaration(const ASTNode& m);
 
     const Type* convertType(const ASTNode& type);
+    std::unique_ptr<Expression> inlineExpression(int offset,
+                                                 std::map<const Variable*, const Variable*>* varMap,
+                                                 const Expression& expression);
+    std::unique_ptr<Statement> inlineStatement(int offset,
+                                               std::map<const Variable*, const Variable*>* varMap,
+                                               const Variable* returnVar,
+                                               const Statement& statement);
+    std::unique_ptr<Expression> inlineCall(int offset, const FunctionDefinition& function,
+                                           std::vector<std::unique_ptr<Expression>> arguments);
     std::unique_ptr<Expression> call(int offset,
                                      const FunctionDeclaration& function,
                                      std::vector<std::unique_ptr<Expression>> arguments);
@@ -156,6 +166,9 @@
     std::unordered_map<String, Program::Settings::Value> fCapsMap;
     std::shared_ptr<SymbolTable> fRootSymbolTable;
     std::shared_ptr<SymbolTable> fSymbolTable;
+    // additional statements that need to be inserted before the one that convertStatement is
+    // currently working on
+    std::vector<std::unique_ptr<Statement>> fExtraStatements;
     // Symbols which have definitions in the include files. The bool tells us whether this
     // intrinsic has been included already.
     std::map<String, std::pair<std::unique_ptr<ProgramElement>, bool>>* fIntrinsics = nullptr;
@@ -168,6 +181,7 @@
     Variable* fRTAdjust;
     Variable* fRTAdjustInterfaceBlock;
     int fRTAdjustFieldIndex;
+    bool fCanInline = true;
 
     friend class AutoSymbolTable;
     friend class AutoLoopLevel;
diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp
index 099ab4c..2b222d8 100644
--- a/src/sksl/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -1164,11 +1164,15 @@
 }
 
 void MetalCodeGenerator::writeBlock(const Block& b) {
-    this->writeLine("{");
-    fIndentation++;
+    if (b.fIsScope) {
+        this->writeLine("{");
+        fIndentation++;
+    }
     this->writeStatements(b.fStatements);
-    fIndentation--;
-    this->write("}");
+    if (b.fIsScope) {
+        fIndentation--;
+        this->write("}");
+    }
 }
 
 void MetalCodeGenerator::writeIfStatement(const IfStatement& stmt) {
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index b01bd95..6726a8f 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2987,14 +2987,6 @@
 }
 
 void SPIRVCodeGenerator::writeDoStatement(const DoStatement& d, OutputStream& out) {
-    // We believe the do loop code below will work, but Skia doesn't actually use them and
-    // adequately testing this code in the absence of Skia exercising it isn't straightforward. For
-    // the time being, we just fail with an error due to the lack of testing. If you encounter this
-    // message, simply remove the error call below to see whether our do loop support actually
-    // works.
-    fErrors.error(d.fOffset, "internal error: do loop support has been disabled in SPIR-V, see "
-                  "SkSLSPIRVCodeGenerator.cpp for details");
-
     SpvId header = this->nextId();
     SpvId start = this->nextId();
     SpvId next = this->nextId();
diff --git a/src/sksl/SkSLSectionAndParameterHelper.cpp b/src/sksl/SkSLSectionAndParameterHelper.cpp
index a897386..6d99548 100644
--- a/src/sksl/SkSLSectionAndParameterHelper.cpp
+++ b/src/sksl/SkSLSectionAndParameterHelper.cpp
@@ -199,9 +199,9 @@
         }
         case Statement::kFor_Kind: {
             const ForStatement& f = (const ForStatement&) s;
-            return this->hasCoordOverrides(*f.fInitializer, fp) ||
-                   this->hasCoordOverrides(*f.fTest, fp) ||
-                   this->hasCoordOverrides(*f.fNext, fp) ||
+            return (f.fInitializer ? this->hasCoordOverrides(*f.fInitializer, fp) : 0) ||
+                   (f.fTest        ? this->hasCoordOverrides(*f.fTest, fp)        : 0) ||
+                   (f.fNext        ? this->hasCoordOverrides(*f.fNext, fp)        : 0) ||
                    this->hasCoordOverrides(*f.fStatement, fp);
         }
         case Statement::kWhile_Kind: {
@@ -228,7 +228,6 @@
         case Statement::kBreak_Kind:
         case Statement::kContinue_Kind:
         case Statement::kDiscard_Kind:
-        case Statement::kGroup_Kind:
         case Statement::kNop_Kind:
             return false;
     }
@@ -321,6 +320,13 @@
     return SampleMatrix();
 }
 
+SampleMatrix SectionAndParameterHelper::getMatrix(const Expression* e, const Variable& fp) {
+    if (e) {
+        return this->getMatrix(*e, fp);
+    }
+    return SampleMatrix();
+}
+
 SampleMatrix SectionAndParameterHelper::getMatrix(const Statement& s, const Variable& fp) {
     switch (s.fKind) {
         case Statement::kBlock_Kind: {
@@ -361,9 +367,9 @@
         }
         case Statement::kFor_Kind: {
             const ForStatement& f = (const ForStatement&) s;
-            return this->getMatrix(*f.fInitializer, fp).merge(
-                   this->getMatrix(*f.fTest, fp).merge(
-                   this->getMatrix(*f.fNext, fp).merge(
+            return this->getMatrix(f.fInitializer.get(), fp).merge(
+                   this->getMatrix(f.fTest.get(), fp).merge(
+                   this->getMatrix(f.fNext.get(), fp).merge(
                    this->getMatrix(*f.fStatement, fp))));
         }
         case Statement::kWhile_Kind: {
@@ -387,7 +393,6 @@
         case Statement::kBreak_Kind:
         case Statement::kContinue_Kind:
         case Statement::kDiscard_Kind:
-        case Statement::kGroup_Kind:
         case Statement::kNop_Kind:
             return SampleMatrix();
     }
@@ -395,4 +400,12 @@
     return SampleMatrix();
 }
 
+SampleMatrix SectionAndParameterHelper::getMatrix(const Statement* s, const Variable& fp) {
+    if (s) {
+        return this->getMatrix(*s, fp);
+    }
+    return SampleMatrix();
+}
+
+
 }
diff --git a/src/sksl/SkSLSectionAndParameterHelper.h b/src/sksl/SkSLSectionAndParameterHelper.h
index d52302a..4bc7a47 100644
--- a/src/sksl/SkSLSectionAndParameterHelper.h
+++ b/src/sksl/SkSLSectionAndParameterHelper.h
@@ -122,6 +122,10 @@
 
     SampleMatrix getMatrix(const Expression& e, const Variable& fp);
 
+    SampleMatrix getMatrix(const Statement* s, const Variable& fp);
+
+    SampleMatrix getMatrix(const Expression* e, const Variable& fp);
+
     SampleMatrix getMatrix(const ProgramElement& p, const Variable& fp);
 
     const Program& fProgram;
diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h
index 9fc8683..f4a48e9 100644
--- a/src/sksl/ir/SkSLBinaryExpression.h
+++ b/src/sksl/ir/SkSLBinaryExpression.h
@@ -45,6 +45,10 @@
         return fLeft->hasProperty(property) || fRight->hasProperty(property);
     }
 
+    int nodeCount() const override {
+        return 1 + fLeft->nodeCount() + fRight->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new BinaryExpression(fOffset, fLeft->clone(), fOperator,
                                                                 fRight->clone(), fType));
diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h
index 8a4449a..b9fdf29 100644
--- a/src/sksl/ir/SkSLBlock.h
+++ b/src/sksl/ir/SkSLBlock.h
@@ -18,10 +18,11 @@
  */
 struct Block : public Statement {
     Block(int offset, std::vector<std::unique_ptr<Statement>> statements,
-          const std::shared_ptr<SymbolTable> symbols = nullptr)
+          const std::shared_ptr<SymbolTable> symbols = nullptr, bool isScope = true)
     : INHERITED(offset, kBlock_Kind)
     , fSymbols(std::move(symbols))
-    , fStatements(std::move(statements)) {}
+    , fStatements(std::move(statements))
+    , fIsScope(isScope) {}
 
     bool isEmpty() const override {
         for (const auto& s : fStatements) {
@@ -32,12 +33,21 @@
         return true;
     }
 
+    int nodeCount() const override {
+        int result = 1;
+        for (const auto& s : fStatements) {
+            result += s->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         std::vector<std::unique_ptr<Statement>> cloned;
         for (const auto& s : fStatements) {
             cloned.push_back(s->clone());
         }
-        return std::unique_ptr<Statement>(new Block(fOffset, std::move(cloned), fSymbols));
+        return std::unique_ptr<Statement>(new Block(fOffset, std::move(cloned), fSymbols,
+                                                    fIsScope));
     }
 
     String description() const override {
@@ -54,6 +64,10 @@
     // because destroying statements can modify reference counts in symbols
     const std::shared_ptr<SymbolTable> fSymbols;
     std::vector<std::unique_ptr<Statement>> fStatements;
+    // if isScope is false, this is just a group of statements rather than an actual language-level
+    // block. This allows us to pass around multiple statements as if they were a single unit, with
+    // no semantic impact.
+    bool fIsScope;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLBoolLiteral.h b/src/sksl/ir/SkSLBoolLiteral.h
index 74f48ad..85dd8c5 100644
--- a/src/sksl/ir/SkSLBoolLiteral.h
+++ b/src/sksl/ir/SkSLBoolLiteral.h
@@ -38,6 +38,10 @@
         return fValue == b.fValue;
     }
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new BoolLiteral(fOffset, fValue, &fType));
     }
diff --git a/src/sksl/ir/SkSLBreakStatement.h b/src/sksl/ir/SkSLBreakStatement.h
index ae0c198..daedece 100644
--- a/src/sksl/ir/SkSLBreakStatement.h
+++ b/src/sksl/ir/SkSLBreakStatement.h
@@ -20,6 +20,10 @@
     BreakStatement(int offset)
     : INHERITED(offset, kBreak_Kind) {}
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new BreakStatement(fOffset));
     }
diff --git a/src/sksl/ir/SkSLConstructor.h b/src/sksl/ir/SkSLConstructor.h
index 524a6ad..cdd6f84 100644
--- a/src/sksl/ir/SkSLConstructor.h
+++ b/src/sksl/ir/SkSLConstructor.h
@@ -59,6 +59,14 @@
         return false;
     }
 
+    int nodeCount() const override {
+        int result = 1;
+        for (const auto& a : fArguments) {
+            result += a->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         std::vector<std::unique_ptr<Expression>> cloned;
         for (const auto& arg : fArguments) {
diff --git a/src/sksl/ir/SkSLContinueStatement.h b/src/sksl/ir/SkSLContinueStatement.h
index ecd9c3f..1e01ac8 100644
--- a/src/sksl/ir/SkSLContinueStatement.h
+++ b/src/sksl/ir/SkSLContinueStatement.h
@@ -20,6 +20,10 @@
     ContinueStatement(int offset)
     : INHERITED(offset, kContinue_Kind) {}
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new ContinueStatement(fOffset));
     }
diff --git a/src/sksl/ir/SkSLDiscardStatement.h b/src/sksl/ir/SkSLDiscardStatement.h
index 40f625c..8cc7e3f 100644
--- a/src/sksl/ir/SkSLDiscardStatement.h
+++ b/src/sksl/ir/SkSLDiscardStatement.h
@@ -20,6 +20,10 @@
     DiscardStatement(int offset)
     : INHERITED(offset, kDiscard_Kind) {}
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new DiscardStatement(fOffset));
     }
diff --git a/src/sksl/ir/SkSLDoStatement.h b/src/sksl/ir/SkSLDoStatement.h
index 5cab5c8..474aac7 100644
--- a/src/sksl/ir/SkSLDoStatement.h
+++ b/src/sksl/ir/SkSLDoStatement.h
@@ -23,6 +23,10 @@
     , fStatement(std::move(statement))
     , fTest(std::move(test)) {}
 
+    int nodeCount() const override {
+        return 1 + fStatement->nodeCount() + fTest->nodeCount();
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new DoStatement(fOffset, fStatement->clone(),
                                                           fTest->clone()));
diff --git a/src/sksl/ir/SkSLExpressionStatement.h b/src/sksl/ir/SkSLExpressionStatement.h
index 80a8d31..1732bbb 100644
--- a/src/sksl/ir/SkSLExpressionStatement.h
+++ b/src/sksl/ir/SkSLExpressionStatement.h
@@ -21,6 +21,10 @@
     : INHERITED(expression->fOffset, kExpression_Kind)
     , fExpression(std::move(expression)) {}
 
+    int nodeCount() const override {
+        return 1 + fExpression->nodeCount();
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new ExpressionStatement(fExpression->clone()));
     }
diff --git a/src/sksl/ir/SkSLExternalFunctionCall.h b/src/sksl/ir/SkSLExternalFunctionCall.h
index deb1f4d..bf70561 100644
--- a/src/sksl/ir/SkSLExternalFunctionCall.h
+++ b/src/sksl/ir/SkSLExternalFunctionCall.h
@@ -36,6 +36,14 @@
         return false;
     }
 
+    int nodeCount() const override {
+        int result = 1;
+        for (const auto& a : fArguments) {
+            result += a->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         std::vector<std::unique_ptr<Expression>> cloned;
         for (const auto& arg : fArguments) {
diff --git a/src/sksl/ir/SkSLExternalValueReference.h b/src/sksl/ir/SkSLExternalValueReference.h
index aa0ec18..868c6c9 100644
--- a/src/sksl/ir/SkSLExternalValueReference.h
+++ b/src/sksl/ir/SkSLExternalValueReference.h
@@ -25,6 +25,10 @@
         return property == Property::kSideEffects;
     }
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     String description() const override {
         return String(fValue->fName);
     }
diff --git a/src/sksl/ir/SkSLFieldAccess.h b/src/sksl/ir/SkSLFieldAccess.h
index 59f4c16..6cba566 100644
--- a/src/sksl/ir/SkSLFieldAccess.h
+++ b/src/sksl/ir/SkSLFieldAccess.h
@@ -35,6 +35,10 @@
         return fBase->hasProperty(property);
     }
 
+    int nodeCount() const override {
+        return 1 + fBase->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new FieldAccess(fBase->clone(), fFieldIndex,
                                                            fOwnerKind));
diff --git a/src/sksl/ir/SkSLFloatLiteral.h b/src/sksl/ir/SkSLFloatLiteral.h
index 79f5dcf..9d81771 100644
--- a/src/sksl/ir/SkSLFloatLiteral.h
+++ b/src/sksl/ir/SkSLFloatLiteral.h
@@ -53,6 +53,10 @@
         return fValue;
     }
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new FloatLiteral(fOffset, fValue, &fType));
     }
diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h
index 4906e19..29c61bf 100644
--- a/src/sksl/ir/SkSLForStatement.h
+++ b/src/sksl/ir/SkSLForStatement.h
@@ -28,16 +28,36 @@
     , fNext(std::move(next))
     , fStatement(std::move(statement)) {}
 
+    int nodeCount() const override {
+        int result = 1;
+        if (fInitializer) {
+            result += fInitializer->nodeCount();
+        }
+        if (fTest) {
+            result += fTest->nodeCount();
+        }
+        if (fNext) {
+            result += fNext->nodeCount();
+        }
+        result += fStatement->nodeCount();
+        return result;
+    }
+
     std::unique_ptr<Statement> clone() const override {
-        return std::unique_ptr<Statement>(new ForStatement(fOffset, fInitializer->clone(),
-                                                           fTest->clone(), fNext->clone(),
-                                                           fStatement->clone(), fSymbols));
+        return std::unique_ptr<Statement>(new ForStatement(fOffset,
+                                                     fInitializer ? fInitializer->clone() : nullptr,
+                                                     fTest ? fTest->clone() : nullptr,
+                                                     fNext ? fNext->clone() : nullptr,
+                                                     fStatement->clone(),
+                                                     fSymbols));
     }
 
     String description() const override {
         String result("for (");
         if (fInitializer) {
             result += fInitializer->description();
+        } else {
+            result += ";";
         }
         result += " ";
         if (fTest) {
diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h
index 296aa0b..f575879 100644
--- a/src/sksl/ir/SkSLFunctionCall.h
+++ b/src/sksl/ir/SkSLFunctionCall.h
@@ -36,6 +36,14 @@
         return false;
     }
 
+    int nodeCount() const override {
+        int result = 1;
+        for (const auto& a : fArguments) {
+            result += a->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         std::vector<std::unique_ptr<Expression>> cloned;
         for (const auto& arg : fArguments) {
diff --git a/src/sksl/ir/SkSLFunctionDeclaration.h b/src/sksl/ir/SkSLFunctionDeclaration.h
index f2a3e0f..0301ed7 100644
--- a/src/sksl/ir/SkSLFunctionDeclaration.h
+++ b/src/sksl/ir/SkSLFunctionDeclaration.h
@@ -17,6 +17,8 @@
 
 namespace SkSL {
 
+struct FunctionDefinition;
+
 /**
  * A function declaration (not a definition -- does not contain a body).
  */
@@ -24,7 +26,7 @@
     FunctionDeclaration(int offset, Modifiers modifiers, StringFragment name,
                         std::vector<const Variable*> parameters, const Type& returnType)
     : INHERITED(offset, kFunctionDeclaration_Kind, std::move(name))
-    , fDefined(false)
+    , fDefinition(nullptr)
     , fBuiltin(false)
     , fModifiers(modifiers)
     , fParameters(std::move(parameters))
@@ -107,7 +109,7 @@
         return true;
     }
 
-    mutable bool fDefined;
+    mutable FunctionDefinition* fDefinition;
     bool fBuiltin;
     Modifiers fModifiers;
     const std::vector<const Variable*> fParameters;
diff --git a/src/sksl/ir/SkSLFunctionDefinition.h b/src/sksl/ir/SkSLFunctionDefinition.h
index 511a0f8..4bcadcf 100644
--- a/src/sksl/ir/SkSLFunctionDefinition.h
+++ b/src/sksl/ir/SkSLFunctionDefinition.h
@@ -26,6 +26,11 @@
     , fDeclaration(declaration)
     , fBody(std::move(body)) {}
 
+    bool canBeInlined() const {
+        static const int INLINE_THRESHOLD = 50; // chosen arbitrarily, feel free to adjust
+        return fBody->nodeCount() < INLINE_THRESHOLD;
+    }
+
     std::unique_ptr<ProgramElement> clone() const override {
         return std::unique_ptr<ProgramElement>(new FunctionDefinition(fOffset, fDeclaration,
                                                                       fBody->clone()));
diff --git a/src/sksl/ir/SkSLIRNode.h b/src/sksl/ir/SkSLIRNode.h
index ca9ea99..cace825 100644
--- a/src/sksl/ir/SkSLIRNode.h
+++ b/src/sksl/ir/SkSLIRNode.h
@@ -23,6 +23,12 @@
 
     virtual ~IRNode() {}
 
+    virtual int nodeCount() const {
+        SkASSERT(false);
+        return 1;
+    }
+
+
     virtual String description() const = 0;
 
     // character offset of this element within the program being compiled, for error reporting
diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h
index 5d0a22b..6af6740 100644
--- a/src/sksl/ir/SkSLIfStatement.h
+++ b/src/sksl/ir/SkSLIfStatement.h
@@ -25,6 +25,11 @@
     , fIfTrue(std::move(ifTrue))
     , fIfFalse(std::move(ifFalse)) {}
 
+    int nodeCount() const override {
+        return 1 + fTest->nodeCount() + fIfTrue->nodeCount() +
+               (fIfFalse ? fIfFalse->nodeCount() : 0);
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new IfStatement(fOffset, fIsStatic, fTest->clone(),
                 fIfTrue->clone(), fIfFalse ? fIfFalse->clone() : nullptr));
diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h
index 2b3a3a9..2018881 100644
--- a/src/sksl/ir/SkSLIndexExpression.h
+++ b/src/sksl/ir/SkSLIndexExpression.h
@@ -62,6 +62,10 @@
         return fBase->hasProperty(property) || fIndex->hasProperty(property);
     }
 
+    int nodeCount() const override {
+        return 1 + fBase->nodeCount() + fIndex->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new IndexExpression(fBase->clone(), fIndex->clone(),
                                                                &fType));
diff --git a/src/sksl/ir/SkSLIntLiteral.h b/src/sksl/ir/SkSLIntLiteral.h
index 6c686e1..4977fbf 100644
--- a/src/sksl/ir/SkSLIntLiteral.h
+++ b/src/sksl/ir/SkSLIntLiteral.h
@@ -56,6 +56,10 @@
         return fValue;
     }
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new IntLiteral(fOffset, fValue, &fType));
     }
diff --git a/src/sksl/ir/SkSLNop.h b/src/sksl/ir/SkSLNop.h
index 2ead371..787060d 100644
--- a/src/sksl/ir/SkSLNop.h
+++ b/src/sksl/ir/SkSLNop.h
@@ -28,6 +28,10 @@
         return String(";");
     }
 
+    int nodeCount() const override {
+        return 0;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new Nop());
     }
diff --git a/src/sksl/ir/SkSLNullLiteral.h b/src/sksl/ir/SkSLNullLiteral.h
index d04fc5c..0d04b77 100644
--- a/src/sksl/ir/SkSLNullLiteral.h
+++ b/src/sksl/ir/SkSLNullLiteral.h
@@ -39,6 +39,10 @@
         return true;
     }
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new NullLiteral(fOffset, fType));
     }
diff --git a/src/sksl/ir/SkSLPostfixExpression.h b/src/sksl/ir/SkSLPostfixExpression.h
index d95f687..c6f89eb 100644
--- a/src/sksl/ir/SkSLPostfixExpression.h
+++ b/src/sksl/ir/SkSLPostfixExpression.h
@@ -30,6 +30,10 @@
         return fOperand->hasProperty(property);
     }
 
+    int nodeCount() const override {
+        return 1 + fOperand->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new PostfixExpression(fOperand->clone(), fOperator));
     }
diff --git a/src/sksl/ir/SkSLPrefixExpression.h b/src/sksl/ir/SkSLPrefixExpression.h
index 76f144b..ab0363e 100644
--- a/src/sksl/ir/SkSLPrefixExpression.h
+++ b/src/sksl/ir/SkSLPrefixExpression.h
@@ -64,6 +64,10 @@
         return -fOperand->getMatComponent(col, row);
     }
 
+    int nodeCount() const override {
+        return 1 + fOperand->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new PrefixExpression(fOperator, fOperand->clone()));
     }
diff --git a/src/sksl/ir/SkSLReturnStatement.h b/src/sksl/ir/SkSLReturnStatement.h
index e61fa36..82b4678 100644
--- a/src/sksl/ir/SkSLReturnStatement.h
+++ b/src/sksl/ir/SkSLReturnStatement.h
@@ -24,6 +24,10 @@
     : INHERITED(expression->fOffset, kReturn_Kind)
     , fExpression(std::move(expression)) {}
 
+    int nodeCount() const override {
+        return 1 + fExpression->nodeCount();
+    }
+
     std::unique_ptr<Statement> clone() const override {
         if (fExpression) {
             return std::unique_ptr<Statement>(new ReturnStatement(fExpression->clone()));
diff --git a/src/sksl/ir/SkSLSetting.h b/src/sksl/ir/SkSLSetting.h
index a47b42b..9a69802 100644
--- a/src/sksl/ir/SkSLSetting.h
+++ b/src/sksl/ir/SkSLSetting.h
@@ -28,6 +28,10 @@
     std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
                                                   const DefinitionMap& definitions) override;
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new Setting(fOffset, fName, fValue->clone()));
     }
diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h
index ed8d33b..2260cdd 100644
--- a/src/sksl/ir/SkSLStatement.h
+++ b/src/sksl/ir/SkSLStatement.h
@@ -25,7 +25,6 @@
         kDo_Kind,
         kExpression_Kind,
         kFor_Kind,
-        kGroup_Kind,
         kIf_Kind,
         kNop_Kind,
         kReturn_Kind,
diff --git a/src/sksl/ir/SkSLSwitchCase.h b/src/sksl/ir/SkSLSwitchCase.h
index b1ddb01..dcfe956 100644
--- a/src/sksl/ir/SkSLSwitchCase.h
+++ b/src/sksl/ir/SkSLSwitchCase.h
@@ -23,6 +23,17 @@
     , fValue(std::move(value))
     , fStatements(std::move(statements)) {}
 
+    int nodeCount() const override {
+        int result = 1;
+        if (fValue) {
+            result += fValue->nodeCount();
+        }
+        for (const auto& s : fStatements) {
+            result += s->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         std::vector<std::unique_ptr<Statement>> cloned;
         for (const auto& s : fStatements) {
diff --git a/src/sksl/ir/SkSLSwitchStatement.h b/src/sksl/ir/SkSLSwitchStatement.h
index 0777c5c..a85420a 100644
--- a/src/sksl/ir/SkSLSwitchStatement.h
+++ b/src/sksl/ir/SkSLSwitchStatement.h
@@ -28,6 +28,14 @@
     , fSymbols(std::move(symbols))
     , fCases(std::move(cases)) {}
 
+    int nodeCount() const override {
+        int result = 1 + fValue->nodeCount();
+        for (const auto& c : fCases) {
+            result += c->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         std::vector<std::unique_ptr<SwitchCase>> cloned;
         for (const auto& s : fCases) {
diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h
index 3428f28..59ff04c 100644
--- a/src/sksl/ir/SkSLSwizzle.h
+++ b/src/sksl/ir/SkSLSwizzle.h
@@ -136,6 +136,10 @@
         return fBase->hasProperty(property);
     }
 
+    int nodeCount() const override {
+        return 1 + fBase->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new Swizzle(fType, fBase->clone(), fComponents));
     }
diff --git a/src/sksl/ir/SkSLSymbol.h b/src/sksl/ir/SkSLSymbol.h
index 4ee4d3d..df606b8 100644
--- a/src/sksl/ir/SkSLSymbol.h
+++ b/src/sksl/ir/SkSLSymbol.h
@@ -30,7 +30,7 @@
     , fKind(kind)
     , fName(name) {}
 
-    virtual ~Symbol() {}
+    virtual ~Symbol() override {}
 
     Kind fKind;
     StringFragment fName;
diff --git a/src/sksl/ir/SkSLSymbolTable.cpp b/src/sksl/ir/SkSLSymbolTable.cpp
index bbf001d..b0a2a56 100644
--- a/src/sksl/ir/SkSLSymbolTable.cpp
+++ b/src/sksl/ir/SkSLSymbolTable.cpp
@@ -72,6 +72,12 @@
     return result;
 }
 
+String* SymbolTable::takeOwnership(std::unique_ptr<String> n) {
+    String* result = n.get();
+    fOwnedStrings.push_back(std::move(n));
+    return result;
+}
+
 void SymbolTable::add(StringFragment name, std::unique_ptr<Symbol> symbol) {
     this->addWithoutOwnership(name, symbol.get());
     this->takeOwnership(std::move(symbol));
diff --git a/src/sksl/ir/SkSLSymbolTable.h b/src/sksl/ir/SkSLSymbolTable.h
index 6969ba5..a3f89e2 100644
--- a/src/sksl/ir/SkSLSymbolTable.h
+++ b/src/sksl/ir/SkSLSymbolTable.h
@@ -41,6 +41,8 @@
 
     IRNode* takeOwnership(std::unique_ptr<IRNode> n);
 
+    String* takeOwnership(std::unique_ptr<String> n);
+
     void markAllFunctionsBuiltin();
 
     std::unordered_map<StringFragment, const Symbol*>::iterator begin();
@@ -56,6 +58,8 @@
 
     std::vector<std::unique_ptr<IRNode>> fOwnedNodes;
 
+    std::vector<std::unique_ptr<String>> fOwnedStrings;
+
     std::unordered_map<StringFragment, const Symbol*> fSymbols;
 
     ErrorReporter& fErrorReporter;
diff --git a/src/sksl/ir/SkSLTernaryExpression.h b/src/sksl/ir/SkSLTernaryExpression.h
index 80dcb8f..cb4c004 100644
--- a/src/sksl/ir/SkSLTernaryExpression.h
+++ b/src/sksl/ir/SkSLTernaryExpression.h
@@ -36,6 +36,10 @@
                fIfFalse->isConstantOrUniform();
     }
 
+    int nodeCount() const override {
+        return 1 + fTest->nodeCount() + fIfTrue->nodeCount() + fIfFalse->nodeCount();
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new TernaryExpression(fOffset, fTest->clone(),
                                                                  fIfTrue->clone(),
diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h
index 82dbd86..86aafdb 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -29,6 +29,19 @@
     , fSizes(std::move(sizes))
     , fValue(std::move(value)) {}
 
+    int nodeCount() const override {
+        int result = 1;
+        for (const auto& s : fSizes) {
+            if (s) {
+                result += s->nodeCount();
+            }
+        }
+        if (fValue) {
+            result += fValue->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<Statement> clone() const override {
         std::vector<std::unique_ptr<Expression>> sizesClone;
         for (const auto& s : fSizes) {
@@ -77,6 +90,14 @@
         }
     }
 
+    int nodeCount() const override {
+        int result = 1;
+        for (const auto& v : fVars) {
+            result += v->nodeCount();
+        }
+        return result;
+    }
+
     std::unique_ptr<ProgramElement> clone() const override {
         std::vector<std::unique_ptr<VarDeclaration>> cloned;
         for (const auto& v : fVars) {
@@ -91,8 +112,15 @@
         if (!fVars.size()) {
             return String();
         }
-        String result = ((VarDeclaration&) *fVars[0]).fVar->fModifiers.description() +
-                fBaseType.description() + " ";
+        String result;
+        for (const auto& var : fVars) {
+            if (var->fKind != Statement::kNop_Kind) {
+                SkASSERT(var->fKind == Statement::kVarDeclaration_Kind);
+                result = ((const VarDeclaration&) *var).fVar->fModifiers.description();
+                break;
+            }
+        }
+        result += fBaseType.description() + " ";
         String separator;
         for (const auto& var : fVars) {
             result += separator;
diff --git a/src/sksl/ir/SkSLVarDeclarationsStatement.h b/src/sksl/ir/SkSLVarDeclarationsStatement.h
index 465b691..eb7880f 100644
--- a/src/sksl/ir/SkSLVarDeclarationsStatement.h
+++ b/src/sksl/ir/SkSLVarDeclarationsStatement.h
@@ -30,6 +30,10 @@
         return true;
     }
 
+    int nodeCount() const override {
+        return 1 + fDeclaration->nodeCount();
+    }
+
     std::unique_ptr<Statement> clone() const override {
         std::unique_ptr<VarDeclarations> cloned((VarDeclarations*) fDeclaration->clone().release());
         return std::unique_ptr<Statement>(new VarDeclarationsStatement(std::move(cloned)));
diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h
index 5d962b9..c675144 100644
--- a/src/sksl/ir/SkSLVariableReference.h
+++ b/src/sksl/ir/SkSLVariableReference.h
@@ -62,6 +62,10 @@
         return (fVariable.fModifiers.fFlags & Modifiers::kUniform_Flag) != 0;
     }
 
+    int nodeCount() const override {
+        return 1;
+    }
+
     std::unique_ptr<Expression> clone() const override {
         return std::unique_ptr<Expression>(new VariableReference(fOffset, fVariable, fRefKind));
     }
diff --git a/src/sksl/ir/SkSLWhileStatement.h b/src/sksl/ir/SkSLWhileStatement.h
index 8e311a0..0782a01 100644
--- a/src/sksl/ir/SkSLWhileStatement.h
+++ b/src/sksl/ir/SkSLWhileStatement.h
@@ -23,6 +23,10 @@
     , fTest(std::move(test))
     , fStatement(std::move(statement)) {}
 
+    int nodeCount() const override {
+        return 1 + fTest->nodeCount() + fStatement->nodeCount();
+    }
+
     std::unique_ptr<Statement> clone() const override {
         return std::unique_ptr<Statement>(new WhileStatement(fOffset, fTest->clone(),
                                                              fStatement->clone()));
diff --git a/tests/SkSLErrorTest.cpp b/tests/SkSLErrorTest.cpp
index 53214d2..73e364a 100644
--- a/tests/SkSLErrorTest.cpp
+++ b/tests/SkSLErrorTest.cpp
@@ -371,7 +371,7 @@
                  "void main() { if (true) return; else discard; return; }",
                  "error: 1: unreachable\n1 error\n");
     test_failure(r,
-                 "void main() { return; while (true); }",
+                 "void main() { return; main(); }",
                  "error: 1: unreachable\n1 error\n");
 }
 
diff --git a/tests/SkSLFPTest.cpp b/tests/SkSLFPTest.cpp
index c1de765..413f46e 100644
--- a/tests/SkSLFPTest.cpp
+++ b/tests/SkSLFPTest.cpp
@@ -784,8 +784,11 @@
             "const GrShaderVar flip_args[] = { GrShaderVar(\"c\", kHalf4_GrSLType)};",
             "fragBuilder->emitFunction(kHalf4_GrSLType, \"flip\", 1, flip_args, "
                                       "\"return c.wzyx;\\n\", &flip_name);",
-            "fragBuilder->codeAppendf(\"%s = %s(%s);\\n\", args.fOutputColor, flip_name.c_str(), "
-                                      "args.fInputColor);"
+            "fragBuilder->codeAppendf(\"half4 inlineResult101;\\nhalf4 inlineArg101_0 = %s;\\ndo {"
+                                     "\\n    {\\n        inlineResult101 = inlineArg101_0.wzyx;\\n"
+                                     "        break;\\n    }\\n} while (false);\\n%s = "
+                                     "inlineResult101;\\n\\n\", args.fInputColor, "
+                                     "args.fOutputColor);"
          });
 }
 
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index a67ca25..d864726 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -111,7 +111,16 @@
          "    float y[2], z;\n"
          "    y[0] = x;\n"
          "    y[1] = x * 2.0;\n"
-         "    z = foo(y);\n"
+         "    float inlineResult117;\n"
+         "    float[2] inlineArg117_0 = y;\n"
+         "    do {\n"
+         "        {\n"
+         "            inlineResult117 = inlineArg117_0[0] * inlineArg117_0[1];\n"
+         "            break;\n"
+         "        }\n"
+         "    } while (false);\n"
+         "    z = inlineResult117;\n"
+         "\n"
          "    x = z;\n"
          "}\n"
          "void main() {\n"