Omit dead SkSL functions
Now that SkSL inlines functions, dead functions are very common. This
change causes them to be omitted from the final output.
Change-Id: Ie466a3f748812eff1a368498365c89d73ab0b7be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292684
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
diff --git a/modules/particles/src/SkParticleEffect.cpp b/modules/particles/src/SkParticleEffect.cpp
index 1cca060..55c1bba 100644
--- a/modules/particles/src/SkParticleEffect.cpp
+++ b/modules/particles/src/SkParticleEffect.cpp
@@ -121,6 +121,7 @@
auto buildProgram = [this](const SkSL::String& code, Program* p) {
SkSL::Compiler compiler;
SkSL::Program::Settings settings;
+ settings.fRemoveDeadFunctions = false;
SkTArray<std::unique_ptr<SkParticleExternalValue>> externalValues;
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 9916d6d..626b5ec 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -1448,6 +1448,20 @@
this->scanCFG((FunctionDefinition&) element);
}
}
+ // we wait until after analysis to remove dead functions so that we still report errors
+ // even in unused code
+ if (program.fSettings.fRemoveDeadFunctions) {
+ for (auto iter = program.fElements.begin(); iter != program.fElements.end(); ) {
+ if ((*iter)->fKind == ProgramElement::kFunction_Kind) {
+ const FunctionDefinition& f = (const FunctionDefinition&) **iter;
+ if (!f.fDeclaration.fCallCount && f.fDeclaration.fName != "main") {
+ iter = program.fElements.erase(iter);
+ continue;
+ }
+ }
+ ++iter;
+ }
+ }
if (program.fKind != Program::kFragmentProcessor_Kind) {
for (auto iter = program.fElements.begin(); iter != program.fElements.end();) {
if ((*iter)->fKind == ProgramElement::kVar_Kind) {
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 8fa91b3..48ef4bb 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -2061,10 +2061,15 @@
}
std::unique_ptr<Expression> initialValue = expr(decl.fValue);
const Variable* old = decl.fVar;
+ // need to copy the var name in case the originating function is discarded and we lose
+ // its symbols
+ std::unique_ptr<String> name(new String(old->fName));
+ String* namePtr = (String*) fSymbolTable->takeOwnership(std::move(name));
Variable* clone = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
- new Variable(offset, old->fModifiers, old->fName,
- old->fType, old->fStorage,
- initialValue.get())));
+ new Variable(offset, old->fModifiers,
+ namePtr->c_str(),
+ old->fType, old->fStorage,
+ initialValue.get())));
(*varMap)[old] = clone;
return std::unique_ptr<Statement>(new VarDeclaration(clone, std::move(sizes),
std::move(initialValue)));
diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h
index f575879..7f7e7c5 100644
--- a/src/sksl/ir/SkSLFunctionCall.h
+++ b/src/sksl/ir/SkSLFunctionCall.h
@@ -21,7 +21,13 @@
std::vector<std::unique_ptr<Expression>> arguments)
: INHERITED(offset, kFunctionCall_Kind, type)
, fFunction(std::move(function))
- , fArguments(std::move(arguments)) {}
+ , fArguments(std::move(arguments)) {
+ ++fFunction.fCallCount;
+ }
+
+ ~FunctionCall() override {
+ --fFunction.fCallCount;
+ }
bool hasProperty(Property property) const override {
if (property == Property::kSideEffects && (fFunction.fModifiers.fFlags &
diff --git a/src/sksl/ir/SkSLFunctionDeclaration.h b/src/sksl/ir/SkSLFunctionDeclaration.h
index 0301ed7..d11cc46 100644
--- a/src/sksl/ir/SkSLFunctionDeclaration.h
+++ b/src/sksl/ir/SkSLFunctionDeclaration.h
@@ -114,6 +114,7 @@
Modifiers fModifiers;
const std::vector<const Variable*> fParameters;
const Type& fReturnType;
+ mutable int fCallCount = 0;
typedef Symbol INHERITED;
};
diff --git a/src/sksl/ir/SkSLProgram.h b/src/sksl/ir/SkSLProgram.h
index ed15866..6488fc4 100644
--- a/src/sksl/ir/SkSLProgram.h
+++ b/src/sksl/ir/SkSLProgram.h
@@ -111,6 +111,9 @@
// binding and set number of the uniform buffer.
int fRTHeightBinding = -1;
int fRTHeightSet = -1;
+ // If true, remove any uncalled functions other than main(). Note that a function which
+ // starts out being used may end up being uncalled after optimization.
+ bool fRemoveDeadFunctions = true;
std::unordered_map<String, Value> fArgs;
};
diff --git a/tests/SkSLFPTest.cpp b/tests/SkSLFPTest.cpp
index 23c4fd7..1baa887 100644
--- a/tests/SkSLFPTest.cpp
+++ b/tests/SkSLFPTest.cpp
@@ -14,6 +14,7 @@
std::vector<const char*> expectedH, std::vector<const char*> expectedCPP) {
SkSL::Program::Settings settings;
settings.fCaps = ∩︀
+ settings.fRemoveDeadFunctions = false;
SkSL::Compiler compiler;
SkSL::StringStream output;
std::unique_ptr<SkSL::Program> program = compiler.convertProgram(
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index 9629cc2..a83e2e8 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -104,22 +104,6 @@
*SkSL::ShaderCapsFactory::Default(),
"#version 400\n"
"out vec4 sk_FragColor;\n"
- "float foo(float v[2]) {\n"
- " return v[0] * v[1];\n"
- "}\n"
- "void bar(inout float x) {\n"
- " float y[2], z;\n"
- " y[0] = x;\n"
- " y[1] = x * 2.0;\n"
- " float inlineResult117;\n"
- " float[2] inlineArg117_0 = y;\n"
- " {\n"
- " inlineResult117 = inlineArg117_0[0] * inlineArg117_0[1];\n"
- " }\n"
- " z = inlineResult117;\n"
- "\n"
- " x = z;\n"
- "}\n"
"void main() {\n"
" float x = 10.0;\n"
" float inlineArg161_0 = x;\n"
@@ -1796,6 +1780,7 @@
"EmitVertex();"
"}"
"void main() {"
+ "test();"
"sk_Position = sk_in[0].sk_Position + float4(-0.5, 0, 0, sk_InvocationID);"
"EmitVertex();"
"}",
@@ -1804,11 +1789,12 @@
"int sk_InvocationID;\n"
"layout (points) in ;\n"
"layout (line_strip, max_vertices = 4) out ;\n"
- "void test() {\n"
- " gl_Position = gl_in[0].gl_Position + vec4(0.5, 0.0, 0.0, float(sk_InvocationID));\n"
- " EmitVertex();\n"
- "}\n"
"void _invoke() {\n"
+ " {\n"
+ " gl_Position = gl_in[0].gl_Position + vec4(0.5, 0.0, 0.0, float(sk_InvocationID));\n"
+ " EmitVertex();\n"
+ " }\n"
+ "\n"
" gl_Position = gl_in[0].gl_Position + vec4(-0.5, 0.0, 0.0, float(sk_InvocationID));\n"
" EmitVertex();\n"
"}\n"
diff --git a/tests/SkSLInterpreterTest.cpp b/tests/SkSLInterpreterTest.cpp
index 3609777..3541ded 100644
--- a/tests/SkSLInterpreterTest.cpp
+++ b/tests/SkSLInterpreterTest.cpp
@@ -622,6 +622,7 @@
SkSL::Compiler compiler;
SkSL::Program::Settings settings;
+ settings.fRemoveDeadFunctions = false;
std::unique_ptr<SkSL::Program> program = compiler.convertProgram(
SkSL::Program::kGeneric_Kind,
SkSL::String(src), settings);
@@ -763,6 +764,7 @@
SkSL::Compiler compiler;
SkSL::Program::Settings settings;
+ settings.fRemoveDeadFunctions = false;
std::unique_ptr<SkSL::Program> program = compiler.convertProgram(
SkSL::Program::kGeneric_Kind,
SkSL::String(src), settings);