We now have two functions `writeOpLoad` and `writeOpStore` which are
in charge of writing SpvOpLoad and SpvOpStore instructions.
`writeOpStore` also keeps track of pointer stores in a "store cache."
Subsequent loads from that same pointer will be found in the cache and
will return the value stored in that pointer instead.
Such a cache definitely cannot work in the face of control flow, so we
make the following concessions:
- `pruneReachableOps` is now `pruneConditionalOps`. Any pointers that
are altered inside a potentially-unreachable block are cleared from
the cache entirely.
- The entire store cache is cleared at all OpLabels within a loop.
The cache also cannot work in the presence of swizzled stores, so we
make another significant concession:
- The entire store cache is cleared whenever we store into a non-memory
pointer (e.g., assigning into a swizzled LValue, such as `foo.xz`).
Despite these significant limitations, this manages to dramatically
shrink many real-world examples.
Change-Id: I0981a0cf7b45b064e153e9ada271494c8e00cad5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/530054
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The output changes here are almost entirely a wash, because we already
had support for caching scalars and vectors. Almost all changes are just
inconsequential reorderings of IDs, and the removal of RelaxedPrecision
decorators on constants (which were not meaningful).
Change-Id: I45340c4a240cb504b7c4a934b3db178d2f39ec99
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528709
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This was mistakenly using dFdx in some portions (copy-paste error).
Change-Id: Ifb159b3c44185d9166c10725b24002a28a0895b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464381
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>
When compiling test shaders, we were setting SK_FRAGCOORD_BUILTIN on the
`coords` parameter to main() instead of SK_MAIN_COORDS_BUILTIN. These
two built-ins don't have the same type (float2 vs. float4) and don't
mean quite the same thing.
The SPIR-V code generator saw a variable with the SK_FRAGCOORD_BUILTIN
builtin value and assumed the presence of a global variable named
`sk_FragCoord`, which didn't exist (because it was never referenced in
the code, so it was never cloned in from the sksl_frag module).
This is only a concern when compiling test shaders with skslc; real
shaders don't hit these code paths. The generated code here is still
imperfect; if you look closely, you'll see the GLSL and Metal code is
referencing the `coords` variable but it's never declared anywhere.
Change-Id: I3ad249469927ff35eb1e75d6536f95317502708f
Bug: skia:12340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440520
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>
We can now add functions to sksl_public.sksl with an $es3 prefix. These
will be allowed in a Runtime Effect when strict-ES2 mode is disabled.
Note that the CPU backend still doesn't have support for these calls,
and will fail ungracefully (assertion, nonsense result) if these
intrinsics are used.
The testing here is limited, due to an unrelated bug in SPIR-V
(skia:12340)
Change-Id: I9c911bc2b77f5051e80844607e7fd08ad386ee56
Bug: skia:12202, skia:12340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439058
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The value of 32 was causing errors when other uniforms were present, as
the SPIR-V code generator would detect overlapping uniform offsets and
fail.
Change-Id: I7bb1cf1244e54c39596c3a39e9f6972c6a47899c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439059
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reland works around Adreno issue with this formulation of sk_Clockwise:
(sk_RTFlip.y < 0.0 ? !gl_FrontFacing : gl_FrontFacing)
and instead adds this to the top of the function:
bool sk_Clockwise = gl_FrontFacing;
if (sk_RTFlip.y < 0.0) {
sk_Clockwise = !sk_Clockwise;
}
Original description:
SkSL language features that are origin sensitive now use a uniform
to conditionally flip their result rather than generating different
code.
Previously we would insert a "rt height" uniform if sk_FragCoord needed
to be flipped. sk_FragCoord,y was implemented as "realFragCoord.y" or
"rtHeight - realFragCoord.y" depending on SkSL::ProgramSettings::fFlipY.
Now we instead use a two component vector rtFlip and sk_FragCoord.y is
always "rtFlip.x + rtFlip.y*realFragCoord.y". We configure rtFlip as
either (0, 1) or (rtHeight, -1). sk_Clockwise and dFdy simiarly use
rtFlip.y to emit code that always works with either origin.
Bug: skia:12037
Change-Id: I3a2ad6f5667eb4dcd823b939abd5698f89b58929
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425178
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit 943108b0b2.
Reason for revert: clockwise GM bad on android.
Original change's description:
> Don't key progams/pipelines on origin.
>
> SkSL language features that are origin sensitive now use a uniform
> to conditionally flip their result rather than generating different
> code.
>
> Previously we would insert a "rt height" uniform if sk_FragCoord needed
> to be flipped. sk_FragCoord,y was implemented as "realFragCoord.y" or
> "rtHeight - realFragCoord.y" depending on SkSL::ProgramSettings::fFlipY.
>
> Now we instead use a two component vector rtFlip and sk_FragCoord.y is
> always "rtFlip.x + rtFlip.y*realFragCoord.y". We configure rtFlip as
> either (0, 1) or (rtHeight, -1). sk_Clockwise and dFdy simiarly use
> rtFlip.y to emit code that always works with either origin.
>
> Bug: skia:12037
>
> Change-Id: I7a09d0caac60a58d72b76645ff31bcabde4086b6
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414796
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,ethannicholas@google.com
Change-Id: I91cc0d86be216f6c32e453a231de088c991be4b2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425056
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
SkSL language features that are origin sensitive now use a uniform
to conditionally flip their result rather than generating different
code.
Previously we would insert a "rt height" uniform if sk_FragCoord needed
to be flipped. sk_FragCoord,y was implemented as "realFragCoord.y" or
"rtHeight - realFragCoord.y" depending on SkSL::ProgramSettings::fFlipY.
Now we instead use a two component vector rtFlip and sk_FragCoord.y is
always "rtFlip.x + rtFlip.y*realFragCoord.y". We configure rtFlip as
either (0, 1) or (rtHeight, -1). sk_Clockwise and dFdy simiarly use
rtFlip.y to emit code that always works with either origin.
Bug: skia:12037
Change-Id: I7a09d0caac60a58d72b76645ff31bcabde4086b6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414796
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@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>
'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>
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 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>
Bug: skia:11738
Change-Id: I1dd5e99830f70d72c292379a45c4e39a55588858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383706
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Kevin Lubick <kjlubick@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>
