Reduced skslc memory consumption

Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed: https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
Committed: https://skia.googlesource.com/skia/+/471e89405b71f04e07ae2887bde061185e262c81

[ethannicholas, 2016-10-21 18:12:31]

Description was changed from

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
==========

to

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002
==========

[ethannicholas, 2016-10-21 18:14:19]

The CQ bit was checked by ethannicholas@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 18:14:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 18:15:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 18:15:30]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-GN-Trybot on master.client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-GN-Trybot on
master.client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)
  Build-Mac-Clang-x86_64-Release-GN-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)
  Build-Ubuntu-Clang-arm-Debug-GN_Android-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-a...)
  Build-Ubuntu-Clang-arm64-Debug-GN_Android-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-a...)
  Build-Ubuntu-Clang-arm64-Debug-GN_Android_FrameworkDefs-Trybot on
master.client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-a...)
  Build-Win-MSVC-x86_64-Release-Vulkan-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[ethannicholas, 2016-10-21 19:51:40]

Patchset #2 (id:20001) has been deleted

[ethannicholas, 2016-10-21 20:02:21]

Description was changed from

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002
==========

to

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002
==========

[ethannicholas, 2016-10-21 20:05:59]

ethannicholas@google.com changed reviewers:
+ benjaminwagner@google.com

[ethannicholas, 2016-10-21 20:18:05]

The CQ bit was checked by ethannicholas@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 20:18:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 20:40:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 20:40:26]

Dry run: This issue passed the CQ dry run.

[dogben, 2016-10-25 13:56:50]

lgtm

Sorry, this got lost in my inbox.

LGTM, just nits and nit-picky questions.

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/SkSLContext.h
File src/sksl/SkSLContext.h (right):

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/SkSLContext.h#...
src/sksl/SkSLContext.h:113: , fDVec_Type(new Type("$dvec", { fDVec2_Type.get(),
fDVec2_Type.get(), fDVec3_Type.get(),
Is it allowed/supported to use a $genType (or $gen[DIUB]Type) in the same
declaration as $vec (or $[diub]vec)?

If yes, ISTM that the first coercible type for $[diub]?vec (any others?) should
be the invalid type. Otherwise if I have a function declaration like
$genType foo($vec a, $genType b);
then I will never be able to call it like
foo(vec2(1, 2), vec2(3, 4))
because determineFinalTypes will choose index 0 rather than index 1.

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/ir/SkSLFunctio...
File src/sksl/ir/SkSLFunctionDeclaration.h (right):

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/ir/SkSLFunctio...
src/sksl/ir/SkSLFunctionDeclaration.h:80: if
(arguments[i]->fType.canCoerceTo(*types[j])) {
Is it always the case that a generic type's coercible types are the size-1,
size-2, size-3, and size-4 versions of a scalar type? IWBN to document that
here:
https://skia.googlesource.com/skia/+/0730be7c303ac415484b15ef44ff1dce077a93b8...

If not, ISTM that canCoerceTo could return true even when it's not the type we
really want. Instead, we would need to do something similar to
IRGenerator::determineCallCost.

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/ir/SkSLFunctio...
src/sksl/ir/SkSLFunctionDeclaration.h:88: }
Do we need a range check here? i.e.
  ...
} else {
  if (genericIndex >= types.size()) {
    return false;
  }
}

Also below for fReturnType.coercibleTypes()?

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/sksl.include
File src/sksl/sksl.include (right):

https://codereview.chromium.org/2442063002/diff/30001/src/sksl/sksl.include#n...
src/sksl/sksl.include:25: //$genDType sqrt($genDType x);
nit: Commenting things out seems a little messy.

[ethannicholas, 2016-10-26 19:38:33]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-10-26 19:38:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjaminwagner@google.com
Link to the patchset: https://codereview.chromium.org/2442063002/#ps50001
(title: "fixed nits")

[commit-bot: I haz the power, 2016-10-26 19:38:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 20:01:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 20:01:48]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[ethannicholas, 2016-10-27 13:16:00]

The CQ bit was checked by ethannicholas@google.com

[commit-bot: I haz the power, 2016-10-27 13:16:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 15:15:52]

Description was changed from

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002
==========

to

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed:
https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
==========

[commit-bot: I haz the power, 2016-10-27 15:15:53]

Committed patchset #3 (id:50001) as
https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762

[reed1, 2016-10-27 16:03:20]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2016-10-27 16:03:20]

Is this breaking on the bots (SkSL failing to compile)?

[dogben, 2016-10-27 16:07:17]

On 2016/10/27 at 16:03:20, reed wrote:
> Is this breaking on the bots (SkSL failing to compile)?

Yes. texelFetch was commented out in this CL.

[dogben, 2016-10-27 16:07:52]

A revert of this CL (patchset #3 id:50001) has been created in
https://codereview.chromium.org/2458723002/ by benjaminwagner@google.com.

The reason for reverting is: texelFetch removed, but is used in some shaders..

[ethannicholas, 2016-10-27 18:13:49]

Description was changed from

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed:
https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
==========

to

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed:
https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
==========

[ethannicholas, 2016-10-28 13:32:49]

The CQ bit was checked by ethannicholas@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 13:32:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 13:55:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 13:55:07]

Dry run: This issue passed the CQ dry run.

[ethannicholas, 2016-10-28 15:42:44]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-10-28 15:42:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjaminwagner@google.com
Link to the patchset: https://codereview.chromium.org/2442063002/#ps150001
(title: "added a few more functions")

[commit-bot: I haz the power, 2016-10-28 15:42:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 16:02:48]

Description was changed from

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed:
https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
==========

to

==========
Reduced skslc memory consumption

The big change here is smarter generic type handling which allows us to
keep far fewer entries in the core symboltable. This also comments out
a number of OpenGL builtin functions which Skia does not use and is
unlikely to in the future.
BUG=655673
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2442063002

Committed:
https://skia.googlesource.com/skia/+/cffaa70896fa5bc6c7bf98abbaafb1a755b49762
Committed:
https://skia.googlesource.com/skia/+/471e89405b71f04e07ae2887bde061185e262c81
==========

[commit-bot: I haz the power, 2016-10-28 16:02:50]

Committed patchset #8 (id:150001) as
https://skia.googlesource.com/skia/+/471e89405b71f04e07ae2887bde061185e262c81

[ethannicholas, 2016-11-11 17:35:08]

Patchset #9 (id:170001) has been deleted