Commit Graph

14 Commits

Author SHA1 Message Date
John Stiles
4691b0f82b Fix crash when compiling FP files containing the modulo operator.
Change-Id: Idfbe128978575ab84b54485bffe2d82570ee099f
Bug: skia:10870
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330620
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-10-29 22:55:28 +00:00
John Stiles
8bc1a72cba Add unit test to demonstrate error with modulo in FP files.
(This CL also adds modulo to the IntFolding shared test, since this was
absent from the test. It's implemented and working properly already.)

Change-Id: I24a947ab38754bff2624cd5b58cf7a39553ca888
Bug: skia:10870
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330596
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>
2020-10-29 20:46:08 +00:00
John Stiles
fdf6148102 Pass function arguments using SkSpan instead of count + ptr.
There's no functional change here; it's just using a slightly higher-
level abstraction to pass the same payload.

Change-Id: Ife7efa038db5d6dbde5decae2be79ad9db877aba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329782
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-10-27 14:41:04 +00:00
John Stiles
890363ae34 Prototype helper functions from FP files before use.
GLSL requires that functions are declared before first use. In most
cases, we avoid the requirement for explicit prototypes by this by
strategically ordering our functions within the emitted code, but an FP
file might reference its helper functions in any order or have helper
functions that cross-invoke each other, so prototypes should be emitted.

Change-Id: I3b9e9c9ec4bd5be90b4f71f8165af45364facf30
Bug: skia:10872
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329781
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-10-27 14:33:00 +00:00
John Stiles
6b58a33c16 Mangle function names as a separate step before emitting func-bodies.
This is necessary to support function calls in FP files properly; in
some cases, functions can be referenced before they have been emitted,
and we need to be able to name them.

This CL resolves the remaining errors in GrRecursion.cpp. There are
still additional errors in GrNestedCall.cpp that will be fixed in
followup CLs.

Change-Id: Iec98ef02ea6a98a9945a4e0e3cfa3537dff01305
Bug: skia:10684, skia:10872
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329676
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-10-27 14:22:10 +00:00
John Stiles
d2a3a5b3b4 Add support for fFormatArgs in .fp-file inner functions.
Previously, we'd just emit functions with `%s` in their function bodies.
Also, the fFormatArgs wouldn't get cleaned up, so subsequent code would
fill in the wrong format arguments in each place.

There are still problems with function calls (`sample` is broken;
function names are accessed before they've been declared or initialized)
but this is a step in the right direction.

Bug: skia:10684
Change-Id: I7399a71d44ebba5049703484ec9933dffbe8e2b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329417
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-10-26 16:15:33 +00:00
John Stiles
8744a5c820 Add unit test for nested function calls in FP files.
`func1` and `func2` emit bad code, `return %s()`, and because they don't
consume their `%s` format argument. This leaves the format argument list
unbalanced and all future args are wrong.

Another serious problem is that we don't actually know the names of the
functions that they need to call, because we haven't emitted them yet.

`func3` is not emitted at all. Sampling from a fragment processor
apparently fails in this context.

This is a more general case repro for skia:10684--it turns out that
recursion in particular wasn't the issue, but nested function calls just
don't work properly at all in FP files. This wasn't an issue in practice
because we don't have any existing FP files which nest function calls,
and the inliner also tends to aggressively flatten everything out if we
don't explicitly disable it.

Change-Id: Iff029c459c7d90be566f9b4c9be0e3150e459866
Bug: skia:10684
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329367
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-10-23 21:07:36 +00:00
John Stiles
4c412bce4c Revert "Reland "Remove inliner from IR generation stage.""
This reverts commit e497a08065.

Reason for revert: Pinpoint disagrees

Original change's description:
> Reland "Remove inliner from IR generation stage."
>
> This reverts commit 941fc7174f.
>
> Reason for revert: performance now seems to be roughly equal or better
> (~1%) over several trials.
> Nanobench: http://screen/A8e8sojaXBgbMgF
>
> Original change's description:
> > Revert "Remove inliner from IR generation stage."
> >
> > This reverts commit 21d7778cb5.
> >
> > Reason for revert: Pinpoint absolutely hates this change
> >
> > Original change's description:
> > > Remove inliner from IR generation stage.
> > >
> > > There is no need to inline code during IR generation, as the optimizer
> > > can now handle this.
> > >
> > > Change-Id: If272bfb98e945a75ec91fb4aa026e5631ac51b5b
> > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315971
> > > Commit-Queue: John Stiles <johnstiles@google.com>
> > > Commit-Queue: Brian Osman <brianosman@google.com>
> > > Reviewed-by: Brian Osman <brianosman@google.com>
> > > Auto-Submit: John Stiles <johnstiles@google.com>
> >
> > TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
> >
> > Change-Id: I62c235415bcdc92a088e2a7f9c3d7dbf7e1bf669
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317976
> > Reviewed-by: John Stiles <johnstiles@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I6189806c678283188f4b67ee61e5886f88c2d6fc
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/324891
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I79149467565f22f53b8c28868dd53b80f3421137
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/325626
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-10-13 15:20:28 +00:00
John Stiles
e497a08065 Reland "Remove inliner from IR generation stage."
This reverts commit 941fc7174f.

Reason for revert: performance now seems to be roughly equal or better
(~1%) over several trials.
Nanobench: http://screen/A8e8sojaXBgbMgF

Original change's description:
> Revert "Remove inliner from IR generation stage."
>
> This reverts commit 21d7778cb5.
>
> Reason for revert: Pinpoint absolutely hates this change
>
> Original change's description:
> > Remove inliner from IR generation stage.
> >
> > There is no need to inline code during IR generation, as the optimizer
> > can now handle this.
> >
> > Change-Id: If272bfb98e945a75ec91fb4aa026e5631ac51b5b
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315971
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I62c235415bcdc92a088e2a7f9c3d7dbf7e1bf669
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317976
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I6189806c678283188f4b67ee61e5886f88c2d6fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/324891
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-10-12 16:53:27 +00:00
John Stiles
647a9bd7c2 Convert the remaining FP tests to golden outputs.
(Removed one test, SkSLFPSwitchWithMultipleReturnsInside, because it was
redundant with existing tests.)

Change-Id: I1bfc069babdb5eb0cc515f195c3a2e307bb5871a
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319029
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>
2020-09-23 16:21:58 +00:00
John Stiles
941fc7174f Revert "Remove inliner from IR generation stage."
This reverts commit 21d7778cb5.

Reason for revert: Pinpoint absolutely hates this change

Original change's description:
> Remove inliner from IR generation stage.
> 
> There is no need to inline code during IR generation, as the optimizer
> can now handle this.
> 
> Change-Id: If272bfb98e945a75ec91fb4aa026e5631ac51b5b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315971
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I62c235415bcdc92a088e2a7f9c3d7dbf7e1bf669
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317976
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-19 12:47:25 +00:00
John Stiles
21d7778cb5 Remove inliner from IR generation stage.
There is no need to inline code during IR generation, as the optimizer
can now handle this.

Change-Id: If272bfb98e945a75ec91fb4aa026e5631ac51b5b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315971
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-18 18:41:08 +00:00
John Stiles
6dc5265ae6 Migrate several SkSLFP tests to golden outputs.
Change-Id: I83a38f2c953a560fea3483e95e31df532b90773e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317456
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-17 16:46:18 +00:00
John Stiles
d836f84ab8 Generate SkSL golden output files from test inputs during the build.
Golden SkSL outputs are intended to eventually replace the majority of
our unit tests, since they can automatically update themselves when we
change implementation details of the compiler.

If you change the compiler output without updating the Golden files, the
CheckGeneratedFiles housekeeper will be triggered. Set
`skia_compile_processors` or `skia_compile_sksl_tests` to true in your
GN args to regenerate them.

Almost all of the tests from SkSLFPTests.cpp and SkSLGLSLTests.cpp can
be migrated into separate unit-test .fp/.sksl files in a followup CL.

hcm@ has signed off on removing the copyright boilerplate preamble from
our unit test files.

Change-Id: I9e24a944bbac8f8efd62c92481b022a0b1ecdd0b
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316336
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-14 14:54:12 +00:00