Most of the code generated by the fuzzer is nonsense, but there is a
method to its madness. The crash is only triggered under specific
conditions:
- The runtime effect has enough helper functions to mostly fill up the
call graph hash-map. It won't rehash until it gets close to capacity.
- There must be several calls to built-in functions, in order to add
elements to the call graph to force a rehash.
The fuzzer-generated code manages to satisfy both these requirements.
Change-Id: I9a1d7535557fedd4e9bfece3930ac86ede291ffe
Bug: oss-fuzz:36655
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437118
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GLSL allows an array of `lowp float` to be compared against `highp
float` seamlessly because the types are considered to be the same. SkSL,
however, treats these as different types, so we need to coerce the types
to allow this comparison to work.
In other words, these comparisons can cause an array to be implicitly
casted. The expression `myHalf2Array == float[2](a, b)` should be
allowed when narrowing conversions are enabled. To allow this to work,
we need a dedicated IR node representing this type coercion.
We now allow implicit coercion of array types when the array's component
types would be implicitly coercible, and have a new IR node representing
that implicit conversion.
This CL fixes array comparisons, but array assignment needs additional
fixes. It currently results in:
"type mismatch: '=' cannot operate on (types)".
Bug: skia:12248
Change-Id: I99062486c081f748f65be4b36a3a52e95b559812
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436571
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The optimization logic for swizzling a constructor assumed that every
argument to the constructor was a scalar or vector. When it was written,
this assumption was true. However, we recently added support for casting
mat2x2 to float4 which violates the assumption.
We now check every argument and do not attempt to optimize if a
non-scalar, non-vector arg is found.
Change-Id: Ia2b297bd62dfdf4af56712164fbc80c29c9611eb
Bug: oss-fuzz:36852
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437017
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
OSSFuzz discovered a minor variation of oss-fuzz:36770 which tickled a
different bug in SPIR-V RTFlip handling; we did not properly handle the
case where the InterfaceBlock is an array. SPIR-V does not support this
at all, but the IRGenerator allows it, and we don't detect it an an
error until later in the compilation process.
Change-Id: I80bd67a13dad878717dc122462132a2ed675532d
Bug: oss-fuzz:36850
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437018
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This CL does not update the DSLParser to honor these precision
qualifiers; that will be done in a followup.
Change-Id: Ib629bc99c0e6c7afb550a381d4e3b6ccc26aa64e
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436337
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
These parse into new modifier bits; the IR generator does not yet
support these bits. That's coming in a followup CL.
Change-Id: I362e9227694f9b862eaad100f6afca45a9b62a01
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436336
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We don't currently support this. There's no explicit syntax to cast an
array's type, but it can be implicitly required in some situations, like
`halfArray == floatArray` (when fAllowNarrowingConversions is on).
Change-Id: I00fe0ddd4f2682b2950e828dd78bb941d5f0430e
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436560
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This code intentionally mixes half4s and float4s everywhere. Before
http://review.skia.org/435916 landed, this resulted in a compile error.
Change-Id: I852fef6ee99a8b78623e0e9ddeee2ad84a8c0504
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436058
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SPIR-V code generation synthesizes some extra variables that don't
actually exist in the Program. Checking the ProgramUsage of these
variables would fail; ProgramUsage::get doesn't know about these
variables, so it asserts (and would consider them as dead even if it
didn't assert). We now track our SPIR-V bonus variables in a separate
set, and always report them as live.
Change-Id: If2f681470654025abf7ca4b3ec8126de2eb01297
Bug: oss-fuzz:36770
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435625
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I7b53df1eae83a596c4d1f3620e7f9bd146f68af2
Bug: oss-fuzz:36655
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/434465
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This relaxes our rules to allow calls to declared (but not yet defined)
functions. With that rule change, we have to specifically detect static
recursion and produce an error.
Bug: skia:12137
Change-Id: I39cc281fcd73fb30014bc7b43043552623727e03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431537
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12137
Change-Id: I609dd2578bf39a30e036ea85281886f8c4554579
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431038
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
At present, they aren't hooked up to anything. They will be made
functional in followup CLs.
Change-Id: I4bfc25eb4e19fce4c36ea0b55494bf37b2a9ee23
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430637
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is intended to make it easier for users porting GLSL code with
slicing constructors; the error message should make it clear that a
swizzle can be used here instead.
Change-Id: Ib03276ee910dfffd6146717dbd347b058f9940d1
Bug: skia:12193
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/427200
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
SkSL does not support shrinking a vector via casting. Use a swizzle
instead.
Change-Id: Ieba78a05dad9c55f44c765924e28f0c7e1667a67
Bug: skia:12193
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/427198
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Boolean vector expressions already allowed swizzles (see the
SwizzleBoolConstants.sksl test), but scalars had been inadvertently
disallowed.
Change-Id: I89e7139db50981f0ee1a9a5086b02603e57f967d
Bug: skia:12195
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/427196
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Boolean scalar-swizzling is currently not working.
Change-Id: Icd965e4b64a12311d098168f65622110d5fb3437
Bug: skia:12195
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/427038
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Id2676ffaba1dafc4a0485d99e1c5b87b990e0861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426976
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The tests now check bool4-mat2 conversions, which fortunately do work,
and the vector-to-matrix tests include int and bool conversions as well.
Change-Id: I971271838a93081b9258deb7c1d13b7732fb2440
Bug: skia:12067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426757
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Unfortunately, it's not easily testable since SkSLTest doesn't support
non-ES2 intrinsics.
Change-Id: I5b11e2af55fdc6fc29f48725aa977471c738b55e
Bug: skia:11222
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426538
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The fuzzer quickly discovered that the newly introduced mat2-to-vec4
conversion code did not account for integer vectors. We now handle
`ivec4(mat2)` casts properly. This required some non-trivial
restructuring of the logic, but in the vast majority of cases, the types
will match and the end result will be identical.
Change-Id: If07c2fe4b4345bd767384b1802374910f65cd3f0
Bug: oss-fuzz:35998
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426756
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GLSL disallows mixing swizzle domains within a single swizzle:
http://screen/93eHNQDbx35hMdk
SkSL now disallows it as well.
Change-Id: Ied2e11ee04285b143a864e28cac30335f01aad0e
Bug: skia:10621
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426458
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
GLSL supports casting vec4 into mat2 and vice versa, so SkSL should have
equivalent support. This CL allows the Compound constructor to take a
matrix as input, and fixes up backends to do the right thing when a
matrix shows up in the compound-constructor path.
Change-Id: I13289ad0a27ba59bddc3706093820594efebc693
Bug: skia:12067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426003
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Ic1a60da6359a5eb91da667340fdfdb5c7952d7c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426440
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11296
Change-Id: I7d41614957d6fa535faadebbeca890b54b6977ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425996
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This should generate the same output as before, except that SpvIds are
renumbered. We now use `writeComposite` instead of manually emitting
SpvOpCompositeConstruct instructions, and some logic for column-building
was simplified; columns counts are no longer tracked as a separate
value, since we can just call size() to inspect this.
Change-Id: If273341a0938eb5f7a6e2db12b080c7d0dae600a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426060
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit a89781215a.
Reason for revert: breakage on Windows
Original change's description:
> Add tests for matrix-vector conversions.
>
> GLSL supports casting vec4 into mat2 and vice versa, so SkSL should have
> equivalent support. Adding tests as a starting point.
>
> Change-Id: If8bcbf99afcec94d948d5da9e6205cb4a232af18
> Bug: skia:12067
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425837
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I2563041f538b1b20074385f1b61af5fc506ffad5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426057
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GLSL supports casting vec4 into mat2 and vice versa, so SkSL should have
equivalent support. Adding tests as a starting point.
Change-Id: If8bcbf99afcec94d948d5da9e6205cb4a232af18
Bug: skia:12067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425837
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Id2061ebe7873aa8b9480a2d8b0133c2fb79e79bb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/424098
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I4066aafc5b6137bfaf38100ff237fd9833023f34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/424097
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I7b033c891ee997f1ca022b73d6f78cc501c5cdc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/424302
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reland works around Adreno issue with this formulation of sk_Clockwise:
(sk_RTFlip.y < 0.0 ? !gl_FrontFacing : gl_FrontFacing)
and instead adds this to the top of the function:
bool sk_Clockwise = gl_FrontFacing;
if (sk_RTFlip.y < 0.0) {
sk_Clockwise = !sk_Clockwise;
}
Original description:
SkSL language features that are origin sensitive now use a uniform
to conditionally flip their result rather than generating different
code.
Previously we would insert a "rt height" uniform if sk_FragCoord needed
to be flipped. sk_FragCoord,y was implemented as "realFragCoord.y" or
"rtHeight - realFragCoord.y" depending on SkSL::ProgramSettings::fFlipY.
Now we instead use a two component vector rtFlip and sk_FragCoord.y is
always "rtFlip.x + rtFlip.y*realFragCoord.y". We configure rtFlip as
either (0, 1) or (rtHeight, -1). sk_Clockwise and dFdy simiarly use
rtFlip.y to emit code that always works with either origin.
Bug: skia:12037
Change-Id: I3a2ad6f5667eb4dcd823b939abd5698f89b58929
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425178
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit 943108b0b2.
Reason for revert: clockwise GM bad on android.
Original change's description:
> Don't key progams/pipelines on origin.
>
> SkSL language features that are origin sensitive now use a uniform
> to conditionally flip their result rather than generating different
> code.
>
> Previously we would insert a "rt height" uniform if sk_FragCoord needed
> to be flipped. sk_FragCoord,y was implemented as "realFragCoord.y" or
> "rtHeight - realFragCoord.y" depending on SkSL::ProgramSettings::fFlipY.
>
> Now we instead use a two component vector rtFlip and sk_FragCoord.y is
> always "rtFlip.x + rtFlip.y*realFragCoord.y". We configure rtFlip as
> either (0, 1) or (rtHeight, -1). sk_Clockwise and dFdy simiarly use
> rtFlip.y to emit code that always works with either origin.
>
> Bug: skia:12037
>
> Change-Id: I7a09d0caac60a58d72b76645ff31bcabde4086b6
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414796
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,ethannicholas@google.com
Change-Id: I91cc0d86be216f6c32e453a231de088c991be4b2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425056
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
SkSL language features that are origin sensitive now use a uniform
to conditionally flip their result rather than generating different
code.
Previously we would insert a "rt height" uniform if sk_FragCoord needed
to be flipped. sk_FragCoord,y was implemented as "realFragCoord.y" or
"rtHeight - realFragCoord.y" depending on SkSL::ProgramSettings::fFlipY.
Now we instead use a two component vector rtFlip and sk_FragCoord.y is
always "rtFlip.x + rtFlip.y*realFragCoord.y". We configure rtFlip as
either (0, 1) or (rtHeight, -1). sk_Clockwise and dFdy simiarly use
rtFlip.y to emit code that always works with either origin.
Bug: skia:12037
Change-Id: I7a09d0caac60a58d72b76645ff31bcabde4086b6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414796
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We've lived without this for a long time. It bloats sksl_gpu (inhibits
some inlining!), and if things go according to plan, we'll be removing
enum support from SkSL entirely soon.
Change-Id: If844bbe5fdae41df7930d5c8ea9b832f9dd1b922
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/419099
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I4280b5710dd8749ba766ba74d7a8886bc4e024bb
Bug: oss-fuzz:35124
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/417200
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, when converting a program, the pipeline stage could assume
that an input to main() of type half4/float4 was always the input color.
This was a good assumption since the only possible inputs were
coordinates, or the input color.
This CL now recognizes that when a second float4 is passed to the main()
function, it should be a SK_DEST_COLOR_BUILTIN. This will let blend
functions pass in two colors.
ProgramToSkVM now takes a dest-color argument as well, but existing call
sites won't reference it (since they aren't for blend functions). I've
just passed the input-color a second time, since the value will never
actually be accessed.
Change-Id: I4214586bda605c6d287aa25b1b099e6ef5ba15a4
Bug: skia:12080
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/417261
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Runtime Blend effects always take two input colors--source and
destination--instead of one. This CL adds a new ProgramKind for blend
effects, a new program module (empty for now), and adds a test to
confirm that the signature for blend functions is checked. Currently
these are only accessible via skslc; there's no Runtime Effect API to
create one and the dest color isn't hooked up to anything.
Change-Id: I5272a811d2d76b878cfdf3429efa78c9c8b3fd97
Bug: skia:12080
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/416798
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Add some comments explaining why different APIs flip the way they do.
Remove flip from Metal code generator and reverse the way flipping
occurs in Metal backend.
Change-Id: Ibec71e8d96c66d4b34b5ce6417056674090f979c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/413236
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
mix() has many overloads:
$genType mix($genType x, $genType y, $genType a);
$genType mix($genType x, $genType y, float a);
$genHType mix($genHType x, $genHType y, $genHType a);
$genHType mix($genHType x, $genHType y, half a);
$genType mix($genType x, $genType y, $genBType a);
$genHType mix($genHType x, $genHType y, $genBType a);
$genIType mix($genIType x, $genIType y, $genBType a);
$genBType mix($genBType x, $genBType y, $genBType a);
The top half were simple to implement via `evaluate_3_way_intrinsic`.
The bottom half--`x, y, $genBType`--required adding basic support into
`evaluate_n_way_intrinsic_of_type` for mixed argument types, since `x`
and `y` could be of any numeric type, but `a` is always boolean.
Fortunately, this didn't require major changes.
Change-Id: I015471f053c90d5a5c3ac67cc230d0f90950ff60
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414443
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
$mat matrixCompMult($mat x, $mat y);
$hmat matrixCompMult($hmat x, $hmat y);
This required some minor changes to `evaluate_n_way_intrinsic_of_type`
to allow the inputs to be matrices instead of vectors. Fortunately, most
of the moving parts were already generic/flexible enough that this just
worked, but some explicit checks for `x.isVector` needed to become
`!x.isScalar()`, and `columns` needed to become `slotCount`.
Change-Id: I1e22ecad37a7e187a7171e1f590a720f07cf9832
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414436
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This was handled properly, but lacked a test.
Change-Id: I84adc7cb3d37ab85eef945c1e38fc43c6cd8aa01
Bug: skia:10932
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414437
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Now that we know the minimum and maximum values of a given integer Type,
we can check for assigment statements or variable initial-values that
exceed those bounds and report it as an error. This check should work on
anything that can be optimized or folded down to an IntLiteral, but
isn't meant to be 100% exhaustive.
Change-Id: I4473b5b003e1b8e3385943ce60e303e95664e8ba
Bug: skia:10932
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/413437
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We need to be careful to distinguish cross(vec3, vec3)--a real built-in
intrinsic--from cross(vec2, vec2)--an inline function in sksl_gpu, but
not actually a legitimate GLSL intrinsic.
Change-Id: I7e78c99dadfcbb637ae55a2503acfb7e591c932e
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/413440
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The fuzzer found that it could overflow an int via a properly-crafted
constant-fold expression, leading to a UBSAN error. Constant-fold
expressions now guard against overflow (http://review.skia.org/413138)
and UBSAN is no longer triggered.
Change-Id: I07dba41e87bb9ceed37b84ec6b8922defbdc6550
Bug: oss-fuzz:32156, skia:12050
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/413836
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is one of the few compile-time constant functions with a variable
number of arguments, but it's otherwise pretty normal as intrinsics go.
$genType atan($genType y, $genType x);
$genHType atan($genHType y, $genHType x);
$genType atan($genType y_over_x);
$genHType atan($genHType y_over_x);
Change-Id: Ie852e10f37d73d53f69e806550872bc015f802d6
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/413439
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Expressions which would overflow/wrap the result type are now left
as-is.
Change-Id: I6a942f337e6e5761823f5c9dcd214fa58227a626
Bug: skia:10932, skia:12050
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/413138
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
$squareMat inverse($squareMat m);
$squareHMat inverse($squareHMat m);
Change-Id: I1a2b067dd276bb999107712c38d0124811b95e39
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412937
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Expressions which would optimize to a non-finite value are now left
as-is.
Integer math still has problems with overflow, but that can be addressed
separately.
Change-Id: I363a2c42684989062f606186f48246b7ac5b585c
Bug: skia:12050
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412956
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Ic6a3ce722c607d4d3c47d37e5749a01ae5fb193f
Bug: skia:12050
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412668
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
$genType refract($genType I, $genType N, float eta);
$genHType refract($genHType I, $genHType N, half eta);
The half form of refract was originally taking a `float eta` in our
headers, which seems wrong (and causes the DSL to break unless you add
casts). I've corrected the headers to use `half eta`.
Change-Id: I74b9ac330e0f7e99622d19cf7365aaa4cc910e57
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412664
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I1630cf6ecab6a1a18a7318c247f8263d87e2cda9
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412662
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
$genType reflect($genType I, $genType N);
$genHType reflect($genHType I, $genHType N);
Change-Id: I59889ad767829bf7e33838737d7b7f6d1cbc3ece
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412777
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This implementation leans heavily into DSL. I also reworked the
`normalize` intrinsic to use more DSL to shrink the implementation.
$genType faceforward($genType N, $genType I, $genType Nref);
$genHType faceforward($genHType N, $genHType I, $genHType Nref);
Change-Id: I73ab11d3fe449d2f2c0ae0d745fc39824fc64771
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412637
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This one is structured a bit differently; it gets to length() value,
then divides the input by its length using a bit of DSL. Since all the
inputs are constant, the constant-folder will do the right thing.
$genType normalize($genType x);
$genHType normalize($genHType x);
Change-Id: I51e5c65fa9e33738cbe253fcc97ee2160c48cfdd
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412340
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
float length($genType x);
half length($genHType x);
Change-Id: I65b64fdba5f7bd53afba1d6f930217e3f1bd6f6e
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/412377
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I3427fbaf57787c3051db95ec5882c9292d7985cf
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411312
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In the majority of cases, a uniform is an equally good substitute, and
replacing `sqrt(N)` with `unknownInput` actually makes the test clearer.
Change-Id: I7bcb477571972d7aa2ce8c49b3674471f7310748
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411306
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I2f245892e12416decd4f0aee491f16fb4040ffca
Bug: skia:11308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411036
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This was originally designated for 2x2 matrices only, but this was not
right--all matrix comparisons actually need to be rewritten to fully
work around the bug.
Change-Id: I743d16a65bc55e93361a3dd8753653384583f063
Bug: skia:11308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411416
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is allowed by GLSL, so we allow it too.
GLSL ES 1.0, Section 6.1: "The idiom “(void)” as a parameter list is
provided for convenience."
Change-Id: I551c505d3de518a75acd5e306f09f0f0767e43f2
Bug: skia:12025
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411300
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This will allow us to rewrite `mat == mat` on Adreno 5xx/6xx GPUs when
running in GLSL.
Change-Id: I621e918a545a49b7ecb9c944ae59b1e7a7594bae
Bug: skia:11308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410996
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is a reland of c6260f9742
Problematic DeadReturn.sksl test cases have been moved to DeadReturnES3.
Original change's description:
> Eliminate unreachable code during optimization.
>
> The Adreno 5xx and 6xx previously failed the SkSLStaticSwitchInline
> test; the driver struggled to interpret code with multiple return
> statements in a row. We now detect unreachable statements and eliminate
> them.
>
> Change-Id: I344d632f2488ca65b0635b37bebffe6e4fb607c5
> Bug: skia:12012
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410256
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12012
Change-Id: I748e8761cbc71c811b5ad8fe49186f980261d8b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410793
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
MetalCodeGen would incorrectly identify `(someMatrix, someScalar)` as a
math operation between `someMatrix` and `someScalar` and attempt to
convert `someScalar` to a matrix. If `someScalar` was a Boolean type,
this would lead to an assertion.
The binary expression is now checked more thoroughly before converting
the scalar into a matrix.
Change-Id: Id7e104d5533d8c43375927d4815b83e1a3c36be1
Bug: skia:11125
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410682
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit c6260f9742.
Reason for revert: Nexus 7 is grumpy, fails SkSLDeadReturn_GPU
https://ci.chromium.org/raw/build/logs.chromium.org/skia/539fff50ff093c11/+/annotations
Original change's description:
> Eliminate unreachable code during optimization.
>
> The Adreno 5xx and 6xx previously failed the SkSLStaticSwitchInline
> test; the driver struggled to interpret code with multiple return
> statements in a row. We now detect unreachable statements and eliminate
> them.
>
> Change-Id: I344d632f2488ca65b0635b37bebffe6e4fb607c5
> Bug: skia:12012
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410256
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ia3db1f1b28417e479e2d71a4a6ed94a007e47cf9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12012
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410780
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
The Adreno 5xx and 6xx previously failed the SkSLStaticSwitchInline
test; the driver struggled to interpret code with multiple return
statements in a row. We now detect unreachable statements and eliminate
them.
Change-Id: I344d632f2488ca65b0635b37bebffe6e4fb607c5
Bug: skia:12012
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410256
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
GLSL now emits 4x2 diagonal matrices as:
`(mat4x2(1.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0) * n)`
instead of
`mat4x2(n)`.
This works around a long-standing GLSL bug in both Mesa and glslang that
affects several drivers:
https://github.com/KhronosGroup/glslang/issues/2645
Change-Id: If529d5cd150ce720f436cb3634a2fd3423919278
Bug: skia:12003
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410137
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
- Arrays were unused
- Each ctype was used with exactly one mapper - remove the SkSL type
list
- After that, the mapper was just a key + value, so make it just a value
and use a map (rather than vector)
Change-Id: Ibc150ecc9ac9724ecd1e7056614ca5836ce99d5b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/409998
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This was only used in the context of sk_SampleMask, which was removed
recently.
Change-Id: Id70d7af8b3a100ff157c2984bad4131f1b92f317
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410056
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I6019418526def09c6c9f4b22567a2c76542d043c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/409876
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Although these tests are simple, it ended up uncovering a legitimate
SPIR-V bug (skia:12009).
Change-Id: Ie89235157256b97626aa6ada4d9d6ba62abc57fa
Bug: skia:12009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/409299
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
If we encounter code like `return 1; return 2;` we need to synthesize a
label, even though the second return statement isn't actually reachable.
This is harmless and satisfies the SPIR-V validator.
Ideally we'd eliminate the dead code entirely, but this case is rare and
isn't likely to cause any problems as-is.
Change-Id: I2d6219dff6868011353e19a662301bec44a015d6
Bug: skia:12009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/409402
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Now that the various Metal and SPIR-V bugs have been shaken out, we can
enable these tests. Knock on wood.
Change-Id: If4b4e302cfdd91464aaf00bc9639989de5e49aac
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408640
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
All the pieces of the puzzle were already here to support componentwise
addition and subtraction of matrices. Division was just a forgotten gap
in the implementation and is now patched up to match + and -.
NOTE: if you read the SPIR-V output very closely, you may be surprised
that there are fewer FDiv operations than you'd expect from reading the
input SkSL. As it turns out, a preexisting optimization is rewriting
`mat / 4` into `mat * 0.25` (see line 2689), and this rewritten form can
use the dedicated MatrixTimesScalar op. So we only get componentwise
FDivs for the `4 / mat` lines in the source.
Change-Id: I011c859f5b3a031fbb95a2956f1194a5f3b3794b
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408639
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Multiplication is handled just as before. Ops other than multiplication
are handled by splatting the scalar into a matrix, then performing the
op as a componentwise matrix-op-matrix binary expression.
Change-Id: I654715c45bf5c91b8e9660fdf1c1c6d6818b621a
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408637
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The mat4x2 portion of the test no longer checks its results, as there
are bugs in the Intel and Radeon GPU drivers that prevent mat4x2s
from being constructed properly.
The SkSL optimizer ends up eliminating the 4x2 matrix entirely because
it is unused by the rest of the code, as well at the 4x4 matrix which is
calculated from the 4x2. At this point, I'm OK with this.
Change-Id: If1464f9e4938b0a37b2ec180c686972389d94e83
Bug: skia:12003
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408900
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Previously, it was structured like Matrices.sksl and had no verification
of its results. Now it double-checks that its outputs match our
expectations. The test is still quite simple for now, however.
Change-Id: Iaa45fe58beb497a63801833f8ba5a493a61139d9
Bug: skia:11985, skia:12003
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408646
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 5d61cc2f87.
Reason for revert: break Vk bots
Original change's description:
> Vectorize scalars in SPIR-V using ConstructorSplat.
>
> This avoids redundant code, and has a small side benefit of
> deduplicating constant vectors which appear more than once in the code,
> since `writeConstructorSplat` already supports this.
>
> Change-Id: I2972ee922ac92adeb40bc765da3b490a59b957b3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408360
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I9fc0b896c0cfccc348d510a02df47d5ad74a0e90
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408644
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This test was generating invalid code in Metal
(http://review.skia.org/408356), but we didn't catch it because the code
wasn't actually being run.
Change-Id: I649034593a566f9e835b1cf7b0702c64952d31ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408641
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Support for constant-expression function calls in SkSL now exists, and
support for abs() was added at http://review.skia.org/405676.
Change-Id: I3144af993db93a3d640971734d4cb03e0cfb8589
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408642
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This avoids redundant code, and has a small side benefit of
deduplicating constant vectors which appear more than once in the code,
since `writeConstructorSplat` already supports this.
Change-Id: I2972ee922ac92adeb40bc765da3b490a59b957b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408360
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is a clone of the Metal test. (This can be moved into shared/ and
enabled as a real test once the codegen is fixed.)
At present, this test generates broken code; everything is writing an
SpvOpMatrixTimesScalar opcode regardless of the actual operation being
performed.
Change-Id: If06b4196e7d9be36e41c5c60c006b2a713cc25d8
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408297
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We were emitting this at global scope (not in Globals). That would lead
to errors about the variable needing to be in the constant address
space. (You can see the result in ConstArray.metal - the old code was
invalid). Also, we were already making references use _globals, so the
code was double-wrong (or half-right, depending on your perspective).
After the core change, writeVarDeclaration was only used for local
scope, and writeModifiers never used the 'globalContext' parameter.
The removal of finishLine() changed every test output, unfortunately.
Change-Id: Icc1356ba2cc3c339b2f5759b3d18523fd39395bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408356
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This CL adds a polyfill for componentwise matrix/matrix division to
Metal, as well as matrix/=matrix. Matrix/scalar and scalar/matrix
division work by splatting the scalar out to a matrix (handled in the
prior CL, http://review.skia.org/407616) and then performing
componentwise matrix/matrix division.
Working demonstration (copy-pasted from the Metal output file):
http://screen/BrqyPcbPrB7Dy4m
Change-Id: I6a8b97783be3485f7ffee551b669d14bc58e7568
Bug: skia:11125
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407796
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 90508f02dc.
Reason for revert: avoiding driver bugs this time
Original change's description:
> Revert "The Matrices test now verifies its results."
>
> This reverts commit 86121f6c0e.
>
> Reason for revert: tree sad
>
> Original change's description:
> > The Matrices test now verifies its results.
> >
> > Previously, this test did a bunch of matrix math but never actually
> > checked its results for correctness.
> >
> > Change-Id: I353be58049286266c2d561b0939b3874d2684403
> > Bug: skia:11985
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407360
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I1335f01c14ee955426e02efaa3c30421cd41aa34
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11985
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407617
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11985
Change-Id: I214375d74977f324973da72c440d7ff5ff179016
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408157
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The Metal code generator will now detect matrix-op-scalar expressions
and splat the scalar across a matrix. This allows a scalar to be added
to, or subtracted from, a matrix. (It does not fix division because
Metal also does not natively support componentwise division on
matrices.)
Change-Id: I7d5b0c5bd35393475c524e34cad789bf4f72a103
Bug: skia:11125
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407616
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 86121f6c0e.
Reason for revert: tree sad
Original change's description:
> The Matrices test now verifies its results.
>
> Previously, this test did a bunch of matrix math but never actually
> checked its results for correctness.
>
> Change-Id: I353be58049286266c2d561b0939b3874d2684403
> Bug: skia:11985
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407360
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I1335f01c14ee955426e02efaa3c30421cd41aa34
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407617
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, this test did a bunch of matrix math but never actually
checked its results for correctness.
Change-Id: I353be58049286266c2d561b0939b3874d2684403
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407360
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Aside from sqrt() and normalize(), we now optimize all the intrinsics
which take a single argument as input, and return that argument with
each of its components permuted as output.
This CL also introduces a minor restriction--we no longer optimize
intrinsics which evaluate to inf or nan, such as `inversesqrt(-1)`.
These will be left in the source as-is.
Change-Id: I4919b3c18a2df81accd6daf2f650b9f587ff43fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406577
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is similar to the intrinsic optimization for any() and all(). Tests
for all three intrinsics have been bulked up a bit as well.
Change-Id: I262b9448e543b4709d1e7c8585f74a206c4b5abd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406576
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This layout qualifier is not actually used anywhere.
Change-Id: I817c9affdd00e492c70f251eb52680644b7ff3f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406141
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
UInt support was added to DSL in http://review.skia.org/403601. We use
the UInt type in the DitherEffect fragment processor.
Change-Id: I2770eb0196177ee403b461134c9895d2e0b2e6db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406139
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In particular, this optimizes abs() and sign() when all inputs are known
at compile time. This resolves a TODO on a test case in
`IllegalIndexing.rts`.
Change-Id: Ica310522a85b42dc7ae255bd25004a6629d04176
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405676
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
A cast like `float(five)` or `int4(colorGreen)` will now detect const
variables and replace the expression with its compile-time constant
equivalent value. This can unblock further constant folding
opportunities.
(This CL is very similar in spirit to http://review.skia.org/404676)
Change-Id: If78a2091770777b0caaaec696fe15a0f55d88c24
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405683
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
A cast like `float(five)` or `int4(colorGreen)` should detect const
variables and replace the expression with its compile-time constant
equivalent value. At present, this replacement is missed, which inhibits
further optimization opportunities on the expression.
(This CL is very similar in spirit to http://review.skia.org/404676)
Change-Id: I04b5c435a30d2afcdbdb3d020adc15e9c651cc31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405682
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL only handles a subset of our intrinsics. In particular, it
avoids changing the behavior of `sqrt` as many of our tests use sqrt as
an optimization barrier.
The transcendental test inputs are intentionally kept very simple to
avoid putting numbers in the test outputs which could round differently
on various platforms and cause Housekeeper to complain.
Change-Id: I539f918294332310dcd6fe12fab163c0b6216f65
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405398
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Previously, the code neglected to resolve constant variables into
values. This meant that expressions like `lessThan(zero, one)` could not
be compile-time evaluated even when `zero` and `one` have known values.
Change-Id: I2f5ce303e3dcc682be14e4d2485e24dd7c59212e
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405536
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If every argument passed to lessThan/greaterThan/lessThanEqual/
greaterThanEqual/equal/notEqual() is a compile-time constant, we now
detect this and optimize away the function call entirely.
Change-Id: I3415d21be6ef51b38b682a792bd118fad51957f5
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404776
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If every argument passed to any() or all() is a compile-time constant,
we now detect this and optimize away the function call entirely.
Future CLs will perform a similar optimization on other intrinsic calls
which can be detected and eliminated at compile time, like lessThan().
Change-Id: Ie55aff538b1ccaf2b3bcf9a69573a85f081b7ade
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404417
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
A constructor like `float3(five)` is not a compile-time constant, so we
miss optimization opportunities like folding. Constant variables inside
splat constructors are now replaced when optimization is on, so this
would optimize down to `float3(5.0)` and be eligible for folding.
Change-Id: I4bf6f52a48ef733e6b24791d02687081194ef488
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404676
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
A constructor like `float2(one, two)` is not a compile-time constant, so
we miss optimization opportunities like folding. Constant variables
inside compound constructors are now replaced when optimization is on,
so this would optimize down to `float2(1.0, 2.0)` and be eligible for
folding.
Change-Id: I80dd421f61d4eed21278805e2dc26d198a678e52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404657
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I06631e7f0db518f4de19a39bf1ed368afbd5d409
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/403076
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This adds an explicit DeclareGlobal call, which must now be called for
variables that were previously implicitly global.
Change-Id: Iaf838880d1033ee52aac9246e31e3bda9a3b36f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402399
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is a reland of 7b253d34eehttp://review.skia.org/402781 must land first, to resolve the assertions
we were getting on the Android bots.
Original change's description:
> Optimize away swizzles of constant variables.
>
> Change-Id: I49807f18ea54e85c2b8f1419278c54aa2d6f8fac
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402581
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Ib4f494b16a89ff744d4384db95a8a86d9653c190
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402644
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is a reland of 830c69ca66
Original change's description:
> Implement operator== and != for Metal structs and arrays.
>
> GLSL/SkSL assumes that == and != on struct/array types should work.
> We need to emit equality and inequality operators whenever we find code
> that compares a struct or array.
>
> Structs and arrays can be arbitrarily nested, and either type can
> contain a matrix. All of these things need custom equality operators in
> Metal. Therefore, we need to recursively generate comparison operators
> when any of these types are encountered.
>
> For arrays we get lucky, and we can cover all possible array types and
> sizes with a single templated operator== method. Structs and matrices
> have no such luck, and are generated separately on a per-type basis.
>
> For each of these types, operator== is implemented as an equality check
> on each field, and operator!= is implemented in terms of operator==.
> Equality and inequality are always emitted together. (Previously, matrix
> equality and inequality were emitted and implemented independently, but
> this is no longer the case.)
>
> Change-Id: I69ee01c0a390d7db6bcb2253ed6336ab20cc4d1d
> Bug: skia:11908, skia:11924
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402016
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11908, skia:11924, skia:11929
Change-Id: I6336b6125e9774c1ca73e3d497e3466f11f6f25f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402559
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
All internal usage has migrated to MakeFor..., this removes the old
program kind, and updates some tests.
Bug: skia:11813
Change-Id: I56733b071270e1ae3fab5d851e23acf6c02e3361
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402536
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
At present, this is a missed optimization opportunity. These will be
optimized in a followup CL.
Change-Id: I8882058900cdc12c8ab0df03e36ebfb9d8022f01
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402580
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 830c69ca66.
Reason for revert: Pixel5 issues on tree
Original change's description:
> Implement operator== and != for Metal structs and arrays.
>
> GLSL/SkSL assumes that == and != on struct/array types should work.
> We need to emit equality and inequality operators whenever we find code
> that compares a struct or array.
>
> Structs and arrays can be arbitrarily nested, and either type can
> contain a matrix. All of these things need custom equality operators in
> Metal. Therefore, we need to recursively generate comparison operators
> when any of these types are encountered.
>
> For arrays we get lucky, and we can cover all possible array types and
> sizes with a single templated operator== method. Structs and matrices
> have no such luck, and are generated separately on a per-type basis.
>
> For each of these types, operator== is implemented as an equality check
> on each field, and operator!= is implemented in terms of operator==.
> Equality and inequality are always emitted together. (Previously, matrix
> equality and inequality were emitted and implemented independently, but
> this is no longer the case.)
>
> Change-Id: I69ee01c0a390d7db6bcb2253ed6336ab20cc4d1d
> Bug: skia:11908, skia:11924
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402016
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I06c47923649ea9fb675bab6baab121eb504d5ab8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11908
Bug: skia:11924
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402558
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GLSL/SkSL assumes that == and != on struct/array types should work.
We need to emit equality and inequality operators whenever we find code
that compares a struct or array.
Structs and arrays can be arbitrarily nested, and either type can
contain a matrix. All of these things need custom equality operators in
Metal. Therefore, we need to recursively generate comparison operators
when any of these types are encountered.
For arrays we get lucky, and we can cover all possible array types and
sizes with a single templated operator== method. Structs and matrices
have no such luck, and are generated separately on a per-type basis.
For each of these types, operator== is implemented as an equality check
on each field, and operator!= is implemented in terms of operator==.
Equality and inequality are always emitted together. (Previously, matrix
equality and inequality were emitted and implemented independently, but
this is no longer the case.)
Change-Id: I69ee01c0a390d7db6bcb2253ed6336ab20cc4d1d
Bug: skia:11908, skia:11924
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402016
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Simplifies SampleUsage quite a bit (no need to track multiple kinds of
sampling, variable matrices don't exist any more, etc...).
Change-Id: I58b8de7218d00c4d882d2650672e5fe01892a062
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402177
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
There aren't many cases where array-of-struct types are useful in
ES2, but it looks like function parameters are one such case:
http://screen/7Fnc7GhewAkUK3j
Change-Id: I23410a3824a3c202c12147d6939586cc0e55a9ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402397
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL adds a RuntimeEffect option flag which skips over the various
`strictES2Mode` checks sprinkled throughout IR generation.
Runtime Effects still won't allow a lot of ES3 things (the Pipeline
stage will reject unsupported statement types, SkVM doesn't support most
non-ES2 constructs, etc). However, this change will give us the ability
to test many more features involving arrays and structs that previously
were off-limits due to ES2 restrictions, and will shore up some
legitimate gaps in our testing. This is a useful starting point to allow
for improved test coverage.
Change-Id: I4a5bc43914e65fc7e59f1cecb76a0ec5a7f05f2f
Bug: skia:11209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402157
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is completely unused - GrMatrixEffect is the only thing that deals
with matrix transforms on child sampling. Removing this makes everything
simpler to reason about.
Change-Id: I555a3fd937c064f2480b149a6d4d8e36f7ee69bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402176
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Global variables which can be calculated in C++ code are now written as
constant values in the DSL, instead of performing the same logic
redundantly in the shader.
In some cases this can be fairly significant, e.g. RectBlurEffect has
a global with the expression
abs(rect.x) > 16000 || abs(rect.y) > 16000 ||
abs(rect.z) > 16000 || abs(rect.w) > 16000
Change-Id: I84221f60a4986b3225afcf91ef95cdcfc941b4b7
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401437
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This undoes the change from http://review.skia.org/400541 and reworks
sk_Caps to use macros, exactly like the existing CPPCodeGenerator.
This compromise is necessary for when-expressions. Specifically, when-
expressions must be valid when parsed *either* as SkSL or as C++. We
want to support when-expressions that reference sk_Caps, like:
layout(when=!sk_Caps.floatIs32Bits)
The macro approach allows this when-expression to work.
Change-Id: I346762fb14c0b2f0c10015497f902f037e3461a9
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401157
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
When emitting a built-in function call, we need to honor the `fCPPMode`
state. Previously we were rewriting the call into a DSL call even in C++
mode.
We also now spell out Fract's complete namespace, to avoid a name
collision with MacTypes.h:
https://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/MacTypes.h.auto.html
typedef SInt32 Fract;
Change-Id: I752b7816a64a9b2b2c79d92fe46cd774e1bab96a
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401678
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is a reland of 22dcb5fd7e
Original change's description:
> Add coords parameter to all .sksl test files used as runtime effects
>
> Convert to use the newer MakeForShader factory, which requires this.
>
> Change-Id: Ifaf6054054027c78f3f3fe15596e435e0f79b877
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399336
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:11919
Change-Id: I5f745c54b2bc3712f2281db6e067345903e81931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401836
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 22dcb5fd7e.
Reason for revert: Lot's of red Android and Win bots.
Original change's description:
> Add coords parameter to all .sksl test files used as runtime effects
>
> Convert to use the newer MakeForShader factory, which requires this.
>
> Change-Id: Ifaf6054054027c78f3f3fe15596e435e0f79b877
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399336
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I0fa844c6cf985d16e72c7f26aa217752612dcfc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401077
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Convert to use the newer MakeForShader factory, which requires this.
Change-Id: Ifaf6054054027c78f3f3fe15596e435e0f79b877
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399336
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The comma-warning issue has since been fixed, and we don't need to
use `(void) var;` when we have C++17's [[maybe_unused]] attribute.
Change-Id: I2078354f06801b024638e9c7d9ac699df20a8c48
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401116
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I00cc1e89fd85fdc0ce0860fcb35ececd0eaec50a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400540
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This required some changes to how we name variables in DSL.
When-expressions are designed to expect a local C++ variable with the
same name as the layout key. This constraint means our DSLVar variables
CANNOT have the same name as the layout key. Now, all DSL variables are
given a prefix. We try to keep the code tidy by using just a leading
underscore as the prefix, where it's safe to do so. (The C++ naming
rules put some underscore-names out of bounds, but underscore followed
by a lowercase letter is safe.)
Change-Id: Iaa8878042329b9909096f05712d5cf636ea01822
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400623
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This prevents some ambiguous-expression errors (since we were passing a
`double` typed value into places that only took an `int` or `float`).
(The ambiguous-expression errors were later fixed in
http://review.skia.org/400619, but it's still the right thing to do for
DSL C++ to emit floats.)
Change-Id: I052c9919a9f00cb427dd152722d2f7c370f3f3b4
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400616
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This was almost right, but was missing the trailing () to make a
function call.
Change-Id: I1215a97bb0ac39aceca8ff6bea70af8ff572ef84
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400541
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL also removes some vestiges of the kSampler type, which hasn't
been used in .fp files for a long time.
Change-Id: Iaca1d0c6e77ad2df2b6c5dacd1c68079d6dd5cf2
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398738
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is a reland of c412688798
This CL lands the code changes but not the dm test, which is causing
link errors. Tests will be relanded as a separate CL, at
http://review.skia.org/400097
Original change's description:
> Reland "Implement statements and expressions in DSL C++ code generator."
>
> This is a reland of 16cbfb41df
>
> Tests now rely on `shaderDerivativeSupport` and `integerSupport` as
> proxies to indicate ES3 support. The SwitchStatement test has been
> adjusted to hopefully confuse fewer compilers.
>
> Original change's description:
> > Implement statements and expressions in DSL C++ code generator.
> >
> > This CL removes the bulk of the existing C++ code generator, especially
> > all the complex format-string assembly code. It has been replaced with
> > actual DSL code generation. Simple IR can now be successfully translated
> > to a working DSL fragment processor.
> >
> > This CL also adds a simple test harness which is patterned after the
> > existing SkSLTest; it renders a pixel, reads it back, and fails the test
> > if the result isn't solid green (RGBA=0101).
> >
> > This CL doesn't implement every feature. Some obvious gaps include:
> > - Sampling from children
> > - Uniforms/inputs of any kind
> > - Function calls of any kind
> >
> > Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> > Bug: skia:11854
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:11854, skia:11891
> Change-Id: I91363e31f34611d15ae350b52d6fc459feeace9c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399076
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11854
Bug: skia:11891
Change-Id: Ib1f08256c84d1da2130e0b61356f72435dc0a5a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399740
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit c412688798.
Reason for revert: fix Google3 roll and wasm build
Original change's description:
> Reland "Implement statements and expressions in DSL C++ code generator."
>
> This is a reland of 16cbfb41df
>
> Tests now rely on `shaderDerivativeSupport` and `integerSupport` as
> proxies to indicate ES3 support. The SwitchStatement test has been
> adjusted to hopefully confuse fewer compilers.
>
> Original change's description:
> > Implement statements and expressions in DSL C++ code generator.
> >
> > This CL removes the bulk of the existing C++ code generator, especially
> > all the complex format-string assembly code. It has been replaced with
> > actual DSL code generation. Simple IR can now be successfully translated
> > to a working DSL fragment processor.
> >
> > This CL also adds a simple test harness which is patterned after the
> > existing SkSLTest; it renders a pixel, reads it back, and fails the test
> > if the result isn't solid green (RGBA=0101).
> >
> > This CL doesn't implement every feature. Some obvious gaps include:
> > - Sampling from children
> > - Uniforms/inputs of any kind
> > - Function calls of any kind
> >
> > Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> > Bug: skia:11854
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:11854, skia:11891
> Change-Id: I91363e31f34611d15ae350b52d6fc459feeace9c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399076
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I71a8cf31e8a013b7a2a0d10f0ad3bc3893ea07ea
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Bug: skia:11891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399499
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit f33b061e3b.
Reason for revert: Google3 roll and wasm build
Original change's description:
> Add support for uniforms and layout(key)s to DSLCPPCodeGenerator.
>
> Change-Id: I77c386e3d72fb4a5986e5efb8bc9d409200534d1
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398457
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I006ece639fa6051ff6ef1c496e648db9d5d0b30a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399498
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I77c386e3d72fb4a5986e5efb8bc9d409200534d1
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398457
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of 16cbfb41df
Tests now rely on `shaderDerivativeSupport` and `integerSupport` as
proxies to indicate ES3 support. The SwitchStatement test has been
adjusted to hopefully confuse fewer compilers.
Original change's description:
> Implement statements and expressions in DSL C++ code generator.
>
> This CL removes the bulk of the existing C++ code generator, especially
> all the complex format-string assembly code. It has been replaced with
> actual DSL code generation. Simple IR can now be successfully translated
> to a working DSL fragment processor.
>
> This CL also adds a simple test harness which is patterned after the
> existing SkSLTest; it renders a pixel, reads it back, and fails the test
> if the result isn't solid green (RGBA=0101).
>
> This CL doesn't implement every feature. Some obvious gaps include:
> - Sampling from children
> - Uniforms/inputs of any kind
> - Function calls of any kind
>
> Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11854, skia:11891
Change-Id: I91363e31f34611d15ae350b52d6fc459feeace9c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399076
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 16cbfb41df.
Reason for revert: using ES3 features, breaks on ANGLE ES2 bots
Original change's description:
> Implement statements and expressions in DSL C++ code generator.
>
> This CL removes the bulk of the existing C++ code generator, especially
> all the complex format-string assembly code. It has been replaced with
> actual DSL code generation. Simple IR can now be successfully translated
> to a working DSL fragment processor.
>
> This CL also adds a simple test harness which is patterned after the
> existing SkSLTest; it renders a pixel, reads it back, and fails the test
> if the result isn't solid green (RGBA=0101).
>
> This CL doesn't implement every feature. Some obvious gaps include:
> - Sampling from children
> - Uniforms/inputs of any kind
> - Function calls of any kind
>
> Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I4f3e7667bf1e3a5539d0248b6c47d9ae2296aa88
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398739
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL removes the bulk of the existing C++ code generator, especially
all the complex format-string assembly code. It has been replaced with
actual DSL code generation. Simple IR can now be successfully translated
to a working DSL fragment processor.
This CL also adds a simple test harness which is patterned after the
existing SkSLTest; it renders a pixel, reads it back, and fails the test
if the result isn't solid green (RGBA=0101).
This CL doesn't implement every feature. Some obvious gaps include:
- Sampling from children
- Uniforms/inputs of any kind
- Function calls of any kind
Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Most backends don't like init-stmts with multiple VarDeclarations inside
them, so we no longer emit IR that does this. This lets us avoid doing
backend-level fixups.
Change-Id: Ide839de18953a73e0f9c7a690df59a7bc3523f89
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398221
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is another strange, experimental feature that clutters the
implementation and isn't used by anyone (to my knowledge).
Change-Id: I538b7eca0cd28aab32f4739b23459731ade9105e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398226
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This was an experimental feature. It worked (but only the GPU backend).
It was never adopted or used by anyone, to my knowledge. It's a large
amount of code, and a strange corner of SkSL for users to stumble into.
Bug: skia:10680
Change-Id: I0dda0364bce7dbffa58c32de4c7801ec2a6bc42e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398222
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Unused InterfaceBlocks were not added to the ProgramUsage map. The
ProgramUsageVisitor now makes sure to account for them during its
initial scan.
Change-Id: If3afac8e954c5b685ddc6b63b0f771d8c0b8f207
Bug: oss-fuzz:33405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398016
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SPIR-V would previously jump to the top of the do-while loop when
encountering a `continue`. This is not correct; it should jump to the
loop-exit test. We don't have any real shaders which test this case, and
it can't be checked from inside Runtime Effects since do-while is not
part of ES2.
Change-Id: Iac58e9e7ad0a94640e6418afea096b19ba50a2df
Bug: skia:11874
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397576
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11867
Change-Id: I21e71c77518f0a942651f31061f43f80505090ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397150
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Cleans up registerChild calls, which were all on one long line.
Change-Id: I2b665b033edb88cf1de5e4da433833c0be26c92b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397147
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
These enforce stricter rules about the signature of main, and each one
uses a separate pre-include module. That prevents color filters from
being able to reference sk_FragCoord (or coords passed to main) at all.
It also limits the versions of sample() that are exposed.
In the new world, an effect created for a specific stage of the Skia
pipeline can only be used to create instances of that stage (SkShader or
SkColorFilter). For now, SkRuntimeEffect::Make uses kRuntimeEffect,
which continues to be more lenient and allow creation of either shaders
or color filters from a single effect. After we migrate all clients, we
can deprecate and then delete that mode.
Bug: skia:11813
Change-Id: I0afd79a72beeec84da42c86146e8fcd8d0e4c09f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395716
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
When a Block is detected in the for loop's init-statement, we now
synthesize a scope and write the init-statement directly above the for
loop and leave the for loop's init-stmt section empty.
Change-Id: I3e89c6a4328a79e49b8a19faae7975629cd0e152
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396820
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This is structured differently than the GLSL fix, due to the different
semantics of array-types in Metal.
Change-Id: I27ad11539bbbb96abb0686d5686b8fcd2f5dd6d1
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396916
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We lacked test coverage for this case, and this was broken when compound
VarDeclarations were split from one Statement into several.
Change-Id: I561b4d8acc0bfa01161d873a0c022ec58e316903
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396817
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This requires the GN processing to be slightly more complex, as GN has
no native support for more than one extension on a file.
Context: https://groups.google.com/a/chromium.org/g/gn-dev/c/RdEpjeYtb-4
Change-Id: I630511fca9eb291f0e414481ef439f18a8e1b72f
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396197
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The long term goal is for the DSLCPPCodeGenerator to replace the
CPPCodeGenerator entirely, but we will need both to coexist while DSL is
still under development.
Currently, the DSLCPPCodeGenerator is cloned from CPPCodeGenerator and
emits almost exactly the same code (it adds a comment at the top to
distinguish its output). Its output will change in followup CLs.
This CL also allows skslc to recognize the `_dsl.cpp` output suffix and
generate code using DSLCPPCodeGenerator instead of CPPCodeGenerator.
This allows test DSL FPs to be created for inspection.
Change-Id: If5136279c307ea53a9df3a292caa18344c1eb259
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396096
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Also add unit test of all GLSL type aliases.
Bug: skia:10679
Change-Id: I93e21621c11adfe3f114d0c55fb8043518e62696
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395718
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
In http://review.skia.org/393397, I replaced a Constructor::Convert call
with a call directly to ConstructorCompoundCast::Make.
This worked fine if the input expression was actually a compound, but
if it was not, the code would assert/crash. The fuzzer detected this
error right away. (Enums are not considered to be a scalar, a vector or
a matrix in SkSL.)
Change-Id: Ie0df4c5771ff4f4d8f5251d4703e9c3516b6baad
Bug: oss-fuzz:33113
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395720
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Two of the tests are now (correctly) detected as invalid code, but were
previously checked into /shared/ because they compiled when the fuzzer
first reported them (that is, after the crash was fixed). These have
been moved into the /errors/ test folder.
One of the tests was only generating an error because its main function
was named `a` instead of `main`, so I renamed it to `main`.
Change-Id: I1a2346fb16e304b0c66ff377a3f9bf7e7ee89ba9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394899
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
These builtins have been obsolete for a long time; they were only
implemented on the GLSL backend. These values are provided in
u_skRTHeight and u_skRTWidth instead. (See GrGLSLProgramBuilder's
addRTHeightUniform and addRTWidthUniform.)
Change-Id: I8cceca348cbf9071939618f913693c316d35dbc6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395001
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The code which assembled a uniform-scale matrix (aka
`ConstructorDiagonalMatrix`) accidentally looped over columns instead of
rows when assembling the matrix.
Change-Id: Iefb9ae645a7a85f61443e6e96b752ba729f40c9c
Bug: skia:11278
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394997
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This test fell into disrepair when constant propagation was removed; it
assumed that @if and @switch would work on all unchanging variables, not
just those with the `const` modifier.
Change-Id: Ie9c1816f9c0852fdea998e4e156047e4cca9ad5b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395000
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
GLSL (post-ES2) allows array comparison: http://screen/8gryPvb9T7gndyb
Unfortunately, because ES2 does not support array comparisons, we can't
add this test to the dm test suite.
Change-Id: I06b71683e49b2631669cff801dc647951a81a299
Bug: skia:11849
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394162
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Icc710e414388e4026a5e9819a53b8dac8ee0a2d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394896
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is allowed in OpenGL ES2, and its absence in SkSL has been a pain
point for new users adopting Runtime Effects.
Change-Id: Id2ed78261a2cd2b14b49ad22cb74cdc9e0905f8a
Bug: skia:11368
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393418
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
OpenGL ES2 allows structs to be compared: http://screen/6KnX4ZfkdLtqDWv
This already worked in GLSL, Metal and SkVM.
Change-Id: Iaf7029c0c1ea9d447348c8280a2788f0d36befad
Bug: skia:11846
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393598
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11374
Change-Id: I63d605eabbe514a0469d00d8a671969874f3edd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393081
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Also adds tests of non-uniform shader declarations. These are currently
allowed, but will be detected as an error in the next CL.
Bug: skia:11374
Change-Id: I3fee0a0c97ae590f7bc6952cb367f7e94436b891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393080
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This shook out a long-standing bug; constant folding would treat a
matrix resize as if the cells not covered by the original matrix were
all zero. This is wrong; GLSL populates the unknown cells with an
identity matrix. We actually tested for the wrong behavior, so the tests
were updated to match the correct behavior, and an equivalent test was
added that does not constant-fold (to verify that our constant folder
matches reality).
Change-Id: I03df10ce646fbef0a36e9c1a841a7637182de122
Bug: skia:11032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392916
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
When growing a matrix, the new cells should contain the identity matrix.
`writeMatrixCopy` was populating every cell with zeros instead.
Also, `writeMatrixCopy` had some code for handling int matrices, but
we don't support this in SkSL anywhere else; replaced with an assert
that the component type is float.
Change-Id: Icb80ae711d5c4f1417b62168b203a661f52e8d7d
Bug: skia:11835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393037
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The actual fix happened at prior CL http://review.skia.org/392197, which
reworked how vector-cast constructors function.
Change-Id: Ifb71ec913b349e65d38458dc615441e7a73efddc
Bug: oss-fuzz:32851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392841
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Making a VectorCast from a compile-time constant will perform the cast
at compile-time instead; previously, we did not apply this optimization.
This simplified a few test outputs in subtle ways. (In particular, the
SPIR-V codegen used to occasionally decorate OpConstantComposite of
constant numbers with RelaxedPrecision, and no longer appears to do
this. This should have no effect on results either way AFAICS.)
Because we don't return VectorCast constructors containing compile-time
constant values, we do not need to implement compareConstant for this
constructor; they only wrap non-compile-time-constant expressions.
Change-Id: I28c1f337f64d6f20fb86bc0f58e225af4bd7b26c
Bug: skia:11032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392197
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
No functional change yet, but fragmentProcessor is no longer allowed in
runtime effect SkSL (and errors about 'shader' variables will no longer
refer to 'fragmentProcessor').
Bug: skia:11813
Change-Id: I5161f9adbe79090ec31ed3faf7dcef0e290842b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392440
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Allowed today, will soon be an error.
Bug: skia:11813
Change-Id: I5c13de7657fa85f13fa6d80e1d890225d8a3e868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392439
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This isn't something I expect to occur often. However, we have always
reported that array-ctors with constant arguments are compile-time
constants, and `compareConstant` is supposed to be implemented for all
compile-time constants.
Change-Id: I0bbfe2a3f78c29c14f69c3b6faca71067a0e45c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391116
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We don't directly support this today at all. In practice, though, simple
constant arrays are detected as equal in the constant-folding pass
because they hit the `x == x` self-equality check (using
`IsSameExpressionTree`).
This does not work for our inequality tests, though, so those do not
fold.
Change-Id: I6730a9a2d1da9ac613ee58889d651f3ff65b1d2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391057
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
As soon as a single VarDeclaration is successfully created, its Variable
is added to the current symbol table. However, if a variable-declaration
line declared several variables in a row, we would stop if ANY of the
declarations contained an error and discard the entire statement, but
would continue processing the rest of the program. This left us in a
position where some Variables existed in the SymbolTable with valid,
reachable names, but their corresponding VarDeclaration statement had
been thrown away as erroneous. Since Variables point back to
VarDeclarations for their initialValues, this gave us a stale pointer.
Any future reference to that variable name which could trigger an
access to its initialValue would read from this dead pointer.
This CL fixes the conversion of VarDeclarations so that we no longer
throw away any VarDeclarations associated with a successfully-parsed
Variable.
Change-Id: If8ec3c160933e48a0e1f36414234b3a849d8978c
Bug: oss-fuzz:32587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389636
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is particularly relevant for "vector / scalar" divides, as these
end up generating an extra OpFDiv to take the reciprocal of the scalar.
This meant we were generating code to calculate "literal 1 / literal N"
at runtime whenever dividing vector V against scalar N. This fix moves
the reciprocal calculation to compile-time. And of course, it's a valid
strength reduction in all cases.
Change-Id: I0986057e516120fe872e70135f1ac365155d5377
Bug: skia:11792
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389156
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Due to missing type-checks, we would emit `OpVectorTimesScalar` when
multiplying an ivec with an int, or an `OpFDiv` when dividing an ivec
against an int.
Change-Id: Idc214dbe0ec208cb44f28b22e585584ac2ab7dae
Bug: skia:11267, skia:11788
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388742
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We had constant-folding tests for vector-scalar arithmetic, but didn't
have an equivalent SkSL unit test for vector-scalar arithmetic that
actually needs to be computed at runtime.
This exposes two SPIR-V bugs: one was previously known, but the other
is a new discovery.
Change-Id: I28737128f20b445797c6c29872335d05f94cc95c
Bug: skia:11267, skia:11788
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388739
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Previously, these tests were never actually executed, only read during
code review. They are now properly tested for correctness whenever dm
is run. Non-ES2 compliant statements (do/while/switch) are unfortunately
excluded here, as they are not compatible with Runtime Effects yet.
Change-Id: I965c782baad6f8dd3961a400ae791fb2c1f844d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389296
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If sk_Caps.rewriteMatrixVectorMultiply is false, we generate the same
code as before.
If sk_Caps.rewriteMatrixVectorMultiply is true, and optimization is
enabled, medium-precision multiplication of `matrix * vector` is
rewritten in BinaryExpression as the sum of
(m[0]*v[0] + m[1]*v[1] + ... + m[N]*v[N]).
Change-Id: I9796158fd4203b5cd2278c0d0e9a2e8cda82b136
Bug: skia:11769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388460
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
On ARM GPUs, the Vulkan driver does not honor relaxed precision when
evaluating SpvOpMatrixTimesVector. This leads to reduced performance
(compared to GLSL, where mediump matrices and floats do evaluate all
intermediate values at mediump).
This caps bit will be enabled on ARM GPUs and, in a followup CL, will
be used to toggle a workaround where `m*v` is rewritten as the sum of
(m[0]*v[0] + m[1]*v[1] + ... + m[N]*v[N]).
Change-Id: I310fa73639b6498552c9672e76860f2eded15d0a
Bug: skia:11769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388459
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 7bba1f55e8.
Change-Id: I707a3c215f37376086e22eaa43916afeed6da4c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388456
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I22837a4921238749664217e595d24d196503534d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388096
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Removed error tests which no longer test anything:
- UseWithoutInitialize...: we no longer track variable use-before-init.
- InlineDivideByZero: we no longer constant-fold the result of an
inlined function call into its parent statement.
- Unreachable: we no longer report unreachable statements.
And fixed some minor test-case issues:
- StaticSwitchConditionalBreak: we still check this, but the function
was being dead-stripped before this check could run. Renamed to main.
- OssFuzzXxxxx: these cases no longer report errors, but they are still
valuable as regression tests; moved to `shared/`.
Change-Id: Iade3cff821dc998cacfd02f62d3ac4625e48904c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387820
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
SPIR-V doesn't seem to mind overlapping function names, since they're
not load-bearing in any way, but this keeps us consistent with the other
code generators.
Change-Id: Ifdb4cb17795da88eabc0db841af746fb76caf423
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387757
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This will be implemented in Metal and SPIR-V in followup CLs.
Change-Id: I397b4db40b15dd54cf1d8a17f414c3fe184b48d2
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387638
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: If5fb4f99d327bb429f60e8d6c526720dd02b0928
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386800
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This saves a significant amount of CPU time and, now that the inliner
can handle nested expressions, still inlines almost everything.
Change-Id: I8f198630fa9627bc433ef8fb72f6bcf94595cdaa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386917
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This will allow the inliner to successfully do more work in a single
pass.
Change-Id: I26e8831737c10bdf9a35eebd94ea8b74f6487077
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386916
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Arguments without side-effects that aren't read from more than once can
be moved directly into the inlined function, and don't need a scratch
variable. This can allow functions like `guarded_divide` to inline
completely in more cases.
Change-Id: I0bfce35635cf9779f4af1bc0790da966ccfe4230
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386678
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit a9c187e5cc.
Change-Id: Icbfb8abdfc67fc2e6428d97a6cdede2726fb56e4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385596
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
It is difficult to do this both efficiently and correctly while honoring
GLSL semantics (which require the lvalues to be kept distinct, even when
they point to the same variable). We could make it work by making copies
of every out parameter in each direction (going in for inouts, and
coming out for outs and inouts).
However, this could be self-defeating if it makes it harder for the
driver to track variable lifetimes. Simply opting out of inlining these
functions entirely seems like the best tradeoff; let the driver optimize
them if it can, and we can enjoy reduced complexity in the SkSL inliner.
Change-Id: I62f7b4550cc181cfe789e4f2ff4e408ba1baf9cb
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370257
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
It turns out it is not legal to pass the results of OpAccessChain as a
function argument, for... reasons. This CL switches us over to passing
the argument via a temp variable instead.
Bug: skia:11748
Change-Id: Ib5e86c1d000655ebd7bb62ceea6a27b823808645
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This allows us to remove 100 LOC from the inliner and is very unlikely
to affect any existing benchmark. We don't have any evidence to support
the idea that a one-iteration `for` loop with `continue`-based exits
will be any faster than a standard function call on any existing GPU.
Our fragment processors are generally written to avoid early returns,
in large part to avoid hitting this path.
This drastically impacts BlendEnum.sksl (which can no longer flatten out
a switch over every blend function in SkSL) but is otherwise a wash.
See: http://go/optimization-in-sksl-inliner suggestion 4(a)
Change-Id: I1f9c27bcd7a8de46cc4e8d0b9768d75957cf1c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385377
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This put the coverage for do-while loops on par with for loops.
Change-Id: I53e0d733edd02a6a139792a8d74c68116453e5ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385500
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This can eliminate const variables which have been completely folded
away, unnecessary synthetic variables created during codegen/inlining,
or code that simply didn't need to exist at all.
Change-Id: I37a65e455e6527a6a6c2f4dde918f48d84dc2638
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383496
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
A global variable should be considered "dead" if it's never written and
never read. The previous code checked if it was never written OR never
read, which is not the same.
This would generate GLSL/Metal that didn't compile. In SPIR-V, it would
SkASSERT, then crash, during codegen. The fuzzer was able to detect the
SPIR-V issue, but it was wrong in all three cases.
Change-Id: Id59a2499eb5baa3839b93826bfbc24191bfd490b
Bug: oss-fuzz:32005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385280
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit e7a8f85e4f.
Reason for revert: must revert dependent CL
Original change's description:
> Only include header once in combined MSL shader.
>
> Bug: skia:11389
> Change-Id: I3e24dcaa2cfeddc7efd7985f9f42a59bfc8175f2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385137
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,johnstiles@google.com
Change-Id: I7a886b6c57a666e54e65365e41dcb57bd9ab4ba6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11389
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385237
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
'in' variables without locations aren't allowed. Use uniforms instead.
Bug: skia:11738
Change-Id: Ic066106deb7409cff154b4be7cfb3e03a7025c7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385000
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit 92748af1a5.
Reason for revert: SkSLCommaSideEffects_GPU crashing on Android
Original change's description:
> Inline functions of the form 'return (expr)' only.
>
> This drastically reduces the number of functions which we allow to be
> inlined. If this change does not hurt our performance, it will allow us
> to trivially remove hundreds of LOC. All current data leads us to
> believe that it may affect the Mali 400 but is highly unlikely to change
> results on any other device in the tree.
>
> More info: http://go/optimization-in-sksl-inliner
>
> Change-Id: Ia6b706742ce5407453e0e697b6c1f9201084c0e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384858
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I6a670dacaa58fe3386ff50375ac6d1cac4fd7f2c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385161
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11389
Change-Id: I3e24dcaa2cfeddc7efd7985f9f42a59bfc8175f2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385137
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This reverts commit 9e476b798f.
Reason for revert: Angry Vulkan bots
Original change's description:
> Refactored SPIR-V RelaxedPrecision handling
>
> The RelaxedPrecision decoration is now handled by nextId(), to make it
> easier to see all spots where a RelaxedPrecision decoration might be
> necessary. The goal of this initial refactor is not to actually fix the
> issues with RelaxedPrecision decorations, but rather to lay the
> groundwork for doing so in followup CLs.
>
> The initial intent of this change was to not affect the SPIR-V at all,
> saving modifications for followups, but there ended up being three kinds
> of changes to the output:
>
> 1. Doing things at nextId() time rather than later means some
> decorations move to an earlier spot in the output. This results in
> diffs, but should not cause any behavioral changes.
> 2. We were incorrectly tagging bools as RelaxedPrecision in some
> situations. By funneling things through fewer code paths, the refactor
> would have caused this to happen in even more situations, and the code
> responsible for the bug was being rewritten in this CL anyway, so it
> seemed worth just fixing the issue as part of this change.
> 3. Funneling things through fewer code paths ended up adding
> (correct) RelaxedPrecision modifiers to binary operations that had
> previously been missing them. It seemed better to just let it happen
> than to try to maintain bug-for-bug compatibility with the previous
> approach.
>
> Change-Id: Ia9654d6b5754e2c797e02226660cb618c9189b36
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384318
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I9ada728e5fd5798bc1179640560c2e6045b7efd1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385158
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The RelaxedPrecision decoration is now handled by nextId(), to make it
easier to see all spots where a RelaxedPrecision decoration might be
necessary. The goal of this initial refactor is not to actually fix the
issues with RelaxedPrecision decorations, but rather to lay the
groundwork for doing so in followup CLs.
The initial intent of this change was to not affect the SPIR-V at all,
saving modifications for followups, but there ended up being three kinds
of changes to the output:
1. Doing things at nextId() time rather than later means some
decorations move to an earlier spot in the output. This results in
diffs, but should not cause any behavioral changes.
2. We were incorrectly tagging bools as RelaxedPrecision in some
situations. By funneling things through fewer code paths, the refactor
would have caused this to happen in even more situations, and the code
responsible for the bug was being rewritten in this CL anyway, so it
seemed worth just fixing the issue as part of this change.
3. Funneling things through fewer code paths ended up adding
(correct) RelaxedPrecision modifiers to binary operations that had
previously been missing them. It seemed better to just let it happen
than to try to maintain bug-for-bug compatibility with the previous
approach.
Change-Id: Ia9654d6b5754e2c797e02226660cb618c9189b36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384318
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This drastically reduces the number of functions which we allow to be
inlined. If this change does not hurt our performance, it will allow us
to trivially remove hundreds of LOC. All current data leads us to
believe that it may affect the Mali 400 but is highly unlikely to change
results on any other device in the tree.
More info: http://go/optimization-in-sksl-inliner
Change-Id: Ia6b706742ce5407453e0e697b6c1f9201084c0e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384858
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Prevents us from accepting code that can't be correctly transformed to
GLSL, like:
uniform float x;
float y = x;
(Previously, writing code like that in a runtime effect would
effectively produce the exact same code all the way through to GLSL, and
the driver would fail to compile it).
Bug: skia:11336
Change-Id: Iaa797587c4a4a7289ed59ce2736cf0bf0fc5bca3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384698
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:11716
Change-Id: Ic09071544b5b5216b01fbc9b478b6269dd96202f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382280
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This only affects fragmentProcessors (children) - and the backend SkSL
we're emitting should not contain those. We've just been silently
ignoring those declarations when converting to GLSL, MSL, etc.
Change-Id: I241f2f4fe4614b49ebccc9c2976fd408e94656d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384316
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit a04692f69e.
Reason for revert: Angry Vulkan bots.
Original change's description:
> Fixed a number of spots where we should have been using RelaxedPrecision
>
> Our SPIR-V output was missing many RelaxedPrecision decorations, which
> was presumably impacting performance.
>
> Change-Id: Iee32d4a42f37af167fe0e45f3db94c2142129695
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384178
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=egdaniel@google.com,brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: If4fe945cb363c9b61b5a4abfde649a437689d2eb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384217
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
I ran into an issue in an upcoming CL which generated a particularly
ugly switch statement:
switch (x) {
default:
discard;}
So I cleaned this up, and while resolving this issue, managed to improve
a bunch of existing codegen as well. The formatting change has been
split out to a separate CL since it impacts so many golden outputs.
Change-Id: I7a6be29903c47560dcc7f6acd3ef15fd0c5c3c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384179
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Our SPIR-V output was missing many RelaxedPrecision decorations, which
was presumably impacting performance.
Change-Id: Iee32d4a42f37af167fe0e45f3db94c2142129695
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384178
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11738
Change-Id: I1dd5e99830f70d72c292379a45c4e39a55588858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383706
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
We no longer derive a performance benefit from this pass in practice,
and it is a very expensive compilation step. It is also prone to fuzz-
related errors.
Doc: http://go/optimization-in-sksl
Change-Id: Ief08ffac659a8fe7fe92c92b9a5da14c9f713bc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381261
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
As you might expect, a function tagged with `noinline` will never be
considered as a candidate for inlining.
Change-Id: Ia098f8974e6de251d78bb2a76cd71db8a86bc19c
Bug: skia:11362
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382337
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Currently, only one of three uses (local variables) does this correctly.
Bug: skia:11716
Change-Id: Iad11e8e5998fcc7caee4d438e0558c5d4e2b1821
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382277
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ic2d1240ab785101365b0fd934562505fb5a3e599
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381816
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Expressions like `value == value` or `color.a != color.a` can be
replaced by `true` or `false` on sight. The GLSL spec makes it clear
that checking for NaN is optional:
4.7.1 Range and Precision
"... NaNs are not required to be generated. Support for signaling NaNs
is not required and exceptions are never raised. Operations and built-in
functions that operate on a NaN are not required to return a NaN as the
result."
Change-Id: I2ad9f2dc505b638ea2904bef41b7a79a2b329551
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381262
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL will be used to test for potential performance regressions (or
improvements) that we might cause by disabling this optimization pass.
It will be reverted in ~1 day.
Change-Id: I26b7687c341eb6d81231406381c39869cfccf6d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381259
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I7a7874e58bf53978afce8a41b26092406b6490ed
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380360
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Expressions like `x * 1`, `x *= 1`, `x + 0`, `x * 0`, or `0 / x` don't
actually do anything, and can be simplified to just `x` or `0`. (The
zero case must also check that `x` doesn't have side effects, because
`0 * myFunction()` still needs to call `myFunction`.)
`0 - x` is also detected and rewritten as `-x`.
`0 / 0` is left as-is.
This logic works for scalars and vectors; matrices are left as-is.
A similar optimization also occurs in the constant-propagation pass, so
we see almost no diffs in the tests. If control-flow analysis is turned
off, we do see some improvements. (I didn't reuse the existing code at
all, since it was designed around rewriting the CFG tree, but the
concept was identical.)
Change-Id: Ia99cd81f1d4cd3dafaa43ccac6a2261e3257a185
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380356
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This enables the ternary to be optimized away in code like:
const bool SHINY = true;
color = SHINY ? add_shine(x) : x; // to --> `color = add_shine(x);`
Without constant propagation.
Also, I added a unit test for ternary expression simplification; I
wasn't able to find an existing one.
When the optimization flag is disabled, this CL actually removes the
optimization of `true ? x : y` --> `x` entirely; previously, this
substitution would be made regardless of optimization settings.
Change-Id: I93a8b9d4027902d35f8a19cfd6417170b209d056
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379297
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Swizzle optimizations now occur at IR generation time. These
optimizations are redundant with the control-flow optimization phase so
they are mostly not visible in our test output, but they do affect DSL
test results. Interestingly, they do improve our test output slightly
as well, for various reasons (e.g. we do not fully optimize lvalues in
the control-flow pass).
Change-Id: I6ebe6d71a5c22d9823b5fa500e43078915cbfb45
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372257
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This check now runs at function finalization time, before constant
propagation has occurred; this affected the "DeadIfStatement" test.
Our detection isn't smart enough to realize that a loop will run zero
times, so it treats `for` and `while` loops as always running at least
once. This isn't strictly correct, but it actually mirrors how the CFG
implementation works anyway. The only downside is that we would not flag
code like `for (i=0; i<0; ++i) { return x; }` as an error.
Change-Id: I5e43a6ee3a3993045559f0fb0646d36112543a94
Bug: skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This should be legal, and we support this, but some versions of Android
do not: http://screen/3bkQewHF3xUMn5v There's no point in allowing
these shaders to exist; they can't compile on real-world clients, and
these vardecls are borderline meaningless (as the variables being
declared aren't reachable by any other statements).
Change-Id: Ie1351933c90caee9124eeab8983364ec030b2653
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379584
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 50b1b2b90d.
Reason for revert: ending experiment
Original change's description:
> Disable control-flow analysis in SkSL. (Performance experiment)
>
> This CL will be used to test for potential performance regressions (or
> improvements?) that we might incur by disabling this optimization pass.
>
> It will be reverted in ~1 day.
>
> Change-Id: I775cdb0c95df81fa25ebbd66e4ff01f64c660f68
> Bug: skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378456
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ie385a82db237ff5651348d82b9651f8ba09375b9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379581
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Covers some common geometry processors, texture effect, etc.
This also rearranges how fp keys are arranged in the overall key. We no
longer include the key size as part of the key - this made no sense.
Instead, we explicitly include the number of children. We also put all
data for one fp before any children, so the tree can be reconstructed
more-or-less top-down.
Finally, added an "addBool" helper that reads nicer than addBits(1)
everywhere.
Bug: skia:11372
Change-Id: I4e35257fb5923d88fe6d7522109a0b3f4c4017d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379059
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This CL will be used to test for potential performance regressions (or
improvements?) that we might incur by disabling this optimization pass.
It will be reverted in ~1 day.
Change-Id: I775cdb0c95df81fa25ebbd66e4ff01f64c660f68
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378456
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit e4da7b672f.
Reason for revert: breaks SkSLBench perf test
Original change's description:
> Migrate if-statement simplifyStatement logic to IfStatement::Make.
>
> This performs essentially the same simplifications as before, just at
> a different phase of compilation.
>
> Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
> Bug: skia:11343, skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I0051188ffe69426904066eb60a932435efdc2af8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11343
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379062
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
All layout(key) fields include the field name meta-data, and use as few
bits as possible.
Bug: skia:11372
Change-Id: Ie12b3e0d01148457e5ea078cbf7d0a4bff35302e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378596
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This performs essentially the same simplifications as before, just at
a different phase of compilation.
Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
Bug: skia:11343, skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Rather than have the inliner own this responsibility, the function
finalizer now detects if a function is supposed to return a value but
never actually does. This will allow us to detect this error case even
if the inliner is disabled. The inliner should no longer encounter
functions that claim to return a value but don't, so it will now assert
if one is encountered. (The inliner still has the logic to handle this
case gracefully, just in case.)
The check is currently very simple and doesn't analyze the structure of
the function, so it won't report cases where some paths return a value
and others don't, e.g. this will pass the test:
int func() { if (something()) return 123; }
(This is good enough to resolve the inliner issue, though, as it only
occurred in functions with no value-returns at all.)
Change-Id: I21f13daffe66c8f2e72932b320ee268ba9207bfa
Bug: oss-fuzz:31469, oss-fuzz:31525, skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377196
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11356
Change-Id: I16322e6396dc7e7c8c50ba1d39e07311cf3bd346
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376116
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Interestingly, this improves our codegen even with the optimizer fully
enabled, as apparently statement chains like:
`x = true; x = x; x = x;`
were getting transformed by constant-propagation into:
`x = true; x = true; x = true;`
making them no longer candidates for self-assignment elimination.
Change-Id: I6d94a809e94b01a00fd92459fcbce898b3cbbb11
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377100
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
In http://review.skia.org/375776, an optimization was added to the
Inliner, causing it to skip generation of unnecessary temporary
variables. The fuzzer immediately discovered a flaw in this logic: the
"unnecessary" variable was actually used in the rare case that a
function failed to actually return a value. The inliner didn't detect
this case. Of course, this isn't a valid program either, so now we
report the error and cleanly fail.
Change-Id: I1f201cfd33f45cace3be93765a4e214e43a46e69
Bug: oss-fuzz:31469, oss-fuzz:31525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377101
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Surprisingly, this actually improved our error detection slightly.
The expression `- -half4(0)` can now be simplified to `half4(0)` at
IR generation time, which allows the constant-folder to detect a
constant zero (and from there, a division by constant zero).
Change-Id: I8c4f6ab522efab5bf98913f9c6a1487b7af39a99
Bug: skia:11342, skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376842
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
For now, just use this to prevent *any* layout qualifiers from appearing
on functions, or their parameters.
Bug: skia:11301
Change-Id: I05d8118c7121048c6ef49695a54e3714a8f8687e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376796
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This adds Analysis::IsConstantExpression, to determine if an expression
is a constant-expression. It now expands to cover 'const' local and
global variables, because we also enforce that the initializer on those
variables is - in turn - a constant expression.
This fixes 10837 - previously you could initialize a const variable with
a non-constant expression, and we'd emit GLSL that contained that same
pattern, which would fail to compile at the driver level. That should
not be possible any longer.
Bug: skia:10679
Bug: skia:10837
Change-Id: I517820ef4da57fff45768c0b04c55aebc18d3272
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375856
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The expression `~123` was making a PrefixExpression of type $intLiteral.
It should be converted to type `int` when the ~ prefix is applied.
This change also changes the output from oss-fuzz:27614. Both programs
are essentially nonsense expressions with no real behavior, so this is
fine.
Change-Id: I586be149ce95136fabee72fdd3473814d54948cf
Bug: oss-fuzz:31410
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376620
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
These variables were later being eliminated by the dead-code-elimination
pass, so you can't see them directly in the final output, but removing
them affects the name mangling off all future symbols, so it causes an
enormous ripple effect in the diff. And of course, it's a waste of time
and memory to synthesize IRNodes just to destroy them later.
If we disable control-flow analysis, we lose the dead-code-elimination
pass entirely; this change is also beneficial for emitting better code
when optimizations are turned off.
Change-Id: I882b3be4f3fd99b77d99b6abe128f26bb9252c89
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375776
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Iafeb13812851271a5262730e9c0642d4469c273f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375020
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Now, even if a qualifier has a default value, we will know that it
appeared in the text. We can use that to check for redundant qualifiers
(as is being done here), and in the IR generator to prevent any use of
certain qualifiers, depending on context. (eg, runtime effects, wrong
shader stage, on a parameter declaration, etc.)
Bug: skia:11301
Change-Id: I2cd6ad35c2b4c4d6f87ade97e80aea84dc16ee4b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374616
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:10837
Change-Id: I33da2eb1e723ed04ab62d65c21e54306dd362bed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372677
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Rename factory function from createGLSLInstance() to makeProgramImpl()
Bug: b/180759848
Bug: skia:11358
Change-Id: I095bdf1f26db5a8192fa8ab59000db4a1d561d96
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/373738
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
The test inputs were removed at http://review.skia.org/360778
Change-Id: Ib2918f3f984cd80463bacb822ef510ee9feb1e77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/373916
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This splits switch() construction into two stages.
- One version of Make takes an array of case-values and case-statement
lists, and is responsible for reporting errors if the case-values are
not unique or are improperly typed. This is what the IR generator or DSL
will start with on its first encounter with the switch statement.
- The other version of Make takes an array of already-processed
SwitchCases and can assume the invariant that they're all correctly-
typed with unique values. This is what we will have when a statement
is inlined or otherwise cloned. (We still assert this invariant, for
correctness' sake, but in release mode we assume it.)
This CL doesn't perform any optimizations at Make time yet; it does work
equivalent to how `switch` works in the IR generator today. It does
improve duplicate case-label checking slightly; duplicate case labels
are now reported, and duplicate `default:` labels are detected.
(Multiple `default` labels won't pass the parser, but they can be
constructed in DSL.)
Change-Id: I537ce2c8236152d58641fb1793619d66a62c01a8
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372616
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These were unused - we always enable the advanced blend equation
extension using "blend_support_all_equations" (if enabling the
extension is required at all).
Change-Id: I95fd6483ec54dfaf983290de95629fe0e86c22e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/373877
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Constant propagation might be going away, but static-switches are likely
here to stay. Avoid conflating the two in this test.
Change-Id: If4b6c99c85f124d3bbc20da858693f09f5e4fd59
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374117
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The optimizer now properly recognizes all types of exits from a switch
statement. Break, continue and return are all potential exits and need
to be considered when determining the exit path from the switch.
Previously, dead code elimination was hiding the effects of this bug
from us, but it meant that an optimized switch had the potential to
generate lots of worthless IR nodes which then needed to be detected and
eliminated by the CFG. In particular, this affected the enum form of
blend, causing a catastrophic amount of extra work to be done.
Change-Id: If857e38cadfc016884624ea4db25a273ad3dce5b
Bug: skia:11352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372958
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I672345116e3b5538c0f7e8c5f2f74aa56bb81e6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372676
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
When we detect a static switch, the optimizer finds the matching switch-
case and eliminates all the other switch-cases. It handles case
fall-through by scanning forward and looking for an unconditional break.
However, the inliner has an interesting quirk--it can replace `return`
statements inside of a switch with `continue` statements, since the body
of the inlined function has been wrapped with a for-loop to allow for
early exits. The optimizer does not recognize these continue statements
as exits from the switch (although they certainly qualify), so it
treats continues as fallen-through and keeps emitting switch-cases.
The dead-code elimination pass was actually doing us a favor here and
eliminating the excess code later. A flag was added to disable DCE in
order to reveal the problem in a test.
Change-Id: I8ff19fde5e32d0ab73d7c5411da40cb953a446f5
Bug: skia:11352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
There are two forms. Swizzle::Make supports components XYZW only;
Swizzle::MakeWith01 also supports the 01 components, and restructures
the zeros and ones into a constructor (as IRGenerator::convertSwizzle
has historically done). This means that once we are past the initial
IR generation stage, and we know that the 01 components have been
eliminated, we can avoid the extra 01-handling logic and just call
Swizzle::Make directly. This isn't a huge deal but it means that call
sites like the inliner can avoid some extra work that will never happen.
Change-Id: I46690c3d6b07feb6327ee72e8f66f15592a35554
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371398
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Surprisingly, this error is actually caught by our parser, which
interprets the default label in a unique way. From the parser comments:
"Requiring default: to be last (in defiance of C and GLSL) was a
deliberate decision. Other parts of the compiler may rely upon this
assumption."
The comment is true--we don't check for duplicate default switch-case
labels anywhere else in the code, just here in the parser.
We rely on this, so we should have a test for it.
Change-Id: I6df5c565aca4d4b8565b96638dce9504efc39ccc
Bug: skia:11340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372617
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I4df18946cdb3d9f1f7833461f913f2df94696821
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372197
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
These were all unused, and only implemented on one backend.
Change-Id: Ibd2fcef1a971e6c1bd9da0784c5d852a60708484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372117
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I885149c73be63c223ac88a697ffe046a7f8384d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372116
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:11335
Change-Id: I88c952cbfe2d2c5920e17675da1674928f37b982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371480
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:11295
Change-Id: Iec11f3f4d26eb5b1c07707b3cedd09096bad80d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371478
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
When coercing a type, we would previously call checkValid() so we could
detect function-references and type-references, so we could get a nicer
error message.
It turns out that we can just do the "is this a type-reference/
function-reference?" check directly inside coerce() and get the same
improved error messages. Since we should be coercing all our values to
the right type, and type/function-references aren't coercible to
anything, this should catch them all. I don't expect any of these
to survive all the way to the end of IR generation.
(In case one of these types does slip through, I've left the error case
in checkValid, but I've also put in an assertion. If the fuzzer can
make that assertion fire, we are probably missing a call to coerce()
somewhere.)
This cleanup is meant to help migrate coerce() out of IRGenerator.
Change-Id: I031809adf439b1766048768b782c57e7f2494006
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371479
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Adds trivial name mangling to the .stage output, so we can verify that
it's working in all places (declarations, references, etc). Also added
another global variable whose initializer is - in turn - another global.
Bug: skia:11295
Change-Id: Ic220bfae0a6d1eeeba66ade30d3d781af15c5dea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371477
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Includes variables with and without initializers. Note that both the
.skvm and .stage output is incorrect right now. (No declarations for
global variables in .stage, and the initializer is dropped in .skvm).
Bug: skia:11295
Change-Id: Icb6d797616be6a1bc7cbdc9db4fefa7e30c65656
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371143
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
None of these are legal in GLSL ES 1.0. Added a new test that previously
compiled without error. Started out with just assignment and equality,
then realized that sequence and ternary should be blocked, too.
Bug: skia:11323
Change-Id: I02691f819565afabeadbb12cab6c07acf40093f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370880
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
When SPIR-V generates function calls to an intrinsic, it assumes that
it can get a pointer to out-parameters referenced by the intrinsic.
This does not account for swizzled out-parameters; these are valid
lvalues, but do not work with getPointer().
The two intrinsics supported by SkSL which have an out-parameter are
frexp and modf, so these tests were fleshed out to trigger the error.
Neither of these are supported in ES2, though, so we cannot test them
via Runtime Effects.
Change-Id: Ib92707a28ba6d1c282d20e29a2a387bddf74ad23
Bug: skia:11052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370116
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The out-param helpers emitted by the Metal code gen (intended to provide
GLSL out-parameter semantics in Metal) emitted bad code if passed the
same variable for two separate out parameters. It would previously
create two parameters in the helper with the same name. The helper
function now omits the name of the second variable in the parameter list
if it is redundant; we already know the caller is passing the same
variable twice.
Change-Id: Ibdc6c02a9e9e4bdb4f4546a25068f2018aa07b10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370258
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
GLSL ES2 documentation on out parameters: "Evaluation of an out
parameter results in an l-value that is used to copy out a value when
the function returns."
The inliner does not do any alias checking when inlining an `out` param.
That is, passing the same variable to two separate `out` parameters
would not generate two distinct lvalues in the inlined code; it reuses
the same variable for each out-params in the inlined code.
(Amusingly, our CFG can fully optimize away this test code so it just
returns "red".)
Change-Id: Ib781d2cfdac54f01b6abe159af0c84ff24ff6976
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Multi-dimensional arrays aren't legal in GLSL/SkSL, so this should be
caught and flagged as an error. The parser now verifies that a
variable's type isn't an array-type before accepting a `[` token to
open an array on the variable name.
This CL also refactors the IR generator's `convertArraySize` method to
make sure that various checks are made for all callers. Originally this
restructuring was used to verify array multi-dimensionality, but that
didn't detect errors inside struct declarations (which get no error
checking inside the IR generator) so the IR generator updates no longer
need to check the array dimensions.
Bug: skia:11322
Change-Id: Id33f4bdfb544019ddf995a8196c3c09cfe5a4525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369916
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We now interpret any statement of the form `Type identifier...` as a
var-declaration and report errors as such. Previously, if a var-decl
statement generated an error during parse, we'd report errors as if it
were an expression-statement, which meant that slightly-invalid code
could return out-of-context, misleading errors.
Bug: skia:11287
Change-Id: I2c6cf2984760eb34593c80cb30f8c4e007d42027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370036
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11314
Change-Id: I66476543462ae378a5bfb6cbd902dfa2f5fc45f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369917
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The CL at http://review.skia.org/366399 introduced a bug with
LValue::getPointer. Specifically, getPointer used to return zero when
no pointer is available. (This happens when the LValue is a swizzle.)
That CL changed the error code to -1. However, it did not fix up all
the call sites that checked the return value of getPointer().
This CL fixes up those call sites to use -1 consistently, and adds
TODOs in spots which do not check the result from getPointer() at all
(instead assuming it cannot fail). This will allow swizzled out-
parameters to work in SPIR-V as they did before. (Except in intrinsics,
where they seem to have been broken all along, but those are now marked
with a TODO at least.)
Note that we still do not fully emulate GLSL semantics for out
parameters, as out-parameters should only be copied back to the original
variable at the end of the function call to be fully GLSL compliant.
(This CL also replaces a tuple with a named struct for readability.)
Change-Id: I708dc7a69296a4244ba9ceb85c3e68d1f331bbc9
Bug: skia:11052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368618
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Uses the pipeline-stage callback mechanism. It mangles the type name
(with a test to verify that this works), and then calls defineStruct
with the entire SkSL struct definition string.
Bug: skia:10939
Change-Id: If14cf1b11faaa80ad8d4086cdacf68532bac43fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368809
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This was being set to zero instead of one by mistake. Interestingly,
this was undetected by the CPU backend, but appears to matter sometimes
on the GPU side.
Change-Id: If827863f69c140f933696c6ff55c8a7095620c29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368858
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is just like mul(F32,F32) but optimizes 0*x == 0.
Use it in SkSLVMGenerator; sksl already applies this optimization.
PS2 has a sneaky version using % as a fast_mul() operator, and
PS3 has a sneakier version using ** instead.
We could of course write this all out using fast_mul() the long way,
but I found that quickly became difficult to read.
Change-Id: Iae35ce54411abc00e7729e178eb6a10f151a5304
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368838
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Fixes another instance of anglebug.com/2098 with advanced blend
functions.
Change-Id: I91863723d8b4c33ab2f5a527fe0374e8947bba16
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368813
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Disabled on Adreno 5xx/6xx as the tests do not pass on those GPUs:
http://screen/3Dkgs9syj37cjBV
Change-Id: Ib935d01e8f06dbfe7decd5cc4e52e0688b48be08
Bug: skia:11306, skia:11308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368805
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 4908a24d4b.
Reason for revert: test fails on Adreno 5xx/6xx, will land tests
separately and disable on Adreno
Original change's description:
> Revert "Add support for matrix == and != in Metal shaders."
>
> This reverts commit c501857188.
>
> Reason for revert: breaking many bots
>
> Original change's description:
> > Add support for matrix == and != in Metal shaders.
> >
> > We need to polyfill an operator== and != when these are first
> > encountered in the code.
> >
> > Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
> > Bug: skia:11306
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: Id583109a0d167c2c58a57644b14cd5f49d670737
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11306
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368801
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Bug: skia:11306
Change-Id: If7c628b8c7a2ce40d6c88599a7660ff91c4ac67a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368804
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
"Constant" is an address space qualifier and can't be applied to a
local variable. "Const" in GLSL (and hypothetically SkSL) is meant to
apply to a constant expression regardless of address space.
Our previous test was not finding any error because the optimizer was
eliminating the constant expressions entirely.
Change-Id: I6cfe8e2a621c79945b33e0166780d81e79890a1b
Bug: skia:11304
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368517
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit c501857188.
Reason for revert: breaking many bots
Original change's description:
> Add support for matrix == and != in Metal shaders.
>
> We need to polyfill an operator== and != when these are first
> encountered in the code.
>
> Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
> Bug: skia:11306
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Id583109a0d167c2c58a57644b14cd5f49d670737
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11306
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368801
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
We need to polyfill an operator== and != when these are first
encountered in the code.
Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
Bug: skia:11306
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This emits SkSL that is more-or-less what the compiler re-ingests when a
runtime effect is used to create a GrFragmentProcessor.
Change-Id: I0926be44fc4493e722a5edc18198e161e4192cde
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367883
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Currently, SkSL is able to constant-propagate `x = x + constant` into
`x = constant` when the starting value of x is known. However, it is not
able to do the same optimization for `x += constant`. This test
demonstrates that once += is encountered, we lose track of x's value and
can no longer propagate its value.
(This is equally true of all the op-assignment operators, += -=
*= /= etc.)
Change-Id: I3523e96baf9a73982cf3b09f0d23b95adacf106b
Bug: skia:11192
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368248
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The leftover tests in shared/ are not easily testable as Runtime
Effects; they do things that ES2 doesn't support or use a feature not
exposed directly by Runtime Effects.
Change-Id: I7ebe170cf713c4a0d2dbef333c1fcbac2410c67f
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367059
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
These cover new ground; when combined with some additional optimization
work, they can cause crashes in the optimizer that we don't see from any
existing test.
Change-Id: I3958a5522cfe0929d0753e6e617d72e032c7f5a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367063
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
SkSL.
It would previously catch 1 / 0, but fail to detect x / 0.
Bug: skia:11051
Change-Id: I3adb5942cce03a7ad40a13a8ca5d5a7f2029d6ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366720
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
This takes away one of our gadgets for thwarting dead-code elimination
in unit tests, but it's the right thing to do. Comma expression left-
sides without side effects are clearly dead code.
Change-Id: Iaee490b4a742d06a0a0be94cddaa69a51543d8f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366719
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fixes the OutParamsTricky test.
Change-Id: If59637bc946b71b141ae1d90cf1652bf80b163c4
Bug: skia:11269
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366399
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
- MultipleAssignments
- NegatedVectorLiteral
- NumberCasts
- OutParams
- OutParamsTricky (disabled on GPU due to skia:11269)
Change-Id: I87dc9c5019931f3d2dc3aafbe1e02d0eee2e1a05
Bug: skia:11009, skia:11269
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366400
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is enforced by ANGLE in Strict ES2 mode; we need to enforce it as
well.
Change-Id: I6e2f547ad8e0ce817742cf84659764cf6bce38b9
Bug: skia:11270
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366339
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 38df4c8470.
Reason for revert: updated ArrayTypes test for ES2 compatibility
Original change's description:
> Revert "Improve support for arrays in Metal."
>
> This reverts commit dd904af566.
>
> Reason for revert: breaks ANGLE
>
> Original change's description:
> > Improve support for arrays in Metal.
> >
> > Arrays in Metal now use the `array<T, N>` type instead of the C-style
> > `T[N]` type. This gives them semantics much more in line with GLSL,
> > so they can be initialized and assigned like GLSL arrays.
> >
> > This allows the ArrayTypes and Assignment tests to pass, so they have
> > been added to our dm SkSL tests. (ArrayConstructors also passes, but
> > is not ES2-compliant so it is not enabled.)
> >
> > Change-Id: Id1028311963084befd0e044e11e223af6a064dda
> > Bug: skia:10761, skia:10760, skia:11022, skia:10939
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365699
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: If6a18dea7d6a45fa7836e9129bf81c2e536f07e3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10761
> Bug: skia:10760
> Bug: skia:11022
> Bug: skia:10939
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365976
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Bug: skia:10761
Bug: skia:10760
Bug: skia:11022
Bug: skia:10939
Change-Id: Ia1c4917f5d3c41162d282b3093814d861707ad30
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366144
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
