Commit Graph

11 Commits

Author SHA1 Message Date
John Stiles
345d72124d Eliminate inliner temporary variables for functions with a single exit.
When we determine that a function only contains a single return
statement, there is no need to create a temporary variable and store the
result expression into a variable. Instead, we can directly replace the
function-call expression with the return-statement's expression.

This dramatically simplifies the final optimized output from chains of
very simple inlined functions, which is a very common pattern for trees
of Skia fragment processors.

Change-Id: I6789064a321daf43db2e1cef4915f25ed74d6131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344665
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-12-16 17:26:06 +00:00
John Stiles
d059005b93 Avoid creating unnecessary scopes during inlining.
The additional scopes were harmless, but didn't really add any value.
Originally they were used to tightly scope inlined variables, but we now
mangle inlined variable names.

Change-Id: I7b35e737598036c0b6d3d9f71cbcd4a53d609ce9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344757
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-12-15 21:11:45 +00:00
John Stiles
0f46450775 Reland "Remove inliner from IR generation stage."
This reverts commit 4c412bce4c.

Reason for revert: investigating Pinpoint failure cases, if any

Original change's description:
> 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>

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

Change-Id: I2727bd4a2b43e8d12b36b1979ce6fe4a2d935380
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-11-20 18:44:07 +00:00
Brian Osman
2e25ff436a Directly compute call counts, rather than mutating state
This causes a ~4% regression on sksl_large, but some of that
can be bought back in two ways:

1) Removing (now unnecessary) cloning of program elements
2) Hoisting the new analysis passes, with (nontrivial)
   logic to update/maintain the call counts as we edit IR.

Also, this fixes bugs where we were emitting functions that
had "calls" from no-longer called functions.

Bug: skia:10776
Change-Id: I4f8c29957be2e4233a883c9a1125f363b82ee40c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/327198
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-10-19 16:08:39 +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
0f37907ac7 Undo call-count fix for intrinsic functions.
This CL is conceptually a revert of http://review.skia.org/320258,
although the code has changed shape a bit since that CL was landed.

This fix was too aggressive, and can lead to functions being dead-
stripped while they still have an active reference.

Change-Id: I6ce8b0ad9cc2a42e8be8cb10d3a8219149eca6aa
Bug: skia:10776
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/325462
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>
2020-10-12 15:58:27 +00:00
Brian Osman
c021360a88 Only include one variable per declaration statement
This removes VarDeclarationsStatement entirely. VarDeclaration instances
appear directly as statements in Programs. SkSL that declares multiple
variables in a single declaration is transformed to represent that as a
series of VarDeclaration statements.

Similarly, global variable declarations are represented by
GlobalVarDeclaration program elements, one per variable.

Bug: skia:10806
Change-Id: Idd8a2d971a8217733ed57f0dd2249d62f2f0e9c5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323102
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-10-07 17:27:01 +00:00
John Stiles
bc0c29ead3 Fix call counts for intrinsic functions.
This allows dead-stripping to properly optimize away unreferenced clones
of intrinsic functions, and allows the inliner to detect intrinsic
functions that are only called once (which can generally always be
inlined without penalty).

Change-Id: I0cf034d880ae5d52f4cc0f93de6e2c7aad34e975
Bug: skia:10776
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320258
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-28 18:04:34 +00:00
John Stiles
aeae3a58e3 Add golden outputs for the Metal backend.
This CL also updates the blend tests to use sk_FragColor instead of
returning a half4, as the Metal backend assumes that a fragment
processor's `main` will return void and always synthesizes a
`return *_out;` at the end.

(context link:
https://osscs.corp.google.com/android/platform/superproject/+/master:external/skqp/src/sksl/SkSLMetalCodeGenerator.cpp;l=803;drc=842d31b14159626054e01dd32826563a8f4214bf )

BYPASS_INCLUSIVE_LANGUAGE_REASON=see http://b/168134166

Change-Id: I330a456bf25ee72d3a29c59cd624625378ae80a0
Bug: skia:10649, skia:10757, skia:10758, skia:10759, skia:10760, skia:10761, skia:10762
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319409
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2020-09-25 17:46:43 +00:00
John Stiles
371fde549e Rename 'DefaultSettings' golden outputs to 'StandaloneSettings'.
I realized that "DefaultSettings" as a name suffix was unclear, because
"Default" is a different settings mode from skslc running with
--nosettings.
In --nosettings mode, skslc uses "standalone" settings.

Change-Id: I1f5d80df0a21cec55948c4ad146169bcb34f4999
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318210
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-21 22:15:51 +00:00