Convert IRGenerator::convertBinaryExpr to BinaryExpr::Make.

Interestingly, this reduces the size of our dehydated data
significantly, because we were storing the result type of the binary
expression explicitly. Now the result type is deduced automatically from
the left and right types by the Make call.

Change-Id: Ic6187a5c36774f5a4ed2ec579e9cd13b331832b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377099
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2021-03-01 09:27:48 -05:00 committed by Skia Commit-Bot
parent f1af57cf0a
commit e2aec43cfc
11 changed files with 304 additions and 551 deletions

View File

@ -82,6 +82,7 @@ skia_sksl_sources = [
"$_src/sksl/dsl/priv/DSLFPs.h",
"$_src/sksl/dsl/priv/DSLWriter.cpp",
"$_src/sksl/dsl/priv/DSLWriter.h",
"$_src/sksl/ir/SkSLBinaryExpression.cpp",
"$_src/sksl/ir/SkSLBinaryExpression.h",
"$_src/sksl/ir/SkSLBlock.h",
"$_src/sksl/ir/SkSLBoolLiteral.h",

View File

@ -262,7 +262,6 @@ void Dehydrator::write(const Expression* e) {
this->write(b.left().get());
this->writeU8((int) b.getOperator().kind());
this->write(b.right().get());
this->write(b.type());
break;
}
case Expression::Kind::kBoolLiteral: {

View File

@ -763,16 +763,14 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
fProgramElements->push_back(std::move(invokeDef));
std::vector<std::unique_ptr<VarDeclaration>> variables;
const Variable* loopIdx = &(*fSymbolTable)["sk_InvocationID"]->as<Variable>();
auto test = std::make_unique<BinaryExpression>(
/*offset=*/-1,
auto test = BinaryExpression::Make(
fContext,
std::make_unique<VariableReference>(/*offset=*/-1, loopIdx),
Token::Kind::TK_LT,
std::make_unique<IntLiteral>(fContext, /*offset=*/-1, fInvocations),
fContext.fTypes.fBool.get());
std::make_unique<IntLiteral>(fContext, /*offset=*/-1, fInvocations));
auto next = PostfixExpression::Make(
fContext,
std::make_unique<VariableReference>(/*offset=*/-1, loopIdx,
VariableReference::RefKind::kReadWrite),
std::make_unique<VariableReference>(/*offset=*/-1, loopIdx,VariableRefKind::kReadWrite),
Token::Kind::TK_PLUSPLUS);
ASTNode endPrimitiveID(&fFile->fNodes, -1, ASTNode::Kind::kIdentifier, "EndPrimitive");
std::unique_ptr<Expression> endPrimitive = this->convertExpression(endPrimitiveID);
@ -786,13 +784,11 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
loopBody.push_back(ExpressionStatement::Make(fContext, this->call(/*offset=*/-1,
std::move(endPrimitive),
ExpressionArray{})));
auto assignment = std::make_unique<BinaryExpression>(
/*offset=*/-1,
std::make_unique<VariableReference>(/*offset=*/-1, loopIdx,
VariableReference::RefKind::kWrite),
auto assignment = BinaryExpression::Make(
fContext,
std::make_unique<VariableReference>(/*offset=*/-1, loopIdx, VariableRefKind::kWrite),
Token::Kind::TK_EQ,
std::make_unique<IntLiteral>(fContext, /*offset=*/-1, /*value=*/0),
fContext.fTypes.fInt.get());
std::make_unique<IntLiteral>(fContext, /*offset=*/-1, /*value=*/0));
auto initializer = ExpressionStatement::Make(fContext, std::move(assignment));
auto loop = ForStatement::Make(
fContext, /*offset=*/-1, std::move(initializer), std::move(test), std::move(next),
@ -839,8 +835,7 @@ std::unique_ptr<Statement> IRGenerator::getNormalizeSkPositionCode() {
};
auto Op = [&](std::unique_ptr<Expression> left, Token::Kind op,
std::unique_ptr<Expression> right) -> std::unique_ptr<Expression> {
return std::make_unique<BinaryExpression>(/*offset=*/-1, std::move(left), op,
std::move(right), fContext.fTypes.fFloat2.get());
return BinaryExpression::Make(fContext, std::move(left), op, std::move(right));
};
static const ComponentArray kXYIndices{0, 1};
@ -1610,82 +1605,8 @@ std::unique_ptr<Expression> IRGenerator::convertBinaryExpression(const ASTNode&
if (!right) {
return nullptr;
}
return this->convertBinaryExpression(std::move(left), expression.getOperator(),
std::move(right));
}
std::unique_ptr<Expression> IRGenerator::convertBinaryExpression(
std::unique_ptr<Expression> left,
Operator op,
std::unique_ptr<Expression> right) {
if (!left || !right) {
return nullptr;
}
int offset = left->fOffset;
const Type* leftType;
const Type* rightType;
const Type* resultType;
const Type* rawLeftType;
if (left->is<IntLiteral>() && right->type().isInteger()) {
rawLeftType = &right->type();
} else {
rawLeftType = &left->type();
}
const Type* rawRightType;
if (right->is<IntLiteral>() && left->type().isInteger()) {
rawRightType = &left->type();
} else {
rawRightType = &right->type();
}
if (this->strictES2Mode() && !op.isAllowedInStrictES2Mode()) {
this->errorReporter().error(offset,
String("operator '") + op.operatorName() + "' is not allowed");
return nullptr;
}
bool isAssignment = op.isAssignment();
if (isAssignment &&
!Analysis::MakeAssignmentExpr(left.get(),
op.kind() != Token::Kind::TK_EQ
? VariableReference::RefKind::kReadWrite
: VariableReference::RefKind::kWrite,
&fContext.fErrors)) {
return nullptr;
}
if (!op.determineBinaryType(fContext, *rawLeftType, *rawRightType,
&leftType, &rightType, &resultType)) {
this->errorReporter().error(offset, String("type mismatch: '") + op.operatorName() +
"' cannot operate on '" + left->type().displayName() +
"', '" + right->type().displayName() + "'");
return nullptr;
}
if (isAssignment && leftType->componentType().isOpaque()) {
this->errorReporter().error(offset, "assignments to opaque type '" +
left->type().displayName() + "' are not permitted");
}
if (this->strictES2Mode() && leftType->isOrContainsArray()) {
// Most operators are already rejected on arrays, but GLSL ES 1.0 is very explicit that the
// *only* operator allowed on arrays is subscripting (and the rules against assignment,
// comparison, and even sequence apply to structs containing arrays as well).
this->errorReporter().error(
offset,
String("operator '") + op.operatorName() +
"' can not operate on arrays (or structs containing arrays)");
return nullptr;
}
left = this->coerce(std::move(left), *leftType);
right = this->coerce(std::move(right), *rightType);
if (!left || !right) {
return nullptr;
}
std::unique_ptr<Expression> result;
if (!ConstantFolder::ErrorOnDivideByZero(fContext, offset, op, *right)) {
result = ConstantFolder::Simplify(fContext, offset, *left, op, *right);
}
if (!result) {
result = std::make_unique<BinaryExpression>(offset, std::move(left), op, std::move(right),
resultType);
}
return result;
return BinaryExpression::Make(fContext, std::move(left), expression.getOperator(),
std::move(right));
}
std::unique_ptr<Expression> IRGenerator::convertTernaryExpression(const ASTNode& node) {

View File

@ -192,9 +192,6 @@ private:
int convertArraySize(const Type& type, std::unique_ptr<Expression> s);
bool containsConstantZero(Expression& expr);
bool dividesByZero(Operator op, Expression& right);
std::unique_ptr<Expression> convertBinaryExpression(std::unique_ptr<Expression> left,
Operator op,
std::unique_ptr<Expression> right);
std::unique_ptr<Block> convertBlock(const ASTNode& block);
std::unique_ptr<Statement> convertBreak(const ASTNode& b);
std::unique_ptr<Statement> convertContinue(const ASTNode& c);

View File

@ -319,12 +319,10 @@ std::unique_ptr<Expression> Inliner::inlineExpression(int offset,
switch (expression.kind()) {
case Expression::Kind::kBinary: {
const BinaryExpression& binaryExpr = expression.as<BinaryExpression>();
return std::make_unique<BinaryExpression>(
offset,
expr(binaryExpr.left()),
binaryExpr.getOperator(),
expr(binaryExpr.right()),
binaryExpr.type().clone(symbolTableForExpression));
return BinaryExpression::Make(*fContext,
expr(binaryExpr.left()),
binaryExpr.getOperator(),
expr(binaryExpr.right()));
}
case Expression::Kind::kBoolLiteral:
case Expression::Kind::kIntLiteral:
@ -502,12 +500,11 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
SkASSERT(*resultExpr);
auto assignment = ExpressionStatement::Make(
*fContext,
std::make_unique<BinaryExpression>(
offset,
clone_with_ref_kind(**resultExpr, VariableReference::RefKind::kWrite),
BinaryExpression::Make(
*fContext,
clone_with_ref_kind(**resultExpr, VariableRefKind::kWrite),
Token::Kind::TK_EQ,
expr(r.expression()),
(*resultExpr)->type().clone(symbolTableForStatement)));
expr(r.expression())));
// Early returns are wrapped in a for loop; we need to synthesize a continue statement
// to "leave" the function.
@ -708,12 +705,11 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
&initialValue);
// _1_loop < 1;
std::unique_ptr<Expression> test = std::make_unique<BinaryExpression>(
/*offset=*/-1,
std::unique_ptr<Expression> test = BinaryExpression::Make(
*fContext,
std::make_unique<VariableReference>(/*offset=*/-1, loopVar.fVarSymbol),
Token::Kind::TK_LT,
std::make_unique<IntLiteral>(/*offset=*/-1, /*value=*/1, intType),
fContext->fTypes.fBool.get());
std::make_unique<IntLiteral>(/*offset=*/-1, /*value=*/1, intType));
// _1_loop++
std::unique_ptr<Expression> increment = PostfixExpression::Make(
@ -752,12 +748,10 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
SkASSERT(varMap.find(p) != varMap.end());
inlineStatements->push_back(ExpressionStatement::Make(
*fContext,
std::make_unique<BinaryExpression>(
offset,
clone_with_ref_kind(*arguments[i], VariableReference::RefKind::kWrite),
Token::Kind::TK_EQ,
std::move(varMap[p]),
&arguments[i]->type())));
BinaryExpression::Make(*fContext,
clone_with_ref_kind(*arguments[i], VariableRefKind::kWrite),
Token::Kind::TK_EQ,
std::move(varMap[p]))));
}
if (resultExpr) {

View File

@ -448,9 +448,7 @@ std::unique_ptr<Expression> Rehydrator::expression() {
std::unique_ptr<Expression> left = this->expression();
Token::Kind op = (Token::Kind) this->readU8();
std::unique_ptr<Expression> right = this->expression();
const Type* type = this->type();
return std::make_unique<BinaryExpression>(-1, std::move(left), op, std::move(right),
type);
return BinaryExpression::Make(fContext, std::move(left), op, std::move(right));
}
case Rehydrator::kBoolLiteral_Command: {
bool value = this->readU8();

View File

@ -15,6 +15,7 @@
#include "src/sksl/SkSLIRGenerator.h"
#include "src/sksl/dsl/DSLCore.h"
#include "src/sksl/dsl/DSLErrorHandling.h"
#include "src/sksl/ir/SkSLBinaryExpression.h"
#include "src/sksl/ir/SkSLConstructor.h"
#include "src/sksl/ir/SkSLPostfixExpression.h"
#include "src/sksl/ir/SkSLPrefixExpression.h"
@ -118,7 +119,7 @@ DSLPossibleExpression DSLWriter::Construct(const SkSL::Type& type,
std::unique_ptr<SkSL::Expression> DSLWriter::ConvertBinary(std::unique_ptr<Expression> left,
Operator op,
std::unique_ptr<Expression> right) {
return IRGenerator().convertBinaryExpression(std::move(left), op, std::move(right));
return BinaryExpression::Make(Context(), std::move(left), op, std::move(right));
}
std::unique_ptr<SkSL::Expression> DSLWriter::ConvertField(std::unique_ptr<Expression> base,

File diff suppressed because it is too large Load Diff

View File

@ -1333,7 +1333,6 @@ static uint8_t SKSL_INCLUDE_sksl_public[] = {19,2,
49,138,1,0,1,3,
19,
40,123,0,23,183,209,56,
40,5,1,
39,
49,138,1,0,1,3,1,0,
22,143,1,
@ -1351,7 +1350,6 @@ static uint8_t SKSL_INCLUDE_sksl_public[] = {19,2,
49,141,1,0,1,3,
19,
40,115,0,23,183,209,56,
40,1,1,
39,
49,141,1,0,1,3,1,0,
13,};

View File

@ -0,0 +1,132 @@
/*
* Copyright 2021 Google LLC
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLConstantFolder.h"
#include "src/sksl/ir/SkSLBinaryExpression.h"
#include "src/sksl/ir/SkSLType.h"
namespace SkSL {
std::unique_ptr<Expression> BinaryExpression::Make(const Context& context,
std::unique_ptr<Expression> left,
Operator op,
std::unique_ptr<Expression> right) {
if (!left || !right) {
return nullptr;
}
int offset = left->fOffset;
const Type* rawLeftType;
if (left->is<IntLiteral>() && right->type().isInteger()) {
rawLeftType = &right->type();
} else {
rawLeftType = &left->type();
}
const Type* rawRightType;
if (right->is<IntLiteral>() && left->type().isInteger()) {
rawRightType = &left->type();
} else {
rawRightType = &right->type();
}
if (context.fConfig->strictES2Mode() && !op.isAllowedInStrictES2Mode()) {
context.fErrors.error(offset, String("operator '") + op.operatorName() +
"' is not allowed");
return nullptr;
}
bool isAssignment = op.isAssignment();
if (isAssignment &&
!Analysis::MakeAssignmentExpr(left.get(),
op.kind() != Token::Kind::TK_EQ
? VariableReference::RefKind::kReadWrite
: VariableReference::RefKind::kWrite,
&context.fErrors)) {
return nullptr;
}
const Type* leftType;
const Type* rightType;
const Type* resultType;
if (!op.determineBinaryType(context, *rawLeftType, *rawRightType,
&leftType, &rightType, &resultType)) {
context.fErrors.error(offset, String("type mismatch: '") + op.operatorName() +
"' cannot operate on '" + left->type().displayName() +
"', '" + right->type().displayName() + "'");
return nullptr;
}
if (isAssignment && leftType->componentType().isOpaque()) {
context.fErrors.error(offset, "assignments to opaque type '" + left->type().displayName() +
"' are not permitted");
}
if (context.fConfig->strictES2Mode() && leftType->isOrContainsArray()) {
// Most operators are already rejected on arrays, but GLSL ES 1.0 is very explicit that the
// *only* operator allowed on arrays is subscripting (and the rules against assignment,
// comparison, and even sequence apply to structs containing arrays as well).
context.fErrors.error(offset, String("operator '") + op.operatorName() +
"' can not operate on arrays (or structs containing arrays)");
return nullptr;
}
left = leftType->coerceExpression(std::move(left), context);
right = rightType->coerceExpression(std::move(right), context);
if (!left || !right) {
return nullptr;
}
std::unique_ptr<Expression> result;
if (!ConstantFolder::ErrorOnDivideByZero(context, offset, op, *right)) {
result = ConstantFolder::Simplify(context, offset, *left, op, *right);
}
if (!result) {
result = std::make_unique<BinaryExpression>(offset, std::move(left), op, std::move(right),
resultType);
}
return result;
}
bool BinaryExpression::CheckRef(const Expression& expr) {
switch (expr.kind()) {
case Expression::Kind::kFieldAccess:
return CheckRef(*expr.as<FieldAccess>().base());
case Expression::Kind::kIndex:
return CheckRef(*expr.as<IndexExpression>().base());
case Expression::Kind::kSwizzle:
return CheckRef(*expr.as<Swizzle>().base());
case Expression::Kind::kTernary: {
const TernaryExpression& t = expr.as<TernaryExpression>();
return CheckRef(*t.ifTrue()) && CheckRef(*t.ifFalse());
}
case Expression::Kind::kVariableReference: {
const VariableReference& ref = expr.as<VariableReference>();
return ref.refKind() == VariableRefKind::kWrite ||
ref.refKind() == VariableRefKind::kReadWrite;
}
default:
return false;
}
}
std::unique_ptr<Expression> BinaryExpression::clone() const {
return std::make_unique<BinaryExpression>(fOffset,
this->left()->clone(),
this->getOperator(),
this->right()->clone(),
&this->type());
}
String BinaryExpression::description() const {
return "(" + this->left()->description() +
" " + this->getOperator().operatorName() +
" " + this->right()->description() + ")";
}
} // namespace SkSL

View File

@ -18,29 +18,9 @@
#include "src/sksl/ir/SkSLSwizzle.h"
#include "src/sksl/ir/SkSLTernaryExpression.h"
namespace SkSL {
#include <memory>
static inline bool check_ref(const Expression& expr) {
switch (expr.kind()) {
case Expression::Kind::kFieldAccess:
return check_ref(*expr.as<FieldAccess>().base());
case Expression::Kind::kIndex:
return check_ref(*expr.as<IndexExpression>().base());
case Expression::Kind::kSwizzle:
return check_ref(*expr.as<Swizzle>().base());
case Expression::Kind::kTernary: {
const TernaryExpression& t = expr.as<TernaryExpression>();
return check_ref(*t.ifTrue()) && check_ref(*t.ifFalse());
}
case Expression::Kind::kVariableReference: {
const VariableReference& ref = expr.as<VariableReference>();
return ref.refKind() == VariableReference::RefKind::kWrite ||
ref.refKind() == VariableReference::RefKind::kReadWrite;
}
default:
return false;
}
}
namespace SkSL {
/**
* A binary operation.
@ -49,16 +29,22 @@ class BinaryExpression final : public Expression {
public:
static constexpr Kind kExpressionKind = Kind::kBinary;
// Use BinaryExpression::Make to get a potentially-simplified form of the expression.
BinaryExpression(int offset, std::unique_ptr<Expression> left, Operator op,
std::unique_ptr<Expression> right, const Type* type)
: INHERITED(offset, kExpressionKind, type)
, fLeft(std::move(left))
, fOperator(op)
, fRight(std::move(right)) {
// If we are assigning to a VariableReference, ensure that it is set to Write or ReadWrite
SkASSERT(!op.isAssignment() || check_ref(*this->left()));
: INHERITED(offset, kExpressionKind, type)
, fLeft(std::move(left))
, fOperator(op)
, fRight(std::move(right)) {
// If we are assigning to a VariableReference, ensure that it is set to Write or ReadWrite.
SkASSERT(!op.isAssignment() || CheckRef(*this->left()));
}
static std::unique_ptr<Expression> Make(const Context& context,
std::unique_ptr<Expression> left,
Operator op,
std::unique_ptr<Expression> right);
std::unique_ptr<Expression>& left() {
return fLeft;
}
@ -96,21 +82,13 @@ public:
return this->left()->hasProperty(property) || this->right()->hasProperty(property);
}
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new BinaryExpression(fOffset,
this->left()->clone(),
this->getOperator(),
this->right()->clone(),
&this->type()));
}
std::unique_ptr<Expression> clone() const override;
String description() const override {
return "(" + this->left()->description() + " " +
this->getOperator().operatorName() + " " + this->right()->description() +
")";
}
String description() const override;
private:
static bool CheckRef(const Expression& expr);
std::unique_ptr<Expression> fLeft;
Operator fOperator;
std::unique_ptr<Expression> fRight;