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

Commit 71624de

Browse files
johnstiles-googleSkia Commit-Bot
authored andcommitted
Allow constant propagation for negated constant-vectors and ints.
This CL improves on the previous fix for oss-fuzz:26789 by actually propagating the negation from the PrefixExpression inside the constructor, which unblocks further optimizations. Interestingly, this fix also exposes a further missing optimization--we optimize away comparisons of constant-vectors for floats, but fail to do the same for ints. Change-Id: I9d4cb92b10452a74db96ff264322cdc8a8f2a41f Bug: oss-fuzz:26830, oss-fuzz:26789, skia:10908 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332263 Commit-Queue: John Stiles <[email protected]> Reviewed-by: Brian Osman <[email protected]> Auto-Submit: John Stiles <[email protected]>
1 parent efc89d2 commit 71624de

File tree

8 files changed

+105
-22
lines changed

8 files changed

+105
-22
lines changed

gn/sksl.gni

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ skia_sksl_sources = [
8888
"$_src/sksl/ir/SkSLNop.h",
8989
"$_src/sksl/ir/SkSLNullLiteral.h",
9090
"$_src/sksl/ir/SkSLPostfixExpression.h",
91+
"$_src/sksl/ir/SkSLPrefixExpression.cpp",
9192
"$_src/sksl/ir/SkSLPrefixExpression.h",
9293
"$_src/sksl/ir/SkSLProgram.h",
9394
"$_src/sksl/ir/SkSLProgramElement.h",
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Copyright 2020 Google LLC
3+
*
4+
* Use of this source code is governed by a BSD-style license that can be
5+
* found in the LICENSE file.
6+
*/
7+
8+
#include "src/sksl/ir/SkSLPrefixExpression.h"
9+
10+
#include "src/sksl/ir/SkSLConstructor.h"
11+
12+
namespace SkSL {
13+
14+
std::unique_ptr<Expression> PrefixExpression::constantPropagate(const IRGenerator& irGenerator,
15+
const DefinitionMap& definitions) {
16+
if (this->isNegationOfCompileTimeConstant()) {
17+
if (this->operand()->is<FloatLiteral>()) {
18+
// Converts -floatLiteral(1) to floatLiteral(-1).
19+
return std::make_unique<FloatLiteral>(irGenerator.fContext, fOffset,
20+
-this->operand()->as<FloatLiteral>().value());
21+
}
22+
if (this->operand()->is<IntLiteral>()) {
23+
// Converts -intLiteral(1) to intLiteral(-1).
24+
return std::make_unique<IntLiteral>(irGenerator.fContext, fOffset,
25+
-this->operand()->as<IntLiteral>().value());
26+
}
27+
if (this->operand()->is<Constructor>()) {
28+
// We've found a negated constant vector, e.g.:
29+
// -float4(float3(floatLiteral(1)), floatLiteral(2))
30+
// To optimize this, the outer negation is removed and each argument is negated:
31+
// float4(-float3(floatLiteral(1)), -floatLiteral(2))
32+
// After another optimization pass:
33+
// float4(float3(-floatLiteral(1)), floatLiteral(-2))
34+
// Steady state is reached after another optimization pass:
35+
// float4(float3(floatLiteral(-1)), floatLiteral(-2))
36+
std::unique_ptr<Expression> result = this->operand()->clone();
37+
for (std::unique_ptr<Expression>& arg : result->as<Constructor>().arguments()) {
38+
arg = std::make_unique<PrefixExpression>(Token::Kind::TK_MINUS, std::move(arg));
39+
}
40+
return result;
41+
}
42+
}
43+
return nullptr;
44+
}
45+
46+
} // namespace SkSL

src/sksl/ir/SkSLPrefixExpression.h

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,15 @@ class PrefixExpression final : public Expression {
4040
return fOperand;
4141
}
4242

43-
bool isCompileTimeConstant() const override {
43+
bool isNegationOfCompileTimeConstant() const {
4444
return this->getOperator() == Token::Kind::TK_MINUS &&
4545
this->operand()->isCompileTimeConstant();
4646
}
4747

48+
bool isCompileTimeConstant() const override {
49+
return this->isNegationOfCompileTimeConstant();
50+
}
51+
4852
bool hasProperty(Property property) const override {
4953
if (property == Property::kSideEffects &&
5054
(this->getOperator() == Token::Kind::TK_PLUSPLUS ||
@@ -55,16 +59,7 @@ class PrefixExpression final : public Expression {
5559
}
5660

5761
std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
58-
const DefinitionMap& definitions) override {
59-
if (this->operand()->kind() == Expression::Kind::kFloatLiteral) {
60-
return std::unique_ptr<Expression>(new FloatLiteral(
61-
irGenerator.fContext,
62-
fOffset,
63-
-this->operand()->as<FloatLiteral>().value()));
64-
65-
}
66-
return nullptr;
67-
}
62+
const DefinitionMap& definitions) override;
6863

6964
SKSL_FLOAT getFVecComponent(int index) const override {
7065
SkASSERT(this->getOperator() == Token::Kind::TK_MINUS);
Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,24 @@
1+
void test_half() {
2+
half one = 1;
3+
half two = 2;
4+
5+
sk_FragColor.r = (half4(-1) == -half4(-half2(-1), half2(1))) ? 1 : 0;
6+
sk_FragColor.g = (half4(1) != -half4(1)) ? 1 : 0;
7+
sk_FragColor.b = (-half4(two) == half4(-two, half3(-two))) ? 1 : 0;
8+
sk_FragColor.a = (-half2(-one, one + one) == -half2(one - two, two)) ? 1 : 0;
9+
}
10+
11+
void test_int() {
12+
int one = 1;
13+
int two = 2;
14+
15+
sk_FragColor.r = (int4(-1) == -int4(-int2(-1), int2(1))) ? 1 : 0;
16+
sk_FragColor.g = (int4(1) != -int4(1)) ? 1 : 0;
17+
sk_FragColor.b = (-int4(two) == int4(-two, int3(-two))) ? 1 : 0;
18+
sk_FragColor.a = (-int2(-one, one + one) == -int2(one - two, two)) ? 1 : 0;
19+
}
20+
121
void main() {
2-
sk_FragColor.r = (half4(1) == half4(-half2(-1), half2(1))) ? 1 : 0;
3-
sk_FragColor.g = (half4(1) == -half4(1)) ? 1 : 0;
4-
sk_FragColor.b = (half4(0) == -half4(0)) ? 1 : 0;
22+
test_half();
23+
test_int();
524
}
Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11

22
out vec4 sk_FragColor;
33
void main() {
4-
sk_FragColor.x = 1.0;
5-
sk_FragColor.y = float(vec4(1.0) == -vec4(1.0) ? 1 : 0);
6-
sk_FragColor.z = float(vec4(0.0) == -vec4(0.0) ? 1 : 0);
4+
{
5+
sk_FragColor.x = 1.0;
6+
sk_FragColor.y = 1.0;
7+
sk_FragColor.z = 1.0;
8+
sk_FragColor.w = 1.0;
9+
}
10+
11+
{
12+
sk_FragColor.x = float(ivec4(-1) == ivec4(ivec2(-1), ivec2(-1)) ? 1 : 0);
13+
sk_FragColor.y = float(ivec4(1) != ivec4(-1) ? 1 : 0);
14+
sk_FragColor.z = float(ivec4(-2) == ivec4(-2, ivec3(-2)) ? 1 : 0);
15+
sk_FragColor.w = float(ivec2(1, -2) == ivec2(1, -2) ? 1 : 0);
16+
}
17+
718
}

tests/sksl/shared/golden/NegatedVectorLiteral.metal

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,19 @@ struct Outputs {
99
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
1010
Outputs _outputStruct;
1111
thread Outputs* _out = &_outputStruct;
12-
_out->sk_FragColor.x = 1.0;
13-
_out->sk_FragColor.y = float(all(float4(1.0) == -float4(1.0)) ? 1 : 0);
14-
_out->sk_FragColor.z = float(all(float4(0.0) == -float4(0.0)) ? 1 : 0);
12+
{
13+
_out->sk_FragColor.x = 1.0;
14+
_out->sk_FragColor.y = 1.0;
15+
_out->sk_FragColor.z = 1.0;
16+
_out->sk_FragColor.w = 1.0;
17+
}
18+
19+
{
20+
_out->sk_FragColor.x = float(all(int4(-1) == int4(int2(-1), int2(-1))) ? 1 : 0);
21+
_out->sk_FragColor.y = float(any(int4(1) != int4(-1)) ? 1 : 0);
22+
_out->sk_FragColor.z = float(all(int4(-2) == int4(-2, int3(-2))) ? 1 : 0);
23+
_out->sk_FragColor.w = float(all(int2(1, -2) == int2(1, -2)) ? 1 : 0);
24+
}
25+
1526
return *_out;
1627
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11

22
out vec4 sk_FragColor;
33
void main() {
4-
sk_FragColor.xy = -vec2(1.0);
4+
sk_FragColor.xy = vec2(-1.0);
55
}

tests/sksl/shared/golden/UnaryPositiveNegative.metal

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ struct Outputs {
99
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
1010
Outputs _outputStruct;
1111
thread Outputs* _out = &_outputStruct;
12-
_out->sk_FragColor.xy = -float2(1.0);
12+
_out->sk_FragColor.xy = float2(-1.0);
1313
return *_out;
1414
}

0 commit comments

Comments
 (0)