Commit Graph

52 Commits

Author SHA1 Message Date
John Stiles
f10eff363b Fix misdetection of dead global variables.
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>
2021-03-16 16:02:03 +00:00
Brian Osman
8c595fed7c Change sksl tests to avoid SPIR-V validation errors
'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>
2021-03-16 13:39:53 +00:00
Brian Osman
fc4b9919d3 Force global initializers to be constant expressions
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>
2021-03-15 16:54:05 +00:00
John Stiles
f3a28db703 Eliminate control-flow analysis.
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>
2021-03-11 13:24:54 +00:00
John Stiles
51d33982ad Add Convert/Make factory functions to IndexExpression.
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>
2021-03-08 15:55:59 +00:00
John Stiles
6fb520fcd5 Improve optimization of prefix expressions at IR time.
- Values from constant variables are folded in when it helps, e.g.:

    const bool SHINY = true;
    const float SHININESS = 2;
    if (!SHINY) {  // <-- optimizes directly to `false`
       param = -SHININESS; // <-- optimizes to `-2`

- Doubled-up logical-not and negation are stripped:

    y = -(-x); // <-- optimizes to `y = x;`
    b = !!a; // <-- optimizes to `b = a;`

Removal of doubled-up negation and logical-not was actually never
implemented in the constant-propagation phase; I just noticed it while
I was here and thinking about it.

Change-Id: Ie28bb9b5af91376f03d926e26e37f4a131bbf550
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379298
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-05 21:41:55 +00:00
John Stiles
28054added Optimize ternary tests that check a const variable.
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>
2021-03-05 21:41:05 +00:00
John Stiles
b3dcbb12ef Detect functions that fail to return a value, without using CFG.
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>
2021-03-04 22:47:05 +00:00
John Stiles
2b6ec98a82 Disallow unscoped for blocks which declare a variable.
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>
2021-03-04 17:04:40 +00:00
John Stiles
4633c9149b # Enter a description of the change.
Reland "Migrate if-statement simplifyStatement logic to IfStatement::Make."

This reverts commit 7e685f0377.

Reason for revert: fixed SkSLBench perf test

Original change's description:
> Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
>
> 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>

Bug: skia:11343
Bug: skia:11319
Change-Id: I74cc3295004133e9fdcf16e388106eb83603f526
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379063
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 22:21:36 +00:00
John Stiles
7e685f0377 Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
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>
2021-03-03 20:50:37 +00:00
John Stiles
e4da7b672f 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>
2021-03-03 18:12:35 +00:00
Brian Osman
7b361499c9 Align SkSL const rules more closely with GLSL
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>
2021-02-26 17:44:11 +00:00
John Stiles
bb8542f086 Fix fuzzer-discovered error with ~ prefix on literals.
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>
2021-02-26 17:30:26 +00:00
John Stiles
642cde289d Optimize @switch statements in SwitchStatement::Make.
At IR generation time, this CL limits our optimizations to only
@switch statements. A regular switch statement will only be optimized
during the optimization phase even if the switch-value is a known
compile-time constant. This is done to avoid upsetting our reachability
analysis.

Most of this CL is moving existing logic from SkSLCompiler into
SkSLAnalysis and SkSLSwitchStatement. Although the diffs look large, the
actual changes are very small.

Change-Id: I90920f41bc386dfa7a980ae7510f6681231a5120
Bug: skia:11340, skia:11342, skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372679
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-23 21:36:04 +00:00
John Stiles
04ca41acf3 Fix switch optimization pass.
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>
2021-02-23 15:38:24 +00:00
John Stiles
ec24154521 Fix SPIR-V bug with swizzled out parameters.
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>
2021-02-12 15:21:27 +00:00
Brian Osman
8e756f379c Support structs in runtime effects
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>
2021-02-11 21:09:15 +00:00
John Stiles
a0e407d4b7 Add SkSL test for matrix equality/inequality operators.
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>
2021-02-10 18:57:22 +00:00
John Stiles
0bd5578c31 Replace Metal constant keyword with const.
"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>
2021-02-10 15:23:25 +00:00
Greg Daniel
4908a24d4b 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>
2021-02-10 14:25:54 +00:00
John Stiles
c501857188 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>
2021-02-09 22:53:18 +00:00
John Stiles
ecd7c2278f Migrate the last SkSL tests in shared/ to dm.
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>
2021-02-08 21:37:48 +00:00
John Stiles
c2c1b0c460 Migrate some SkSL swizzle tests to dm.
Change-Id: I2bd6bf9c368359a2b21861c1b6f621040d335111
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367056
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-08 16:16:28 +00:00
John Stiles
abc3b78011 Eliminate left-side comma expressions without side effects.
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>
2021-02-05 16:50:33 +00:00
John Stiles
d437478348 Migrate even more SkSL tests to run in dm.
- ResizeMatrix
- ScalarConversionConstructorsES2
- StackingVectorCasts
- StaticIf
- SwizzleBoolConstants

Non-ES2 compatible tests:

- ResizeMatrixNonsquare
- ScalarConversionConstructorsES3

Newly discovered bugs:
- skia:11278 SPIR-V does not support casting non-square matrices

Change-Id: I7feb78f0380d0a9cb328ec61f96a364ad1281432
Bug: skia:11009, skia:11278
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366408
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-05 16:04:43 +00:00
John Stiles
b88b4e1a12 Migrate additional SkSL tests to run in dm.
- 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>
2021-02-04 22:12:27 +00:00
John Stiles
b44185086e Reland "Improve support for arrays in Metal."
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>
2021-02-04 17:33:28 +00:00
John Stiles
16a97bb2cb Re-enable SkSL Hex test with constants that fit in 16 bits.
Larger constants caused failures on Adreno 330 (Nexus 5).

Change-Id: I1af1df786f8e4d34f8ec47152875927bfe948190
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366336
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-04 17:15:08 +00:00
John Stiles
38df4c8470 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>
2021-02-04 00:50:34 +00:00
John Stiles
ecd2036eae Migrate more SkSL tests to run in dm.
- CastsRoundTowardZero
- Functions
- GeometricIntrinsics
- HelloWorld
- Hex

Other test updates:
- DeadLoopVariable (for-loop structure incompatible with ES2)
- FunctionPrototype (RTE currently disallows calling undef'd functions)
- HexUnsigned (unsigned types incompatible with ES2)

Change-Id: I794bee24dfac55281c9b8b130529888ba2546290
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364678
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-03 23:45:15 +00:00
John Stiles
dd904af566 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>
2021-02-03 22:33:19 +00:00
John Stiles
248f57b39b Reject array constructors with the wrong number of elements.
We now catch this error at IR generation time; previously we'd send it
to the driver (where it would fail to compile).

Change-Id: I45890214ffa164be1c0f359320f942bc4dc479ca
Bug: skia:11265
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365697
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-03 22:33:02 +00:00
John Stiles
56233d1379 Migrate matrix SkSL test to dm.
This uncovered a bug in Metal code generation of `matX *= matY` which is
now fixed. (It was emitting the helper function more than once.)

Change-Id: I0aeb0efe7ab5fbf5592a8ca6f4f5b50354d3d7f4
Bug: skia:11262
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365489
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>
2021-02-03 20:42:57 +00:00
John Stiles
5a956ce54b Add more dm tests for the comma operator.
Change-Id: I37a95fd95c23a91c3510f922bab0569093e612a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364604
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-03 01:28:48 +00:00
John Stiles
d842feea48 Avoid comma usage in Operators test.
This works around a GLSL compilation bug on the Tecno Spark 3 Pro.

Change-Id: I516bd64745a8e99cccc87ee4bb2e1f5d5b26c130
Bug: skia:11255
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364116
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-02-02 14:34:24 +00:00
John Stiles
931cad1d74 Update function-type-matching tests to work as Runtime Effects.
Previously, our only test was invoking `sin(1)` which is a pretty
ineffective test. Now, we test args and return types for all the basic
scalars/vectors/matrices.

Change-Id: I7d335303eef8b9c9c6cfef2265a15bbd9bd73e0c
Bug: skia:11246
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363943
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-02 13:47:35 +00:00
John Stiles
c068a8f00b Update various shared/ tests to run in dm.
These tests have updated to return green on success, or red on failure.
Some tests were modified slightly to conform to ES2 limitations, or
split into separate ES2 and ES3 parts.

Change-Id: Ib47aeca217aef33f3c4b5999d93afed5d42a1e62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363876
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-01 22:06:59 +00:00
Brian Osman
21d2b6a651 Use unsigned values when constant-folding integer vectors
We already had this trick for scalar integers, this extends it to
integer vectors. As with prior work in this area, it would be better to
detect this case and produce an error, but now we at least produce
consistent and well-defined results (rather than undefined signed
integer overflow).

Bug: skia:10932
Bug: oss-fuzz:29494
Change-Id: I45526fe96b6ea42c0e88b9862f6961b316810321
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363962
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-01 21:44:28 +00:00
John Stiles
2ff8f86205 Run the SkSL Operators test as a Runtime Effect.
The test has been split into an ES2 version and ES3 version; the ES2
side omits unsupported integer ops like << >> & | ^ %.

Change-Id: Iba16d469a477809b17a823b1c68ae8937624c68e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362616
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-01 17:21:33 +00:00
John Stiles
94e72b90b8 Add test to verify side effects from the comma operator.
SkVMCodeGenerator was lacking support for the comma operator entirely;
this has now been implemented.

Change-Id: I9350f54e6ee52764c620116e6dbfe4ca3e9cd47e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363096
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-01 16:16:58 +00:00
John Stiles
da5cdf6a70 Add default uniform binding value for Metal/SPIR-V.
This allows interface blocks in Metal to compile even if
`layout(binding=...)` is not specified. It will also be used in SPIR-V
in the followup CL, when an interface block is automatically synthesized
for top-level uniforms.

This CL also reorganizes the unit tests around uniforms a bit.

Change-Id: Ia898c536b454dda6f51677e232a8f6e6c3606022
Bug: skia:11225
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360778
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-28 19:23:49 +00:00
Brian Osman
bf7b4b8890 Add SkSL test for functions that operate on structs
This is a known deficiency of runtime effects, next step is to fix how
they manage function signatures to solve the problem.

Bug: skia:10939
Change-Id: Id934e0acdf774b03bd6edce78d7b2c077bdeae00
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360603
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-28 17:01:59 +00:00
John Stiles
7b92897d62 Reland "Add SkSL for-loop control flow test to dm."
This reverts commit 578f1acbe8.

Reason for revert: updated test to pass on Mac Intel 5100/6000

Original change's description:
> Revert "Add SkSL for-loop control flow test to dm."
>
> This reverts commit a0c266283a.
>
> Reason for revert: failing on Mac Intel
>
> Original change's description:
> > Add SkSL for-loop control flow test to dm.
> >
> > While loops and do-while loops remain untested in dm, as they are not
> > supported in ES2 (and therefore not available in Runtime Effects).
> >
> > Change-Id: I2f1bfccccd571cc4ced096bc18ebbb9ecc9f9b4a
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359556
> > 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>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I45335d16a695644eaeb8a535298c0efcc616c1ce
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359840
> 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: I2dc6e870393708a12286658001b723f25a6aec4a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359856
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-27 15:46:39 +00:00
John Stiles
4d6310ab20 Support half4 return values from main() in the SPIR-V code generator.
This creates a helper function, _entrypoint, which invokes main() and
assigns its result into sk_FragColor. We also make sure to prevent
sk_FragColor from being dead-stripped from the code during IR
generation.

At present this is useful for allowing our SkSL test shaders to compile.

Change-Id: I2d7fab0e1959a77778ffdb18ca569e869bcaeece
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358525
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-27 02:46:03 +00:00
John Stiles
b41d5bb3a7 Add helper uniforms to Runtime Effect tests.
This lets us use descriptive names like `colorRed` and `colorGreen`
instead of `half4(1,0,0,1)` and `half4(0,1,0,1)`. It also lets us use
actual unknown values instead of synthesizing sorta-kinda-unknowns by
calling sqrt.

Change-Id: I61481c33b7ff42182955777b05cfa5fcc13e0efc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359567
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-27 00:53:46 +00:00
John Stiles
d8fc95dc02 Add default uniform set value to SkSL Program settings.
This allows uniforms to be specified without an explicit `layout(set=N)`
modifier. They will assume a default set value instead.

This turns out to fix a handful of tests in Metal/SPIR-V which were
written with GLSL in mind, or adapted from real generated GLSL code, and
didn't have layout information specified on their uniforms. It will also
make it easier to write SkSL tests using uniforms that can compile
either as a runtime effect or as plain Metal/SPIR-V code.

Change-Id: Id79ec06f278b913a45c09c2e6211195dc98b42c0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359838
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-01-27 00:52:16 +00:00
John Stiles
0ecfdea199 Add SkSL unit test for unbraced blocks that optimize away.
In most cases, this works properly and a `;` is emitted, but in one
particular case (int x, y;) we get nothing.

Change-Id: If88d92502f6a533284dd4e0f78daedaf1481ff3d
Bug: skia:11218
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359558
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>
2021-01-26 19:06:41 +00:00
John Stiles
dac139abd9 Add unit test for control statements in for loops.
Change-Id: I94094be7163a04bf48e86406230156a5433469b6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359140
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>
2021-01-26 17:07:51 +00:00
John Stiles
7add07f88d Add unit test for control statements in while loops.
Change-Id: I924ac75b5f8a397f7af7a06925ef0c9deba5c509
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359141
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>
2021-01-26 17:07:36 +00:00