Removed a special case from `writeFunctionCallArgument` which avoided
using a scratch variable for out params; now we always use the scratch
variable and copy it back to the original variable at the end.
Change-Id: I0e446a3fde6d19554943384210bd911f6f9c8cfa
Bug: skia:11052, skia:11919, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501836
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We don't need to initialize input values for `out` params. (We were
treating them the same as `inout`.)
Change-Id: Ib447d15de237a6a03740ad012180691dc60a50bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446717
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously we were not honoring the variable precision at all (passing
null to nextId).
Change-Id: I0e217e6c0d3d701dc0540d4d4069a3597abdad11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446640
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 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>
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>
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 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>
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>
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>
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>
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>
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>
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>
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>
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>
- 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 will allow us to load these inputs for unit testing in `dm`.
Change-Id: Id256ba7c30d3ec94b98048e47af44cf9efe580d5
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357282
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
