Assign unique names to inlined variable declarations.
Change-Id: I6097f50e7be1fa2f7772f6c454410ecbf3470eea
Bug: skia:10722
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316977
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index c59d805..5ede14d 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -263,6 +263,28 @@
fInlineVarCounter = 0;
}
+String Inliner::uniqueNameForInlineVar(const String& baseName, SymbolTable* symbolTable) {
+ // If the base name starts with an underscore, like "_coords", we can't append another
+ // underscore, because OpenGL disallows two consecutive underscores anywhere in the string. But
+ // in the general case, using the underscore as a splitter reads nicely enough that it's worth
+ // putting in this special case.
+ const char* splitter = baseName.startsWith("_") ? "" : "_";
+
+ // Append a unique numeric prefix to avoid name overlap. Check the symbol table to make sure
+ // we're not reusing an existing name. (Note that within a single compilation pass, this check
+ // isn't fully comprehensive, as code isn't always generated in top-to-bottom order.)
+ String uniqueName;
+ for (;;) {
+ uniqueName = String::printf("_%d%s%s", fInlineVarCounter++, splitter, baseName.c_str());
+ StringFragment frag{uniqueName.data(), uniqueName.length()};
+ if ((*symbolTable)[frag] == nullptr) {
+ break;
+ }
+ }
+
+ return uniqueName;
+}
+
std::unique_ptr<Expression> Inliner::inlineExpression(int offset,
VariableRewriteMap* varMap,
const Expression& expression) {
@@ -466,9 +488,11 @@
}
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));
+ // We assign unique names to inlined variables--scopes hide most of the problems in this
+ // regard, but see `InlinerAvoidsVariableNameOverlap` for a counterexample where unique
+ // names are important.
+ auto name = std::make_unique<String>(
+ this->uniqueNameForInlineVar(String(old->fName), symbolTableForStatement));
const String* namePtr = symbolTableForStatement->takeOwnershipOfString(std::move(name));
const Type* typePtr = copy_if_needed(&old->type(), *symbolTableForStatement);
const Variable* clone = symbolTableForStatement->takeOwnershipOfSymbol(
@@ -551,25 +575,8 @@
type = fContext->fInt_Type.get();
}
- // If the base name starts with an underscore, like "_coords", we can't append another
- // underscore, because some OpenGL platforms error out when they see two consecutive
- // underscores (anywhere in the string!). But in the general case, using the underscore as
- // a splitter reads nicely enough that it's worth putting in this special case.
- const char* splitter = baseName.startsWith("_") ? "_X" : "_";
-
- // Append a unique numeric prefix to avoid name overlap. Check the symbol table to make sure
- // we're not reusing an existing name. (Note that within a single compilation pass, this
- // check isn't fully comprehensive, as code isn't always generated in top-to-bottom order.)
- String uniqueName;
- for (;;) {
- uniqueName = String::printf("_%d%s%s", fInlineVarCounter++, splitter, baseName.c_str());
- StringFragment frag{uniqueName.data(), uniqueName.length()};
- if ((*symbolTableForCall)[frag] == nullptr) {
- break;
- }
- }
-
- // Add our new variable's name to the symbol table.
+ // Provide our new variable with a unique name, and add it to our symbol table.
+ String uniqueName = this->uniqueNameForInlineVar(baseName, symbolTableForCall);
const String* namePtr = symbolTableForCall->takeOwnershipOfString(
std::make_unique<String>(std::move(uniqueName)));
StringFragment nameFrag{namePtr->c_str(), namePtr->length()};
diff --git a/src/sksl/SkSLInliner.h b/src/sksl/SkSLInliner.h
index f8dad30..9c1e043 100644
--- a/src/sksl/SkSLInliner.h
+++ b/src/sksl/SkSLInliner.h
@@ -55,6 +55,8 @@
private:
using VariableRewriteMap = std::unordered_map<const Variable*, const Variable*>;
+ String uniqueNameForInlineVar(const String& baseName, SymbolTable* symbolTable);
+
std::unique_ptr<Expression> inlineExpression(int offset,
VariableRewriteMap* varMap,
const Expression& expression);
diff --git a/tests/SkSLInlinerTest.cpp b/tests/SkSLInlinerTest.cpp
index e901cd1..be12d7b 100644
--- a/tests/SkSLInlinerTest.cpp
+++ b/tests/SkSLInlinerTest.cpp
@@ -725,13 +725,13 @@
void main() {
vec4 _0_switchy;
{
- vec4 result;
+ vec4 _1_result;
switch (int(color.x)) {
case 0:
- result = color.yyyy;
+ _1_result = color.yyyy;
}
- result = color.zzzz;
- _0_switchy = result;
+ _1_result = color.zzzz;
+ _0_switchy = _1_result;
}
sk_FragColor = _0_switchy;
@@ -793,16 +793,16 @@
void main() {
vec4 _0_switchy;
{
- vec4 result;
+ vec4 _1_result;
switch (int(color.x)) {
case 1:
- result = color.yyyy;
+ _1_result = color.yyyy;
break;
default:
- result = color.zzzz;
+ _1_result = color.zzzz;
break;
}
- _0_switchy = result;
+ _0_switchy = _1_result;
}
sk_FragColor = _0_switchy;
@@ -834,12 +834,12 @@
void main() {
vec4 _0_loopy;
{
- vec4 pix;
- for (int x = 0;x < 5; ++x) {
- if (x == int(color.w)) pix = color.yyyy;
+ vec4 _1_pix;
+ for (int _2_x = 0;_2_x < 5; ++_2_x) {
+ if (_2_x == int(color.w)) _1_pix = color.yyyy;
}
- pix = color.zzzz;
- _0_loopy = pix;
+ _1_pix = color.zzzz;
+ _0_loopy = _1_pix;
}
sk_FragColor = _0_loopy;
@@ -873,88 +873,88 @@
/*expectedGLSL=*/R"__GLSL__(#version 400
uniform vec4 color;
vec4 main() {
- float _2_fma;
- float _3_a = color.x;
- float _4_b = color.y;
- float _5_c = color.z;
+ float _3_fma;
+ float _4_a = color.x;
+ float _5_b = color.y;
+ float _6_c = color.z;
{
- float _0_mul;
+ float _7_0_mul;
{
- _0_mul = _3_a * _4_b;
+ _7_0_mul = _4_a * _5_b;
}
- float _1_add;
+ float _8_1_add;
{
- float c = _0_mul + _5_c;
- _1_add = c;
+ float _9_2_c = _7_0_mul + _6_c;
+ _8_1_add = _9_2_c;
}
- _2_fma = _1_add;
+ _3_fma = _8_1_add;
}
- float a = _2_fma;
-
- float _6_fma;
- float _7_a = color.y;
- float _8_b = color.z;
- float _9_c = color.w;
- {
- float _0_mul;
- {
- _0_mul = _7_a * _8_b;
- }
-
- float _1_add;
- {
- float c = _0_mul + _9_c;
- _1_add = c;
- }
-
- _6_fma = _1_add;
-
- }
-
- float b = _6_fma;
+ float a = _3_fma;
float _10_fma;
- float _11_a = color.z;
- float _12_b = color.w;
- float _13_c = color.x;
+ float _11_a = color.y;
+ float _12_b = color.z;
+ float _13_c = color.w;
{
- float _0_mul;
+ float _14_0_mul;
{
- _0_mul = _11_a * _12_b;
+ _14_0_mul = _11_a * _12_b;
}
- float _1_add;
+ float _15_1_add;
{
- float c = _0_mul + _13_c;
- _1_add = c;
+ float _16_2_c = _14_0_mul + _13_c;
+ _15_1_add = _16_2_c;
}
- _10_fma = _1_add;
+ _10_fma = _15_1_add;
}
- float c = _10_fma;
+ float b = _10_fma;
- float _14_mul;
+ float _17_fma;
+ float _18_a = color.z;
+ float _19_b = color.w;
+ float _20_c = color.x;
{
- _14_mul = c * c;
+ float _21_0_mul;
+ {
+ _21_0_mul = _18_a * _19_b;
+ }
+
+ float _22_1_add;
+ {
+ float _23_2_c = _21_0_mul + _20_c;
+ _22_1_add = _23_2_c;
+ }
+
+ _17_fma = _22_1_add;
+
}
- float _15_mul;
+ float c = _17_fma;
+
+ float _24_mul;
{
- _15_mul = b * c;
+ _24_mul = c * c;
}
- float _16_mul;
+ float _25_mul;
{
- _16_mul = a * _15_mul;
+ _25_mul = b * c;
}
- return vec4(a, b, _14_mul, _16_mul);
+ float _26_mul;
+ {
+ _26_mul = a * _25_mul;
+ }
+
+ return vec4(a, b, _24_mul, _26_mul);
}
)__GLSL__");
@@ -1108,8 +1108,8 @@
{
{
if (color.x > 0.0) {
- vec4 d = color * 0.5;
- _2_branchyAndBlocky = d.xxxx;
+ vec4 _3_d = color * 0.5;
+ _2_branchyAndBlocky = _3_d.xxxx;
} else {
{
{
diff --git a/tests/sksl/glsl/golden/Functions.glsl b/tests/sksl/glsl/golden/Functions.glsl
index 77919a3..789e8c1 100644
--- a/tests/sksl/glsl/golden/Functions.glsl
+++ b/tests/sksl/glsl/golden/Functions.glsl
@@ -5,17 +5,17 @@
void main() {
highp float x = 10.0;
{
- highp float y[2], z;
- y[0] = 10.0;
- y[1] = 20.0;
- highp float _0_foo;
+ highp float _1_y[2], _2_z;
+ _1_y[0] = 10.0;
+ _1_y[1] = 20.0;
+ highp float _3_0_foo;
{
- _0_foo = y[0] * y[1];
+ _3_0_foo = _1_y[0] * _1_y[1];
}
- z = _0_foo;
+ _2_z = _3_0_foo;
- x = z;
+ x = _2_z;
}
diff --git a/tests/sksl/inliner/golden/InlinerAvoidsVariableNameOverlap.glsl b/tests/sksl/inliner/golden/InlinerAvoidsVariableNameOverlap.glsl
index 544b8fa..5c69301 100644
--- a/tests/sksl/inliner/golden/InlinerAvoidsVariableNameOverlap.glsl
+++ b/tests/sksl/inliner/golden/InlinerAvoidsVariableNameOverlap.glsl
@@ -3,19 +3,19 @@
precision mediump sampler2D;
in mediump vec2 x;
mediump vec4 main() {
- mediump vec2 _1_InlineA;
+ mediump vec2 _2_InlineA;
{
- mediump vec2 reusedName = x + vec2(1.0, 2.0);
- mediump vec2 _0_InlineB;
+ mediump vec2 _3_reusedName = x + vec2(1.0, 2.0);
+ mediump vec2 _4_0_InlineB;
{
- mediump vec2 reusedName = reusedName + vec2(3.0, 4.0);
- _0_InlineB = reusedName;
+ mediump vec2 _5_1_reusedName = _3_reusedName + vec2(3.0, 4.0);
+ _4_0_InlineB = _5_1_reusedName;
}
- _1_InlineA = _0_InlineB;
+ _2_InlineA = _4_0_InlineB;
}
- return _1_InlineA.xyxy;
+ return _2_InlineA.xyxy;
}
