Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 648a81e

Browse files
johnstiles-googleSkia Commit-Bot
johnstiles-google
authored and
Skia Commit-Bot
committed
Add unit tests for the inliner.
Some of these tests worked perfectly, but others expose latent inlining bugs. The following tests show incorrect results from the inliner: - SkSLFPTernaryExpressionsShouldNotInlineResults - SkSLFPInlinedWhileBodyMustBeInAScope - SkSLFPInlinedDoWhileBodyMustBeInAScope - SkSLFPInlinedForBodyMustBeInAScope Change-Id: I523e2e3272dea01d5c194a478df6f39ecadf2f5c Bug: skia:10687, skia:10688 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314796 Auto-Submit: John Stiles <johnstiles@google.com> Reviewed-by: Brian Osman <brianosman@google.com> Commit-Queue: Brian Osman <brianosman@google.com> Commit-Queue: John Stiles <johnstiles@google.com>
1 parent a171109 commit 648a81e

File tree

2 files changed

+322
-0
lines changed

2 files changed

+322
-0
lines changed

‎src/sksl/SkSLCPPCodeGenerator.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,8 @@ static const char* glsltype_string(const Context& context, const Type& type) {
581581
return "kHalf4x4_GrSLType";
582582
} else if (type == *context.fVoid_Type) {
583583
return "kVoid_GrSLType";
584+
} else if (type == *context.fBool_Type) {
585+
return "kBool_GrSLType";
584586
} else if (type.kind() == Type::kEnum_Kind) {
585587
return "int";
586588
}

‎tests/SkSLFPTest.cpp‎

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,326 @@ R"SkSL(%s = %s(%s);
10871087
)__Cpp__"});
10881088
}
10891089

1090+
DEF_TEST(SkSLFPTernaryExpressionsShouldNotInlineResults, r) {
1091+
// NOTE: this test exposes a bug with the inliner. We don't want to inline both sides of a
1092+
// ternary, since only one side needs to be evaluated, and side effects from the wrong side
1093+
// must not occur.
1094+
test(r,
1095+
*SkSL::ShaderCapsFactory::Default(),
1096+
R"__SkSL__(
1097+
uniform half4 color;
1098+
half count = 0;
1099+
bool test(half4 v) {
1100+
return v.x <= 0.5;
1101+
}
1102+
half4 trueSide(half4 v) {
1103+
count += 1;
1104+
return half4(sin(v.x), sin(v.y), sin(v.z), sin(v.w));
1105+
}
1106+
half4 falseSide(half4 v) {
1107+
count += 1;
1108+
return half4(cos(v.y), cos(v.z), cos(v.w), cos(v.z));
1109+
}
1110+
void main() {
1111+
sk_OutColor = test(color) ? trueSide(color) : falseSide(color);
1112+
sk_OutColor *= count;
1113+
}
1114+
)__SkSL__",
1115+
/*expectedH=*/{},
1116+
/*expectedCPP=*/{
1117+
R"__Cpp__(fragBuilder->codeAppendf(
1118+
R"SkSL(half count = %f;
1119+
bool _0_test;
1120+
{
1121+
_0_test = %s.x <= 0.5;
1122+
}
1123+
1124+
half4 _1_trueSide;
1125+
{
1126+
count += 1.0;
1127+
_1_trueSide = half4(sin(%s.x), sin(%s.y), sin(%s.z), sin(%s.w));
1128+
}
1129+
1130+
half4 _2_falseSide;
1131+
{
1132+
count += 1.0;
1133+
_2_falseSide = half4(cos(%s.y), cos(%s.z), cos(%s.w), cos(%s.z));
1134+
}
1135+
1136+
%s = _0_test ? _1_trueSide : _2_falseSide;
1137+
1138+
%s *= count;
1139+
)SkSL"
1140+
, count, args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fOutputColor, args.fOutputColor);
1141+
)__Cpp__"});
1142+
}
1143+
1144+
DEF_TEST(SkSLFPShortCircuitEvaluationsCannotInlineRightHandSide, r) {
1145+
test(r,
1146+
*SkSL::ShaderCapsFactory::Default(),
1147+
R"__SkSL__(
1148+
uniform half4 color;
1149+
bool testA(half4 v) {
1150+
return v.x <= 0.5;
1151+
}
1152+
bool testB(half4 v) {
1153+
return v.x > 0.5;
1154+
}
1155+
void main() {
1156+
sk_OutColor = half4(0);
1157+
if (testA(color) && testB(color)) {
1158+
sk_OutColor = half4(0.5);
1159+
}
1160+
if (testB(color) || testA(color)) {
1161+
sk_OutColor = half4(1.0);
1162+
}
1163+
}
1164+
)__SkSL__",
1165+
/*expectedH=*/{},
1166+
/*expectedCPP=*/{
1167+
R"__Cpp__( SkString testA_name;
1168+
const GrShaderVar testA_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
1169+
fragBuilder->emitFunction(kBool_GrSLType, "testA", 1, testA_args,
1170+
R"SkSL(return v.x <= 0.5;
1171+
)SkSL", &testA_name);
1172+
SkString testB_name;
1173+
const GrShaderVar testB_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
1174+
fragBuilder->emitFunction(kBool_GrSLType, "testB", 1, testB_args,
1175+
R"SkSL(return v.x > 0.5;
1176+
)SkSL", &testB_name);
1177+
fragBuilder->codeAppendf(
1178+
R"SkSL(%s = half4(0.0);
1179+
bool _0_testA;
1180+
{
1181+
_0_testA = %s.x <= 0.5;
1182+
}
1183+
1184+
if (_0_testA && %s(%s)) {
1185+
%s = half4(0.5);
1186+
}
1187+
1188+
bool _1_testB;
1189+
{
1190+
_1_testB = %s.x > 0.5;
1191+
}
1192+
1193+
if (_1_testB || %s(%s)) {
1194+
%s = half4(1.0);
1195+
}
1196+
1197+
)SkSL"
1198+
, args.fOutputColor, args.fUniformHandler->getUniformCStr(colorVar), testB_name.c_str(), args.fUniformHandler->getUniformCStr(colorVar), args.fOutputColor, args.fUniformHandler->getUniformCStr(colorVar), testA_name.c_str(), args.fUniformHandler->getUniformCStr(colorVar), args.fOutputColor);
1199+
)__Cpp__"});
1200+
}
1201+
1202+
DEF_TEST(SkSLFPWhileTestCannotBeInlined, r) {
1203+
test(r,
1204+
*SkSL::ShaderCapsFactory::Default(),
1205+
R"__SkSL__(
1206+
bool shouldLoop(half4 v) {
1207+
return v.x < 0.5;
1208+
}
1209+
void main() {
1210+
sk_OutColor = half4(0);
1211+
while (shouldLoop(sk_OutColor)) {
1212+
sk_OutColor += half4(0.125);
1213+
}
1214+
}
1215+
)__SkSL__",
1216+
/*expectedH=*/{},
1217+
/*expectedCPP=*/{
1218+
R"__Cpp__(SkString shouldLoop_name;
1219+
const GrShaderVar shouldLoop_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
1220+
fragBuilder->emitFunction(kBool_GrSLType, "shouldLoop", 1, shouldLoop_args,
1221+
R"SkSL(return v.x < 0.5;
1222+
)SkSL", &shouldLoop_name);
1223+
fragBuilder->codeAppendf(
1224+
R"SkSL(%s = half4(0.0);
1225+
while (%s(%s)) {
1226+
%s += half4(0.125);
1227+
}
1228+
)SkSL"
1229+
, args.fOutputColor, shouldLoop_name.c_str(), args.fOutputColor, args.fOutputColor);
1230+
)__Cpp__"});
1231+
}
1232+
1233+
DEF_TEST(SkSLFPInlinedWhileBodyMustBeInAScope, r) {
1234+
// NOTE: this test exposes a bug with the inliner. The inlined function body is not wrapped in a
1235+
// scope, so the inlined code is emitted outside of the while loop.
1236+
test(r,
1237+
*SkSL::ShaderCapsFactory::Default(),
1238+
R"__SkSL__(
1239+
half4 adjust(half4 v) {
1240+
return v + half4(0.125);
1241+
}
1242+
void main() {
1243+
sk_OutColor = half4(0);
1244+
while (sk_OutColor.x < 0.5)
1245+
sk_OutColor = adjust(sk_OutColor);
1246+
}
1247+
)__SkSL__",
1248+
/*expectedH=*/{},
1249+
/*expectedCPP=*/{
1250+
R"__Cpp__(fragBuilder->emitFunction(kHalf4_GrSLType, "adjust", 1, adjust_args,
1251+
R"SkSL(return v + half4(0.125);
1252+
)SkSL", &adjust_name);
1253+
fragBuilder->codeAppendf(
1254+
R"SkSL(%s = half4(0.0);
1255+
while (%s.x < 0.5) half4 _0_adjust;
1256+
{
1257+
_0_adjust = %s + half4(0.125);
1258+
}
1259+
1260+
%s = _0_adjust;
1261+
1262+
)SkSL"
1263+
, args.fOutputColor, args.fOutputColor, args.fOutputColor, args.fOutputColor);
1264+
)__Cpp__"});
1265+
}
1266+
1267+
DEF_TEST(SkSLFPDoWhileTestCannotBeInlined, r) {
1268+
test(r,
1269+
*SkSL::ShaderCapsFactory::Default(),
1270+
R"__SkSL__(
1271+
bool shouldLoop(half4 v) {
1272+
return v.x < 0.5;
1273+
}
1274+
void main() {
1275+
sk_OutColor = half4(0);
1276+
do {
1277+
sk_OutColor += half4(0.125);
1278+
} while (shouldLoop(sk_OutColor));
1279+
}
1280+
)__SkSL__",
1281+
/*expectedH=*/{},
1282+
/*expectedCPP=*/{
1283+
R"__Cpp__(SkString shouldLoop_name;
1284+
const GrShaderVar shouldLoop_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
1285+
fragBuilder->emitFunction(kBool_GrSLType, "shouldLoop", 1, shouldLoop_args,
1286+
R"SkSL(return v.x < 0.5;
1287+
)SkSL", &shouldLoop_name);
1288+
fragBuilder->codeAppendf(
1289+
R"SkSL(%s = half4(0.0);
1290+
do {
1291+
%s += half4(0.125);
1292+
} while (%s(%s));
1293+
)SkSL"
1294+
, args.fOutputColor, args.fOutputColor, shouldLoop_name.c_str(), args.fOutputColor);
1295+
)__Cpp__"});
1296+
}
1297+
1298+
DEF_TEST(SkSLFPInlinedDoWhileBodyMustBeInAScope, r) {
1299+
// NOTE: this test exposes a bug with the inliner. The inlined function body is not wrapped in a
1300+
// scope, so the emitted inlined code is just invalid.
1301+
test(r,
1302+
*SkSL::ShaderCapsFactory::Default(),
1303+
R"__SkSL__(
1304+
half4 adjust(half4 v) {
1305+
return v + half4(0.125);
1306+
}
1307+
void main() {
1308+
sk_OutColor = half4(0);
1309+
do
1310+
sk_OutColor = adjust(sk_OutColor);
1311+
while (sk_OutColor.x < 0.5);
1312+
}
1313+
)__SkSL__",
1314+
/*expectedH=*/{},
1315+
/*expectedCPP=*/{
1316+
R"__Cpp__(fragBuilder->emitFunction(kHalf4_GrSLType, "adjust", 1, adjust_args,
1317+
R"SkSL(return v + half4(0.125);
1318+
)SkSL", &adjust_name);
1319+
fragBuilder->codeAppendf(
1320+
R"SkSL(%s = half4(0.0);
1321+
do half4 _0_adjust;
1322+
{
1323+
_0_adjust = %s + half4(0.125);
1324+
}
1325+
1326+
%s = _0_adjust;
1327+
while (%s.x < 0.5);
1328+
)SkSL"
1329+
, args.fOutputColor, args.fOutputColor, args.fOutputColor, args.fOutputColor);
1330+
)__Cpp__"});
1331+
}
1332+
1333+
DEF_TEST(SkSLFPOnlyForInitializerExpressionsCanBeInlined, r) {
1334+
test(r,
1335+
*SkSL::ShaderCapsFactory::Default(),
1336+
R"__SkSL__(
1337+
half4 initLoopVar() {
1338+
return half4(0.0625);
1339+
}
1340+
bool shouldLoop(half4 v) {
1341+
return v.x < 0.5;
1342+
}
1343+
half4 grow(half4 v) {
1344+
return v + half4(0.125);
1345+
}
1346+
void main() {
1347+
for (sk_OutColor = initLoopVar();
1348+
shouldLoop(sk_OutColor);
1349+
sk_OutColor = grow(sk_OutColor)) {
1350+
}
1351+
}
1352+
)__SkSL__",
1353+
/*expectedH=*/{},
1354+
/*expectedCPP=*/{
1355+
R"__Cpp__(SkString initLoopVar_name;
1356+
const GrShaderVar initLoopVar_args[] = { };
1357+
fragBuilder->emitFunction(kHalf4_GrSLType, "initLoopVar", 0, initLoopVar_args,
1358+
R"SkSL(return half4(0.0625);
1359+
)SkSL", &initLoopVar_name);
1360+
SkString shouldLoop_name;
1361+
const GrShaderVar shouldLoop_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
1362+
fragBuilder->emitFunction(kBool_GrSLType, "shouldLoop", 1, shouldLoop_args,
1363+
R"SkSL(return v.x < 0.5;
1364+
)SkSL", &shouldLoop_name);
1365+
SkString grow_name;
1366+
const GrShaderVar grow_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
1367+
fragBuilder->emitFunction(kHalf4_GrSLType, "grow", 1, grow_args,
1368+
R"SkSL(return v + half4(0.125);
1369+
)SkSL", &grow_name);
1370+
fragBuilder->codeAppendf(
1371+
R"SkSL(for (%s = half4(0.0625);
1372+
%s(%s); %s = %s(%s)) {
1373+
}
1374+
)SkSL"
1375+
, args.fOutputColor, shouldLoop_name.c_str(), args.fOutputColor, args.fOutputColor, grow_name.c_str(), args.fOutputColor);
1376+
)__Cpp__"});
1377+
}
1378+
1379+
DEF_TEST(SkSLFPInlinedForBodyMustBeInAScope, r) {
1380+
// NOTE: this test exposes a bug with the inliner. The inlined function body is not wrapped in a
1381+
// scope, so the inlined code is emitted outside of the for loop.
1382+
test(r,
1383+
*SkSL::ShaderCapsFactory::Default(),
1384+
R"__SkSL__(
1385+
half4 adjust(half4 v) {
1386+
return v + half4(0.125);
1387+
}
1388+
void main() {
1389+
sk_OutColor = half4(0);
1390+
for (int x=0; x<4; ++x)
1391+
sk_OutColor = adjust(sk_OutColor);
1392+
}
1393+
)__SkSL__",
1394+
/*expectedH=*/{},
1395+
/*expectedCPP=*/{
1396+
R"__Cpp__(fragBuilder->codeAppendf(
1397+
R"SkSL(%s = half4(0.0);
1398+
for (int x = 0;x < 4; ++x) half4 _0_adjust;
1399+
{
1400+
_0_adjust = %s + half4(0.125);
1401+
}
1402+
1403+
%s = _0_adjust;
1404+
1405+
)SkSL"
1406+
, args.fOutputColor, args.fOutputColor, args.fOutputColor);
1407+
)__Cpp__"});
1408+
}
1409+
10901410
DEF_TEST(SkSLFPSwitchWithReturnInsideCannotBeInlined, r) {
10911411
test(r,
10921412
*SkSL::ShaderCapsFactory::Default(),

0 commit comments

Comments
 (0)