From f0cb733b1b1c87a7c17ea365e4e1248301ffe4a6 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Fri, 8 Jan 2021 18:39:00 -0500 Subject: [PATCH] Remove redundant constantPropagate in Swizzle. Swizzle::constantPropagate is a limited form of this optimization: https://osscs.corp.google.com/skia/skia/+/master:src/sksl/SkSLCompiler.cpp;l=1159 It predates this optimization code, though. It also flattened out an unnecessary constructor that would have otherwise required an extra pass to eliminate. Updated the optimization code at L1159 to simplify away the unnecessary constructor where possible, and then removed Swizzle::constantPropagate. This has no effect on generated code. Change-Id: I0f43d5c51761965230c853f309a6ef068f9aef77 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352120 Commit-Queue: Brian Osman Reviewed-by: Brian Osman Auto-Submit: John Stiles --- src/sksl/SkSLCompiler.cpp | 16 ++++++++++------ src/sksl/ir/SkSLSwizzle.h | 23 ----------------------- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 6ac8f33199..cc963621f7 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -1158,12 +1158,16 @@ void Compiler::simplifyExpression(DefinitionMap& definitions, if (base.arguments().size() == 1 && base.arguments().front()->type().isScalar()) { // `half4(scalar).zyy` can be optimized to `half3(scalar)`. The swizzle // components don't actually matter since all fields are the same. - ExpressionArray newArgs; - newArgs.push_back(base.arguments().front()->clone()); - replacement = std::make_unique( - base.fOffset, - &componentType.toCompound(*fContext, swizzleSize, /*rows=*/1), - std::move(newArgs)); + const Expression& argument = *base.arguments().front(); + const Type& constructorType = componentType.toCompound(*fContext, swizzleSize, + /*rows=*/1); + replacement = Constructor::SimplifyConversion(constructorType, argument); + if (!replacement) { + ExpressionArray newArgs; + newArgs.push_back(argument.clone()); + replacement = std::make_unique(base.fOffset, &constructorType, + std::move(newArgs)); + } // We're replacing an expression with a cloned version; we'll need a rescan. // There's no fUsage change: `half4(foo).xy` and `half2(foo)` have equivalent diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h index af61404a07..577e9ae1a0 100644 --- a/src/sksl/ir/SkSLSwizzle.h +++ b/src/sksl/ir/SkSLSwizzle.h @@ -43,29 +43,6 @@ struct Swizzle final : public Expression { return fComponents; } - std::unique_ptr constantPropagate(const IRGenerator& irGenerator, - const DefinitionMap& definitions) override { - if (this->base()->is()) { - Constructor& constructor = this->base()->as(); - if (constructor.isCompileTimeConstant()) { - // we're swizzling a constant vector, e.g. float4(1).x. Simplify it. - const Type& type = this->type(); - if (type.isInteger()) { - SkASSERT(this->components().size() == 1); - SKSL_INT value = constructor.getIVecComponent(this->components()[0]); - return std::make_unique(irGenerator.fContext, constructor.fOffset, - value); - } else if (type.isFloat()) { - SkASSERT(this->components().size() == 1); - SKSL_FLOAT value = constructor.getFVecComponent(this->components()[0]); - return std::make_unique(irGenerator.fContext, constructor.fOffset, - value); - } - } - } - return nullptr; - } - bool hasProperty(Property property) const override { return this->base()->hasProperty(property); }