From 8f7e28f3ae262450c5b04deded9fa23041757868 Mon Sep 17 00:00:00 2001 From: Ethan Nicholas Date: Mon, 26 Mar 2018 14:24:27 -0400 Subject: added frexp support to SkSL This includes an optimizer fix for the situation: int i; float f = frexp(foo, i); If we don't read the variable i, it is considered dead and eliminated - which then causes an error when we try to write the expression frexmp(foo, i). Bug: skia: Change-Id: Iac385e38e215455346fab62e1f4ec46fa65b3c21 Reviewed-on: https://skia-review.googlesource.com/116521 Reviewed-by: Chris Dalton Commit-Queue: Ethan Nicholas --- src/sksl/SkSLCompiler.cpp | 7 ++- src/sksl/SkSLIRGenerator.cpp | 33 ++++++----- src/sksl/SkSLIRGenerator.h | 3 +- src/sksl/SkSLSPIRVCodeGenerator.cpp | 24 +++++--- src/sksl/ir/SkSLVariableReference.cpp | 108 ++++++++++++++++++++++++++++++++++ src/sksl/ir/SkSLVariableReference.h | 104 ++++---------------------------- 6 files changed, 161 insertions(+), 118 deletions(-) create mode 100644 src/sksl/ir/SkSLVariableReference.cpp (limited to 'src/sksl') diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 06ab71f922..bbaaf407d2 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -660,8 +660,11 @@ void Compiler::simplifyExpression(DefinitionMap& definitions, } switch (expr->fKind) { case Expression::kVariableReference_Kind: { - const Variable& var = ((VariableReference*) expr)->fVariable; - if (var.fStorage == Variable::kLocal_Storage && !definitions[&var] && + const VariableReference& ref = (VariableReference&) *expr; + const Variable& var = ref.fVariable; + if (ref.refKind() != VariableReference::kWrite_RefKind && + ref.refKind() != VariableReference::kPointer_RefKind && + var.fStorage == Variable::kLocal_Storage && !definitions[&var] && (*undefinedVariables).find(&var) == (*undefinedVariables).end()) { (*undefinedVariables).insert(&var); this->error(expr->fOffset, diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index c6d09de7d5..68d9f9c0ca 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -1403,7 +1403,9 @@ std::unique_ptr IRGenerator::convertBinaryExpression( return nullptr; } if (Compiler::IsAssignment(expression.fOperator)) { - this->markWrittenTo(*left, expression.fOperator != Token::EQ); + this->setRefKind(*left, expression.fOperator != Token::EQ ? + VariableReference::kReadWrite_RefKind : + VariableReference::kWrite_RefKind); } left = this->coerce(std::move(left), *leftType); right = this->coerce(std::move(right), *rightType); @@ -1534,8 +1536,10 @@ std::unique_ptr IRGenerator::call(int offset, return nullptr; } if (arguments[i] && (function.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) { - this->markWrittenTo(*arguments[i], - function.fParameters[i]->fModifiers.fFlags & Modifiers::kIn_Flag); + this->setRefKind(*arguments[i], + function.fParameters[i]->fModifiers.fFlags & Modifiers::kIn_Flag ? + VariableReference::kReadWrite_RefKind : + VariableReference::kPointer_RefKind); } } if (function.fBuiltin && function.fName == "texture" && @@ -1786,7 +1790,7 @@ std::unique_ptr IRGenerator::convertPrefixExpression( "' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->setRefKind(*base, VariableReference::kReadWrite_RefKind); break; case Token::MINUSMINUS: if (!base->fType.isNumber()) { @@ -1795,7 +1799,7 @@ std::unique_ptr IRGenerator::convertPrefixExpression( "' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->setRefKind(*base, VariableReference::kReadWrite_RefKind); break; case Token::LOGICALNOT: if (base->fType != *fContext.fBool_Type) { @@ -2032,7 +2036,7 @@ std::unique_ptr IRGenerator::convertSuffixExpression( "'++' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->setRefKind(*base, VariableReference::kReadWrite_RefKind); return std::unique_ptr(new PostfixExpression(std::move(base), Token::PLUSPLUS)); case ASTSuffix::kPostDecrement_Kind: @@ -2041,7 +2045,7 @@ std::unique_ptr IRGenerator::convertSuffixExpression( "'--' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->setRefKind(*base, VariableReference::kReadWrite_RefKind); return std::unique_ptr(new PostfixExpression(std::move(base), Token::MINUSMINUS)); default: @@ -2077,7 +2081,7 @@ static bool has_duplicates(const Swizzle& swizzle) { return false; } -void IRGenerator::markWrittenTo(const Expression& expr, bool readWrite) { +void IRGenerator::setRefKind(const Expression& expr, VariableReference::RefKind kind) { switch (expr.fKind) { case Expression::kVariableReference_Kind: { const Variable& var = ((VariableReference&) expr).fVariable; @@ -2085,27 +2089,26 @@ void IRGenerator::markWrittenTo(const Expression& expr, bool readWrite) { fErrors.error(expr.fOffset, "cannot modify immutable variable '" + var.fName + "'"); } - ((VariableReference&) expr).setRefKind(readWrite ? VariableReference::kReadWrite_RefKind - : VariableReference::kWrite_RefKind); + ((VariableReference&) expr).setRefKind(kind); break; } case Expression::kFieldAccess_Kind: - this->markWrittenTo(*((FieldAccess&) expr).fBase, readWrite); + this->setRefKind(*((FieldAccess&) expr).fBase, kind); break; case Expression::kSwizzle_Kind: if (has_duplicates((Swizzle&) expr)) { fErrors.error(expr.fOffset, "cannot write to the same swizzle field more than once"); } - this->markWrittenTo(*((Swizzle&) expr).fBase, readWrite); + this->setRefKind(*((Swizzle&) expr).fBase, kind); break; case Expression::kIndex_Kind: - this->markWrittenTo(*((IndexExpression&) expr).fBase, readWrite); + this->setRefKind(*((IndexExpression&) expr).fBase, kind); break; case Expression::kTernary_Kind: { TernaryExpression& t = (TernaryExpression&) expr; - this->markWrittenTo(*t.fIfTrue, readWrite); - this->markWrittenTo(*t.fIfFalse, readWrite); + this->setRefKind(*t.fIfTrue, kind); + this->setRefKind(*t.fIfFalse, kind); break; } default: diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h index 327fe6fc7a..e8ff1d21c8 100644 --- a/src/sksl/SkSLIRGenerator.h +++ b/src/sksl/SkSLIRGenerator.h @@ -50,6 +50,7 @@ #include "ir/SkSLType.h" #include "ir/SkSLTypeReference.h" #include "ir/SkSLVarDeclarations.h" +#include "ir/SkSLVariableReference.h" namespace SkSL { @@ -162,7 +163,7 @@ private: void fixRectSampling(std::vector>& arguments); void checkValid(const Expression& expr); - void markWrittenTo(const Expression& expr, bool readWrite); + void setRefKind(const Expression& expr, VariableReference::RefKind kind); void getConstantInt(const Expression& value, int64_t* out); Program::Kind fKind; diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp index 7d3726a8c1..dadab91753 100644 --- a/src/sksl/SkSLSPIRVCodeGenerator.cpp +++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp @@ -684,7 +684,11 @@ SpvId SPIRVCodeGenerator::writeIntrinsicCall(const FunctionCall& c, OutputStream SpvId result = this->nextId(); std::vector arguments; for (size_t i = 0; i < c.fArguments.size(); i++) { - arguments.push_back(this->writeExpression(*c.fArguments[i], out)); + if (c.fFunction.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag) { + arguments.push_back(this->getLValue(*c.fArguments[i], out)->getPointer()); + } else { + arguments.push_back(this->writeExpression(*c.fArguments[i], out)); + } } this->writeOpCode(SpvOpExtInst, 5 + (int32_t) arguments.size(), out); this->writeWord(this->getType(c.fType), out); @@ -700,7 +704,11 @@ SpvId SPIRVCodeGenerator::writeIntrinsicCall(const FunctionCall& c, OutputStream SpvId result = this->nextId(); std::vector arguments; for (size_t i = 0; i < c.fArguments.size(); i++) { - arguments.push_back(this->writeExpression(*c.fArguments[i], out)); + if (c.fFunction.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag) { + arguments.push_back(this->getLValue(*c.fArguments[i], out)->getPointer()); + } else { + arguments.push_back(this->writeExpression(*c.fArguments[i], out)); + } } if (c.fType != *fContext.fVoid_Type) { this->writeOpCode((SpvOp_) intrinsicId, 3 + (int32_t) arguments.size(), out); @@ -1574,9 +1582,9 @@ std::unique_ptr SPIRVCodeGenerator::getLValue(const auto entry = fVariableMap.find(&var); ASSERT(entry != fVariableMap.end()); return std::unique_ptr(new PointerLValue( - *this, - entry->second, - this->getType(expr.fType))); + *this, + entry->second, + this->getType(expr.fType))); } case Expression::kIndex_Kind: // fall through case Expression::kFieldAccess_Kind: { @@ -1589,9 +1597,9 @@ std::unique_ptr SPIRVCodeGenerator::getLValue(const this->writeWord(idx, out); } return std::unique_ptr(new PointerLValue( - *this, - member, - this->getType(expr.fType))); + *this, + member, + this->getType(expr.fType))); } case Expression::kSwizzle_Kind: { Swizzle& swizzle = (Swizzle&) expr; diff --git a/src/sksl/ir/SkSLVariableReference.cpp b/src/sksl/ir/SkSLVariableReference.cpp new file mode 100644 index 0000000000..37e0ca2e7d --- /dev/null +++ b/src/sksl/ir/SkSLVariableReference.cpp @@ -0,0 +1,108 @@ +/* + * Copyright 2018 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkSLVariableReference.h" + +#include "SkSLConstructor.h" +#include "SkSLFloatLiteral.h" +#include "SkSLIRGenerator.h" +#include "SkSLSetting.h" + +namespace SkSL { + +VariableReference::VariableReference(int offset, const Variable& variable, RefKind refKind) +: INHERITED(offset, kVariableReference_Kind, variable.fType) +, fVariable(variable) +, fRefKind(refKind) { + if (refKind != kRead_RefKind) { + fVariable.fWriteCount++; + } + if (refKind != kWrite_RefKind) { + fVariable.fReadCount++; + } +} + +VariableReference::~VariableReference() { + if (fRefKind != kRead_RefKind) { + fVariable.fWriteCount--; + } + if (fRefKind != kWrite_RefKind) { + fVariable.fReadCount--; + } +} + +void VariableReference::setRefKind(RefKind refKind) { + if (fRefKind != kRead_RefKind) { + fVariable.fWriteCount--; + } + if (fRefKind != kWrite_RefKind) { + fVariable.fReadCount--; + } + if (refKind != kRead_RefKind) { + fVariable.fWriteCount++; + } + if (refKind != kWrite_RefKind) { + fVariable.fReadCount++; + } + fRefKind = refKind; +} + +std::unique_ptr VariableReference::copy_constant(const IRGenerator& irGenerator, + const Expression* expr) { + ASSERT(expr->isConstant()); + switch (expr->fKind) { + case Expression::kIntLiteral_Kind: + return std::unique_ptr(new IntLiteral(irGenerator.fContext, + -1, + ((IntLiteral*) expr)->fValue)); + case Expression::kFloatLiteral_Kind: + return std::unique_ptr(new FloatLiteral( + irGenerator.fContext, + -1, + ((FloatLiteral*) expr)->fValue)); + case Expression::kBoolLiteral_Kind: + return std::unique_ptr(new BoolLiteral(irGenerator.fContext, + -1, + ((BoolLiteral*) expr)->fValue)); + case Expression::kConstructor_Kind: { + const Constructor* c = (const Constructor*) expr; + std::vector> args; + for (const auto& arg : c->fArguments) { + args.push_back(copy_constant(irGenerator, arg.get())); + } + return std::unique_ptr(new Constructor(-1, c->fType, + std::move(args))); + } + case Expression::kSetting_Kind: { + const Setting* s = (const Setting*) expr; + return std::unique_ptr(new Setting(-1, s->fName, + copy_constant(irGenerator, + s->fValue.get()))); + } + default: + ABORT("unsupported constant\n"); + } +} + +std::unique_ptr VariableReference::constantPropagate(const IRGenerator& irGenerator, + const DefinitionMap& definitions) { + if (fRefKind != kRead_RefKind) { + return nullptr; + } + if ((fVariable.fModifiers.fFlags & Modifiers::kConst_Flag) && fVariable.fInitialValue && + fVariable.fInitialValue->isConstant()) { + return copy_constant(irGenerator, fVariable.fInitialValue); + } + auto exprIter = definitions.find(&fVariable); + if (exprIter != definitions.end() && exprIter->second && + (*exprIter->second)->isConstant()) { + return copy_constant(irGenerator, exprIter->second->get()); + } + return nullptr; +} + +} // namespace diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h index e1f19ac742..14ddf796ff 100644 --- a/src/sksl/ir/SkSLVariableReference.h +++ b/src/sksl/ir/SkSLVariableReference.h @@ -8,16 +8,12 @@ #ifndef SKSL_VARIABLEREFERENCE #define SKSL_VARIABLEREFERENCE -#include "SkSLBoolLiteral.h" -#include "SkSLConstructor.h" #include "SkSLExpression.h" -#include "SkSLFloatLiteral.h" -#include "SkSLIRGenerator.h" -#include "SkSLIntLiteral.h" -#include "SkSLSetting.h" namespace SkSL { +class IRGenerator; + /** * A reference to a variable, through which it can be read or written. In the statement: * @@ -29,49 +25,21 @@ struct VariableReference : public Expression { enum RefKind { kRead_RefKind, kWrite_RefKind, - kReadWrite_RefKind + kReadWrite_RefKind, + // taking the address of a variable - we consider this a read & write but don't complain if + // the variable was not previously assigned + kPointer_RefKind }; - VariableReference(int offset, const Variable& variable, RefKind refKind = kRead_RefKind) - : INHERITED(offset, kVariableReference_Kind, variable.fType) - , fVariable(variable) - , fRefKind(refKind) { - if (refKind != kRead_RefKind) { - fVariable.fWriteCount++; - } - if (refKind != kWrite_RefKind) { - fVariable.fReadCount++; - } - } + VariableReference(int offset, const Variable& variable, RefKind refKind = kRead_RefKind); - ~VariableReference() override { - if (fRefKind != kRead_RefKind) { - fVariable.fWriteCount--; - } - if (fRefKind != kWrite_RefKind) { - fVariable.fReadCount--; - } - } + ~VariableReference() override; - RefKind refKind() { + RefKind refKind() const { return fRefKind; } - void setRefKind(RefKind refKind) { - if (fRefKind != kRead_RefKind) { - fVariable.fWriteCount--; - } - if (fRefKind != kWrite_RefKind) { - fVariable.fReadCount--; - } - if (refKind != kRead_RefKind) { - fVariable.fWriteCount++; - } - if (refKind != kWrite_RefKind) { - fVariable.fReadCount++; - } - fRefKind = refKind; - } + void setRefKind(RefKind refKind); bool hasSideEffects() const override { return false; @@ -86,58 +54,10 @@ struct VariableReference : public Expression { } static std::unique_ptr copy_constant(const IRGenerator& irGenerator, - const Expression* expr) { - ASSERT(expr->isConstant()); - switch (expr->fKind) { - case Expression::kIntLiteral_Kind: - return std::unique_ptr(new IntLiteral(irGenerator.fContext, - -1, - ((IntLiteral*) expr)->fValue)); - case Expression::kFloatLiteral_Kind: - return std::unique_ptr(new FloatLiteral( - irGenerator.fContext, - -1, - ((FloatLiteral*) expr)->fValue)); - case Expression::kBoolLiteral_Kind: - return std::unique_ptr(new BoolLiteral(irGenerator.fContext, - -1, - ((BoolLiteral*) expr)->fValue)); - case Expression::kConstructor_Kind: { - const Constructor* c = (const Constructor*) expr; - std::vector> args; - for (const auto& arg : c->fArguments) { - args.push_back(copy_constant(irGenerator, arg.get())); - } - return std::unique_ptr(new Constructor(-1, c->fType, - std::move(args))); - } - case Expression::kSetting_Kind: { - const Setting* s = (const Setting*) expr; - return std::unique_ptr(new Setting(-1, s->fName, - copy_constant(irGenerator, - s->fValue.get()))); - } - default: - ABORT("unsupported constant\n"); - } - } + const Expression* expr); std::unique_ptr constantPropagate(const IRGenerator& irGenerator, - const DefinitionMap& definitions) override { - if (fRefKind != kRead_RefKind) { - return nullptr; - } - if ((fVariable.fModifiers.fFlags & Modifiers::kConst_Flag) && fVariable.fInitialValue && - fVariable.fInitialValue->isConstant()) { - return copy_constant(irGenerator, fVariable.fInitialValue); - } - auto exprIter = definitions.find(&fVariable); - if (exprIter != definitions.end() && exprIter->second && - (*exprIter->second)->isConstant()) { - return copy_constant(irGenerator, exprIter->second->get()); - } - return nullptr; - } + const DefinitionMap& definitions) override; const Variable& fVariable; RefKind fRefKind; -- cgit v1.2.3