Commit Graph

52 Commits

Author SHA1 Message Date
John Stiles
f201af8b00 Allow more types of expressions to be directly inlined.
The following types of expression are hoisted directly into the
inlined code:

- Struct field access: `myStruct.myField`
- Swizzles: `myVector.xzy`
- Simple array indexes: `myArray[0]`

This significantly reduces the number of temporary variables generated
by the inliner.

Change-Id: Ifed226ecc87b096ec1e38752c0c38ae32bd31578
Bug: skia:10737, skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319919
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>
2020-09-30 00:36:40 +00:00
John Stiles
3d921f1b2c Add unit test to demonstrate lack of SkSL swizzle optimization.
The right side of assignments will collapse redundant swizzles, but the
left side does not. Code like this can actually cause the ByteCode-
Generator to assert if it is run (it asserts in `swizzle_is_simple`
during code generation).

Change-Id: I891912fe0b5de2670dfa95f6702a86d5c42bb2ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320296
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 19:02:03 +00:00
John Stiles
06febefd7e Add 'tricky' OutParams golden output.
This was adapted from a test in SkSLInterpreterOutParams and presents a
challenging double swizzle.

Change-Id: Icb7b3bbb18d4b3cfa0c26acb524c08812ba88096
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319920
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:52:13 +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
e41b4ee49e Reland "Support out parameters that use a swizzle."
This is a reland of 435b482638

inlineStatement now takes a `const Expression* resultExpr` instead of
`const Expression& resultExpr` because resultExpr will be null for a
void function.

Original change's description:
> Support out parameters that use a swizzle.
>
> This CL also removes the `VariableExpression` class that was briefly
> added in a prior CL. This class was intended to support cloning an
> expression while changing the refKind of a VariableReference inside of
> the expression, but it added state and complexity. In this CL, rather
> than track this via extra state, the inliner just recurses into the
> expression as needed to find its VariableReference. Since most relevant
> expressions are just a VariableReference anyway, this is inexpensive.
>
> Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
> Bug: skia:10756
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
> 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>

Bug: skia:10756
Change-Id: I35f76c21eccf0ba2ab47e4313e131f7aa26980fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320223
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-28 17:34:03 +00:00
Brian Osman
6bfa63fe39 Revert "Support out parameters that use a swizzle."
This reverts commit 435b482638.

Reason for revert: ASAN/UBSAN unhappy.

Original change's description:
> Support out parameters that use a swizzle.
>
> This CL also removes the `VariableExpression` class that was briefly
> added in a prior CL. This class was intended to support cloning an
> expression while changing the refKind of a VariableReference inside of
> the expression, but it added state and complexity. In this CL, rather
> than track this via extra state, the inliner just recurses into the
> expression as needed to find its VariableReference. Since most relevant
> expressions are just a VariableReference anyway, this is inexpensive.
>
> Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
> Bug: skia:10756
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
> 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>

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

Change-Id: Ibdda47607f9e6e7f3a7459915067cf5e20919993
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320220
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-09-28 15:36:35 +00:00
John Stiles
435b482638 Support out parameters that use a swizzle.
This CL also removes the `VariableExpression` class that was briefly
added in a prior CL. This class was intended to support cloning an
expression while changing the refKind of a VariableReference inside of
the expression, but it added state and complexity. In this CL, rather
than track this via extra state, the inliner just recurses into the
expression as needed to find its VariableReference. Since most relevant
expressions are just a VariableReference anyway, this is inexpensive.

Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
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>
2020-09-28 15:04:33 +00:00
John Stiles
7a6935a528 Update OutParams unit test to demonstrate SkSL failure.
Swizzles in combination with out params are currently broken in SkSL.
They are fixed in the followup CL at http://review.skia.org/319917

Change-Id: I22d9436a15631e6ee2acf9fc312a8634cf3b5407
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319918
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 14:09:03 +00:00
John Stiles
68861e3913 Add unit tests for assignment and invalid field access.
Change-Id: I8b755ae0078d6353e24834cd15603091d681114c
Bug: skia:10766
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319698
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-25 21:42:22 +00:00
John Stiles
c14defb8bf Disallow assignment to unfoldable ternaries in SkSL.
GLSL does not support assigning to ternaries, and will fail to compile
and/or generate non-functional shaders if we pass in a shader that tries
to assign into a ternary expression.

If SkSL is able to completely eliminate the ternary (e.g. if it boils
down to a simple `true ? x : y` or `false ? x : y`), SkSL can strip out
the ternary entirely and generate valid GLSL. This case is harmless and
so it is still allowed.

Change-Id: I960f119fb9934f998697634e6c4e519cd77d3780
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319679
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-25 21:26:40 +00:00
John Stiles
3fabfc0bd1 Fail gracefully when Metal encounters a geometry shader.
Change-Id: I452e52a87d89cefb5c21a0d9d57e9771f3038d73
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319783
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2020-09-25 21:21:50 +00:00
John Stiles
ba38588d88 Remove sk_ClipDistance.
This was unused and did not work on non-GLSL backends.

Change-Id: I6bd314d43cfefa64871b5c0e964b5ae52e494164
Bug: skia:10757
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319778
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
2020-09-25 20:29:10 +00:00
John Stiles
dce4d3e2b1 Migrate setRefKind assignability checker into SkSLAnalysis.
This will allow the inliner to use IsAssignable.

Change-Id: Ic94f71002779b53d0b3dc97f37fbe4bb98b026d8
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319414
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-25 19:48:39 +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
ab4ab20fcf Migrate most SkSL tests currently in /glsl/ to /shared/.
These tests will also be used for Metal and SPIR-V testing. A small
handful of GLSL-specific stragglers (#version-specific or type-precision
related) will remain in /glsl/.

Change-Id: I7f2b2bd92825c327922c8ce74e438d2daa440dff
Bug: skia:10649
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319408
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2020-09-25 17:13:53 +00:00
John Stiles
978674a23e Fix crash with invalid out parameters.
Many calls to `setRefKind` failed to check the return value; if it's
false, an error has occurred and the program is in a bad state.
Specifically, there is an assignment to a variable that's not marked as
"written-to." If we continue processing the program, we're likely to
assert.

Change-Id: I2dd5d1f41aa5ca0d30f8d638f05fe2e838216d78
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319116
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-23 20:36:17 +00:00
John Stiles
59b2a92c96 Add new unit tests for SkSL.
These cover:
- Properly configured out-params
- Invalid/non-lvalue out-params, which currently cause an SkSL crash
- Interactions between the inliner and variable swizzles

Change-Id: I4874101236084f273e704d8717149b431d813883
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319036
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-23 20:30:47 +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
544a32ff23 Promote bool(k) to true/false during constant propagation.
Cleaned up some nearby code while implementing this fix as well.

Change-Id: Ic084451f0d9fc12169e1720a8889a290249eb5e9
Bug: skia:10750
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318796
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-22 21:18:49 +00:00
John Stiles
9878d9e62f Fix nondeterminism when copying intrinsic functions.
Previously, we copied intrinsic functions in a totally arbitrary order;
it used an unordered_set of pointers, so it could be affected by
switching standard libraries OR by malloc nondeterminism. (Surprisingly,
it was fairly consistent in practice on OS X/Linux.) This CL sorts the
intrinsic functions into a consistent order before copying them.

Change-Id: If90342bb77a9ae237a3ce91be3a9847311a722c4
Bug: skia:10749
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318700
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2020-09-22 20:20:19 +00:00
John Stiles
d0e4840b11 Fix crash when swizzling a bvec with constant 0/1s.
Code like
  bool4 result = val.xy01;

Will now be converted to:
  bvec4 result = bvec4(val.xy, bool(0), bool(1));

Previously it tried to do this, but there isn't an implicit conversion
from int to bool, so it was silently failing and adding nulls into the
constructor:
  bvec4 result = bvec4(val.xy, $coerceToBool(0), $coerceToBool(1));

This CL also cleans up some related code that I was checking while
trying to understand the nature of the error.

Change-Id: I5b7d96760a03170ff78b46251c4182cc4e89836f
Bug: oss-fuzz:25781
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318636
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-22 19:08:42 +00:00
John Stiles
5f35ac9faa Add unit test for swizzling booleans.
This test currently crashes skslc, but will be fixed in the followup CL.

Change-Id: I3683d94a310242e8ca67560296518fd1223b28d0
Bug: oss-fuzz:25781
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318656
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-22 18:47:19 +00:00
John Stiles
a9be76de8b Reduce the number of inliner temporaries by swizzling.
Ideally the inliner would be smart enough to avoid creating a temporary
at all just for a swizzle, but a good first step is to create fewer of
them.

Change-Id: Icd6f86c294237488f7923dc787bb64a5f99bd0ac
Bug: skia:10737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318213
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-22 15:43:19 +00:00
John Stiles
1a49a5334c Restructure blend functions to allow for smarter inlining.
Early returns can cause the inliner to generate suboptimal code. We
control our built-ins, so let's avoid them where we can.

Patterns like this challenge the inliner:
    if (x) return y;
    return z;

But this can be replaced by equivalent code that inlines better:
    return x ? y : z;

Or, if a ternary can't be used, this also does a better job:
    if (x) return y;
    else return z;

In several cases, this allows the inliner to avoid generating a
do-while(false) block for control flow.

Change-Id: I921c929122297c40476ff15b4da631fc1581e308
Bug: skia:10737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318211
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-21 22:42:01 +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
John Stiles
2b788b11e0 Create golden outputs for SkSL blend functions.
Several blend functions generate a surprisingly large amount of code,
and appear to have opportunities for further optimization. At any rate,
if we make compiler changes that would affect the output of a blend
function, I think we would want to see the changes reflected in our
golden outputs.

Change-Id: Iff612dcd4bad8824b5e6e97413ffce19e5ea1c0e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318336
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>
2020-09-21 21:53:41 +00:00
John Stiles
046828f239 Fix generated file.
This wasn't rolled back automatically by the revert at
http://review.skia.org/317976, because this unit test did not exist yet
when that CL was submitted.

Change-Id: Ib887e74ddd32c1e576908bbe3d588205e26cdaa5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317978
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-19 14:53:43 +00:00
John Stiles
881a10c6e8 Revert "Add program-settings flag to disable the inliner."
This reverts commit 910845fac1.

Reason for revert: IRGenerator inline change reverted

Original change's description:
> Add program-settings flag to disable the inliner.
>
> Change-Id: I6c4e7f6a2aab6710221029022a3a5f3ec323c5e2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317856
> 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: Ie38a29495ea8497f9db26d2603df179e696ac5ff
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317977
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-19 14:13:54 +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
ca4d074d54 Fix unit test for InlinerWrapsEarlyReturnsWithDoWhileBlock.
This resolves the following TODO block:
   TODO(johnstiles): the skslc standalone caps bits do not enable
   do-while support, so this test does not actually perform as
   described; the `returny` function is not inlined at all. This will
   be fixed when customizable caps-bit support is added to the golden
   tests.

Change-Id: I3495e4813b9be37264a8fda978453594c1f5fa13
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317859
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-18 21:08:52 +00:00
John Stiles
64fc15a279 Add unit test for a dead do-while loop.
Ideally the optimizer should be able to detect and remove this loop.
This CL establishes a baseline.

Change-Id: I6aba0b52fe49552f170fca25d81c29c515044ef5
Bug: skia:10737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317861
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-18 20:57:12 +00:00
John Stiles
910845fac1 Add program-settings flag to disable the inliner.
Change-Id: I6c4e7f6a2aab6710221029022a3a5f3ec323c5e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317856
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 20:30:22 +00:00
John Stiles
be0a9ca63d Migrate remaining SkSL GLSL tests to golden outputs.
Change-Id: I72fd3083f75ca5bf74fb2c3b032465864a13aed5
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317771
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-18 19:22:19 +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
87e6ccde21 Migrate geometry SkSL tests to golden outputs.
Change-Id: I01c150d6bfcdd1500033521a87c058c7428c3521
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317769
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-18 17:34:22 +00:00
John Stiles
c884631f99 Migrate Derivatives tests to golden outputs.
Change-Id: I35ff25c4cc394c1a4a964207ece87095a9ba84cf
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317767
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-18 17:30:07 +00:00
John Stiles
6798e5d0d1 Migrate SkSL error tests to golden outputs.
Change-Id: Ic8f4730d035981c32b4ddb48e5e919b0396b6d93
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317578
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-18 13:46:17 +00:00
Brian Osman
bf2163f267 SkSL: Only allow bitwise ops on integral types
We were allowing these ops on floating point types. The resulting GLSL
would fail to compile. We also allowed >>= and <<= on vectors (but not
any of the other bitwise ops).

The newly added unit tests were failing (eg, not catching errors). New
results are correct.

Bug: skia:10707
Change-Id: I97b769f1ce59261361109a71061b42dc8ef3c74b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317393
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-09-17 18:12:34 +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
0ed9f31f6a Migrate several GLSL workaround unit tests to golden files.
We now support building an SkSL golden output twice, once honoring the
custom #pragma settings, and once more ignoring the settings. This
allows us to see the output of the workaround technique, alongside the
"default-settings" output which should not contain a workaround.

To implement this, skslc now supports a flag: --[no]settings.
When it's set, /*#pragma settings*/ comments are honored. When it's not
set, skslc ignores the comments. compile_sksl_tests.py passes this flag
along to skslc.

This approach is not strictly limited to workarounds; the
"TypePrecision" GLSL test was also updated to use this technique.

Change-Id: I79204df047b024533ed6bc1f4c088e0e878d5bb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317246
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-17 16:41:18 +00:00
John Stiles
72664bede4 Add support for #pragma settings comments in SkSL.
This will allow us to write skslc-based golden tests that deviate from
the standalone skslc settings.

This CL provides six options; more will be added as necessary.
- Default (caps)
- UsesPrecisionModifiers (caps)
- Version110 (caps)
- Version450Core (caps)
- ForceHighPrecision (settings flag bit)
- Sharpen (settings flag bit)

Change-Id: I9b779e039b2f0c0ccf43dca226fb4844aff3526b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317237
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-16 22:19:58 +00:00
John Stiles
9080540b93 Convert additional GLSL tests to use golden files.
A handful of simplifications were made, but these hew very close to the
original tests and are intended to cover the exact same ground. The
remaining unconverted tests depend on non-default caps bits and will
be updated once caps handling in skslc is fully landed.

Change-Id: I3f3c29bf87f73e501561d7bfcdaabe8acc14b89f
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317390
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-09-16 20:11:17 +00:00
John Stiles
6e49a372de Avoid redundant zeros and ones in swizzle constructor.
Bug: skia:10721
Change-Id: Ib9e51b736bafb6e4493db4f50e9835fbd86c415d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317247
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-16 19:10:37 +00:00
John Stiles
7f6378f7f8 Migrate swizzle tests to golden SkSL files.
This simplifies life when revising the swizzle logic.

Change-Id: I7fc63c0cc845c4741c17c82b3078040264b61ba0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317379
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-16 18:16:07 +00:00
John Stiles
db42bf2140 Align default values of GrShaderCaps with StandaloneShaderCaps.
`fBuiltinFMASupport` is now true on both, and
`fUsesPrecisionModifiers` is now false. Other mismatching flags exist,
but they are non-trivial to synchronize as they are tied to extension
strings.

This will help our skslc-based unit tests generate the same results as
our C++ unit tests did, but should not affect real-world results as
these defaults will all be overwritten in a non-testing scenario.

In practice, the `fUsesPrecisionModifiers` change is responsible for all
of the diffs below. The other flags did not change the results of any of
the currently-ported tests.

Change-Id: Ieb056d852b027fa87c56fd89f971a77a10a8a124
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317204
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-16 15:16:37 +00:00
John Stiles
886b9d477c Create blank SkSL test files for Ninja's benefit.
GN lists both the .cpp and the .h as generated outputs, so if they don't
exist, Ninja assumes we need to rebuild the tests every time we compile.

Change-Id: I37b8b3d9e7aef1b0cb8d5c70530c2542a6c0087a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317108
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-15 16:00:30 +00:00
John Stiles
f2cdf59d58 Migrate SkSL inliner tests to golden files.
Our lack of proper caps-bits controls in skslc affects the outcome of
one test: "InlinerWrapsEarlyReturnsWithDoWhileBlock" does not actually
emit the do-while block because the standalone caps bits don't enable
do-while support. This will be fixed in a followup CL that adds caps-bit
support to our tests.

A few tests were renamed for consistency, a few were simplified slightly
and one test was removed because it was simply redundant (there was a
second test that covered the exact same ground as
`ForWithReturnInsideCannotBeInlined`).

Change-Id: I2e3b97cb3aea331b6d806bdb865aa78c35c7a6b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316997
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2020-09-15 15:22:12 +00:00
John Stiles
c75abb8432 Assign unique names to inlined variable declarations.
Change-Id: I6097f50e7be1fa2f7772f6c454410ecbf3470eea
Bug: skia:10722
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316977
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-09-15 13:22:52 +00:00
John Stiles
8f026259d8 Demonstrate name reuse error in inliner.
The following conditions lead to the error:
- A pair of nested functions, both of which must be inlined.
- Both inlined functions create a variable with the same name.
- The outer function passes its variable to the inner function.
- The initialization of the inner variable uses the value from the outer
  variable.
- The inner function does not mutate the variable, use it as an out-
  parameter, or otherwise cause it to receive a temporary copy.

When all these conditions are met, both variable declarations are
inlined as-is without performing any name salting, because it's
seemingly safe to do so. The name overlap issue is not considered in the
safety checks. Inlined variable declarations are not subject to name
salting but they should be; I suspect other adversarial examples could
be crafted as well where unhandled name overlap leads to errors.

Change-Id: Ia754bee8e45c8a5c7548436594bbf04abc7a8396
Bug: skia:10722
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316945
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-15 13:03:22 +00:00
John Stiles
b0245494c6 Convert several SkSL->GLSL unit tests to golden outputs.
The new test files are intended to be identical to the unit tests in
every meaningful way. (Comments and formatting are not preserved
exactly.) In cases where a unit-test method contained more than one
test, multiple test files were created; in these cases, new names were
invented to match the apparent intent of each invocation.

Followup CLs will continue to migrate additional tests.

Change-Id: I785c6761ba7ee2b25b5ddc0108321734be23b77c
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316678
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-14 22:11:20 +00:00