All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=823856

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=743b1b9299e5160122d2e63e1488c277f510d1894f5f76d5e7e229efa7303eb0


Bot(s) for this bug's original alert(s):

Android Builder

Assigning to angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com because this is the only CL in range:
Roll src/third_party/angle/ aa5d883e0..b391ec408 (1 commit)

https://chromium.googlesource.com/angle/angle.git/+log/aa5d883e0215..b391ec40837c

$ git log aa5d883e0..b391ec408 --date=short --no-merges --format='%ad %ae %s'
2018-03-12 oetuaho Generate code for looking up built-ins

Created with:
  roll-dep src/third_party/angle


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=jmadill@chromium.org

Change-Id: Ie1b40e2112a9b13f9fbaa4285c0be51819fbee03
Reviewed-on: https://chromium-review.googlesource.com/970481
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#544372}

Thanks for finding the right owner. Olli would a 16-bit hash work? I think you can find algorithms online for computing ideal hash functions if you know the set of inputs ahead of time.

I already have some follow-on changes that I will upload to review shortly. One of them checks for cases where there can't be hash collisions and in that case reduces the amount of data used for the name comparisons. That will reduce the code size by several kilobytes, though some of the increase still remains.

I could also try out 16-bit hashes and see if it has a significant effect, but there may be a tradeoff between hash size and the data needed for the name comparisons. I'll do some research for more optimal hash functions if it seems necessary.

I tried 16-bit hashes and it did not yield significant savings. However, I have other changes in review now that will bring down the binary size. See the chain of four patches to address this starting from here:
https://chromium-review.googlesource.com/c/angle/angle/+/964447

The total reduction related to code added in this patch is somewhere around 20 KB.

The patches first clean up the code a bit and then remove redundant comparisons by relying more on the hash value.

The set of possible inputs to the built-in symbol lookup is the whole set of valid identifiers + mangled lists of parameters, so I don't think that building an ideal hash function is feasible.

Some neat optimizations in there! Thanks for spending more time on this.

fwiw - I'm fairly sure that 16 bit hashes would reduce code size, but you'd need to store them in an uint16 array and binary search on it rather than use switch statements (then have a second array of corresponding function pointers). I'd guess this would be a bit slower than switch, though I don't know for sure.

The biggest reduction would obviously be from not needing the strings at all though. Why is the possible input the whole set of valid identifiers? Is this code used by third-party webgl shaders or something along those lines (sorry for my ignorance)?

Yes, the code is used by third-party WebGL shaders that are not guaranteed to be valid.

I admit I didn't make sure that the 16-bit hashes were packed well when I tried it out, but let's do a bit of back-of-the-envelope math. There are around a 1000 hashes in the switch statements. If we make all of them 16-bit and pack them tightly then it's still just 2 KB of savings in total. It only makes a small dent in the 34 KB binary size increase. We get much more benefit from tweaking what information goes into the 32-bit hash.

Maybe there's still some room for making the generated code smaller by having a custom lookup implementation, but optimizing the switch statements for speed or size is a job that I think should be left to the compiler, so I'm a bit reluctant to go and try to replace it with custom code.

Sound reasoning.

Another idea re: strings -
* Looks like there are similar strings in //third_party/glslang. If you take care to make the string literals the same (e.g. separate the mangled part from the non-mangled part), then the linking will deduplicate the identical literals. Even without glslang, looks like there is plenty of repetition in the generated file. 
E.g., would look something like:
const char* const kMyFunc = "textureGather"
const char* const kMyFuncMangled = "0U1B0C"

switch (hash) {
case 0xHASH:
  expected_name = kMyFunc;
  expected_suffix = kMyFuncMangled;
  ret = &BuiltInFunction::kFunction_Foo;
  break;
}
if (NameMatches(expected_name, expected_suffix, acutal_name)) {
  return ret;
}


I know I'm asking for a lot here, but when speed and size are trade-offs, there are really very few features on Android where we'd choose speed over size. I'm hopeful that we can improve both here though.

Another totally different option would be to see if re2c could generate something smaller. From what I remember, you can give it your list of strings, and it will generate a trie search using switch statements. It's super fast (likely faster than hashing), but I don't know how compact.

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/e79d0f86eec849c885a3b3b746a690dd6500649a

commit e79d0f86eec849c885a3b3b746a690dd6500649a
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Mon Mar 26 18:32:17 2018

Rely on hash to check for some mangled name matches

When we are looking up a function with only a few parameters, we can
optimize the lookup by relying on the information encoded in the hash
value. There's often only one list of parameters with the same
function name and mangled name length that results in a matching hash,
so we don't actually need to compare the full mangled name. We can
just compare

1) the hash value of the mangled name
2) the mangled name length
3) the function name

to make sure that the mangled name matches the mangled name of the
function.

This decreases the binary size since we don't need store as many
mangled names of built-in functions. Effect on symbol lookup speed is
marginal.

BUG=angleproject:2267
BUG= chromium:823856 
TEST=angle_unittests

Change-Id: I3ef41d943209509d4e8e6ece14ebad7e2677abc6
Reviewed-on: https://chromium-review.googlesource.com/973242
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

[modify] https://crrev.com/e79d0f86eec849c885a3b3b746a690dd6500649a/src/compiler/translator/SymbolTable_autogen.cpp
[modify] https://crrev.com/e79d0f86eec849c885a3b3b746a690dd6500649a/src/tests/compiler_tests/ImmutableString_test_autogen.cpp
[modify] https://crrev.com/e79d0f86eec849c885a3b3b746a690dd6500649a/src/compiler/translator/gen_builtin_symbols.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c849d31db87f160aed138c694ea614bf6d5f1d9

commit 5c849d31db87f160aed138c694ea614bf6d5f1d9
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Mar 27 03:28:55 2018

Roll src/third_party/angle/ 822a84b19..84fdc62c6 (8 commits)

https://chromium.googlesource.com/angle/angle.git/+log/822a84b1959f..84fdc62c6160

$ git log 822a84b19..84fdc62c6 --date=short --no-merges --format='%ad %ae %s'
2018-03-19 jiawei.shao Fix a compile error when ANGLE_D3D9EX == ANGLE_DISABLED
2017-10-03 geofflang Implement robust resource initialization for OpenGL.
2018-03-20 oetuaho Clean up checkCanBeLValue
2018-01-27 kbr Update stencil validation rules for WebGL
2018-03-19 oetuaho Rely on hash to check for some mangled name matches
2018-03-23 oetuaho Clarify aliasing rules in CHROMIUM_bind_uniform_location
2018-03-26 geofflang Check result of D3D11 map operation in Blit11::copyAndConvert.
2018-03-22 oetuaho Move ReplaceVariable to tree_util directory

Created with:
  roll-dep src/third_party/angle
BUG=chromium:693090, chromium:806557 , chromium:823856 , chromium:825503 


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=lucferron@chromium.org

Change-Id: Ib864a0492b29de58564d5522977c11d6623dad4a
Reviewed-on: https://chromium-review.googlesource.com/981675
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#545968}
[modify] https://crrev.com/5c849d31db87f160aed138c694ea614bf6d5f1d9/DEPS

Reduction from #10/#11 was 3.4kb:
https://chromeperf.appspot.com/report?sid=6e2075e36f226f182a202af67eb08fce78c2b1890c5af1418e839c91d50d24ad&rev=545968

That ANGLE roll included many patches, were you somehow able measure what was the effect of the individual changes? On ANGLE Windows standalone build I could see a roughly ~10 KB reduction with just the individual ANGLE patch, but of course more things might be stripped from the APK build so that the delta is smaller. But other normal ongoing development of ANGLE also tends to make the binary larger.

At least one more patch to reduce the binary size is coming soon.

agrieve re: comment #9, we shouldn't be using glslang on Android, at least not right now. We don't even use ANGLE's API translator at all right now. I think Olli's patches he's making now should help alleviate the problem.

Ran the per-commit tool:

        +2 bytes MonochromePublic.apk_Specifics normalized apk size for range: 26ed93d7db23c2f396bc4de0e7cf8c9db3c2f3c6..822a84b1959f9c1bb615ad4dff2569c58fb3b225
        +1 bytes MonochromePublic.apk_Specifics normalized apk size for range: 822a84b1959f9c1bb615ad4dff2569c58fb3b225..4d54993253d225507c180ea1f2ca750a72516af2
        +0 bytes MonochromePublic.apk_Specifics normalized apk size for range: 4d54993253d225507c180ea1f2ca750a72516af2..a571f28d40342b3e509d1c2a299340b5d451b1a3
        +0 bytes MonochromePublic.apk_Specifics normalized apk size for range: a571f28d40342b3e509d1c2a299340b5d451b1a3..44861c48b97d1dc6883c88dd2c8093aa7e83a07c
        +0 bytes MonochromePublic.apk_Specifics normalized apk size for range: e79d0f86eec849c885a3b3b746a690dd6500649a..b9f9250486abdfa1f00355daf95430838b42b662
        +0 bytes MonochromePublic.apk_Specifics normalized apk size for range: daf120b46a9f155982e5b32b2897ae93bb4929f9..3cd48fff96c13921860bd4f311bfb5370462d047
        +0 bytes MonochromePublic.apk_Specifics normalized apk size for range: 3cd48fff96c13921860bd4f311bfb5370462d047..84fdc62c61609d5b9b8fe7c77ce85522837549c8
        -4 bytes MonochromePublic.apk_Specifics normalized apk size for range: b9f9250486abdfa1f00355daf95430838b42b662..daf120b46a9f155982e5b32b2897ae93bb4929f9
     -7470 bytes MonochromePublic.apk_Specifics normalized apk size for range: 44861c48b97d1dc6883c88dd2c8093aa7e83a07c..e79d0f86eec849c885a3b3b746a690dd6500649a

^^ although they all look like 0s, that's because elf sections are aligned to 4kb.

For the same reason (2kb of the savings of last one is from less alignment overhead) real savings was ~5kb.

    .bss: 0 bytes (0 bytes)
    .data: 0 bytes (0 bytes)
    .data.rel.ro: -3.30kb (-3376 bytes)
    .dex: 0 bytes (0 bytes)
    .other: -2.21kb (-2262 bytes)
    .pak.nontranslated: 0 bytes (0 bytes)
    .pak.translations: 0 bytes (0 bytes)
    .rel.dyn: -424 bytes (-424 bytes)
    .rodata: -5.50kb (-5632 bytes)
    .text: 4.12kb (4224 bytes)

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/3b67874568141f0039d9123d36527b5ac3bad45e

commit 3b67874568141f0039d9123d36527b5ac3bad45e
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Wed Mar 28 18:31:15 2018

Use a specialized hash function for mangled names

The hash values used for looking up built-ins now encode the string
length and the location of parentheses as six-bit values, so that we
don't need to check for these if the hash matches.

This decreases shader_translator binary size on Windows by around 10
KB.

BUG=angleproject:2267
BUG= chromium:823856 
TEST=angle_unittests

Change-Id: If8c28e1c8851750633509ec6273f556e06e91cd1
Reviewed-on: https://chromium-review.googlesource.com/973243
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

[modify] https://crrev.com/3b67874568141f0039d9123d36527b5ac3bad45e/src/compiler/translator/SymbolTable_autogen.cpp
[modify] https://crrev.com/3b67874568141f0039d9123d36527b5ac3bad45e/src/compiler/translator/ImmutableString.h
[modify] https://crrev.com/3b67874568141f0039d9123d36527b5ac3bad45e/src/compiler/translator/ImmutableString.cpp
[modify] https://crrev.com/3b67874568141f0039d9123d36527b5ac3bad45e/src/tests/compiler_tests/ImmutableString_test_autogen.cpp
[modify] https://crrev.com/3b67874568141f0039d9123d36527b5ac3bad45e/src/compiler/translator/gen_builtin_symbols.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71adf7be550c9c348673ed3d1b074da9255d442f

commit 71adf7be550c9c348673ed3d1b074da9255d442f
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Mar 28 23:57:16 2018

Roll src/third_party/angle/ 13e31bb03..8a02d3b57 (4 commits)

https://chromium.googlesource.com/angle/angle.git/+log/13e31bb030ec..8a02d3b57b30

$ git log 13e31bb03..8a02d3b57 --date=short --no-merges --format='%ad %ae %s'
2018-03-27 lucferron Vulkan: Enable depth render state dEQP tests
2018-03-27 fjhenigman Return gl::Error from VertexArray::syncState().
2018-03-21 oetuaho Use a specialized hash function for mangled names
2018-03-26 geofflang Add a FixedVector class to have "variable" size vectors on the stack.

Created with:
  roll-dep src/third_party/angle
BUG= chromium:823856 


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=lucferron@chromium.org

Change-Id: I509a5fce20d9dcb9bbf649d4c4c7ef5f483fc877
Reviewed-on: https://chromium-review.googlesource.com/985146
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#546665}
[modify] https://crrev.com/71adf7be550c9c348673ed3d1b074da9255d442f/DEPS

Result of #17:
    .bss: 0 bytes (0 bytes)
    .data: 0 bytes (0 bytes)
    .data.rel.ro: -896 bytes (-896 bytes)
    .dex: 0 bytes (0 bytes)
    .pak.nontranslated: 0 bytes (0 bytes)
    .pak.translations: 0 bytes (0 bytes)
    .rel.dyn: -205 bytes (-205 bytes)
    .rodata: -1024 bytes (-1024 bytes)
    .text: -4.08kb (-4176 bytes) (83.7%)

So, ~6kb.

There's one more potential mitigation worth a few kilobytes. Then there are some other parts of the code that could be cleaned up, saving some binary size here and there, but nothing very substantial. So looks like some of the increase will remain, but I'd say the tradeoff is still definitely worth it.

Generating code to search a trie using re2c sounds like an interesting alternative, but it would require fairly substantial engineering effort. I'm not planning to pursue that at the moment.

I'm fairly confident that the trade-off is not worth it for low-end Android users, but can certainly see that for anyone wanting the feature, faster is nicer.

Still wonder about putting the hashes in arrays rather than switch statements... The decompiled code was quite hard to follow thanks to the compiler switch statement optimizations, but it was at least clear to me that it was generating multiple instructions per entry, and that it wasn't doing an O(1) lookup on the hashes anyways.

Simplest would be a binary search, but could maybe also store them as a dense hash table. Or maybe have multiple tables indexed by the metadata you've put into the hash.

I certainly can't expect to take up all of your time on this, but might at least file another bug to revisit this later on as a possible binary size improvement area.

The change is not just about being faster, but also reducing runtime memory use. With the change linked in this patch, ANGLE no longer needs to initialize a big hash table and store it in memory at runtime. I'm not too familiar with very low-end Android, but any type of GPU hardware acceleration in Chromium that I'm aware of uses the shader translator which benefits from this.

I can still file a separate ANGLE bug about looking into alternative, even more compact representations of the symbol table, but looking at the bigger picture, there might be other possible changes that could improve both performance and binary size that could be less work to implement.

I think doing this in a packed array with binary search could be good.. FWIW that's how I was planning on implementing this feature originally.

Shouldn't we be able to rely on the C++ compiler to optimize switch statements using a method like binary search or whatever is the most appropriate for the size of the switch statement?

Probably not... there's probably some tradeoff in terms of size/speed that it doesn't know how to pick from.

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/89398b6534e3598337778fe69044e8b1f46099b1

commit 89398b6534e3598337778fe69044e8b1f46099b1
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Tue Apr 03 17:35:57 2018

Avoid mangled name comparisons of 3-parameter functions

The hash values used for looking up built-ins now encode whether the
mangled name contains arrays, structs or interface blocks in its
parameters list. This is written in the most significant bit of the
hash value.

We check this bit at the start of built-in lookup and if the bit is
set we exit early. After that we know that the lookup name doesn't
contain array, struct or interface block parameters.

When we find a hash that matches a hash of a built-in function, we now
know 3 things:
1) the length of the mangled name matches
2) the open parentheses in the mangled name matches
3) the lookup doesn't contain array, struct or block parameters.

Additionally, we have an if statement checking whether the function
name matches. Collisions are only possible with functions that
1) have the same name
2) have the same number of parameters

With these preconditions we can check beforehand whether collisions
are possible for 3-parameter functions. If there are no collisions, we
don't need to compare the full mangled name. This is similar to what
was already being done with functions that had 0 to 2 parameters.

This reduces shader_translator binary size by around 4 KB on Windows.

Besides increased complexity, the tradeoff is that an exhaustive
search of hash values for possible 3-parameter combinations is costly,
so the gen_builtin_functions.py code generation script now takes
around one minute to run on a high-end workstation. Due to this, the
script now exits early if it detects it has already been run with the
same inputs based on a hash value stored in
builtin_symbols_hash_autogen.txt.

BUG=angleproject:2267
BUG= chromium:823856 
TEST=angle_unittests

Change-Id: I3ff8c6eb85b90d3c4971ac8d73ee171a07a7e55f
Reviewed-on: https://chromium-review.googlesource.com/973372
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

[modify] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/src/tests/compiler_tests/ImmutableString_test_autogen.cpp
[modify] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/scripts/run_code_generation.py
[modify] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/src/compiler/translator/gen_builtin_symbols.py
[modify] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/src/compiler/translator/SymbolTable_autogen.cpp
[modify] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/src/compiler/translator/ImmutableString.h
[add] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/src/compiler/translator/builtin_symbols_hash_autogen.txt
[modify] https://crrev.com/89398b6534e3598337778fe69044e8b1f46099b1/src/compiler/translator/ImmutableString.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39c77c86bb59cf2714794797fcff4e8913a26722

commit 39c77c86bb59cf2714794797fcff4e8913a26722
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Apr 04 00:10:01 2018

Roll src/third_party/angle/ 30b604d8d..6bc264aee (4 commits)

https://chromium.googlesource.com/angle/angle.git/+log/30b604d8d174..6bc264aee686

$ git log 30b604d8d..6bc264aee --date=short --no-merges --format='%ad %ae %s'
2018-03-31 jmadill Fix potential bad access in LinkProgram.
2018-03-20 oetuaho Collect static use information during parsing
2018-03-21 oetuaho Avoid mangled name comparisons of 3-parameter functions
2018-04-01 fjhenigman Fix conditions for updating element array buffer.

Created with:
  roll-dep src/third_party/angle
BUG= chromium:827158 , chromium:823856 


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=fjhenigman@chromium.org

Change-Id: Idf3b7b345465da375c6dd6ed0f5a934cedc070e8
Reviewed-on: https://chromium-review.googlesource.com/993002
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#547880}
[modify] https://crrev.com/39c77c86bb59cf2714794797fcff4e8913a26722/DEPS

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/5fec7ab262e4c12e724e56f95a1a938c1412caa4

commit 5fec7ab262e4c12e724e56f95a1a938c1412caa4
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Thu Apr 05 14:14:47 2018

Identify functions by unique id in BuiltInFunctionEmulator

Now that unique ids of all builtins are compile-time constants, we can
use them to look up functions in BuiltInFunctionEmulator. This is
simpler than using a custom struct with the name and parameters for
identifying functions.

This requires that we store a reference to a TFunction in those
TIntermUnary nodes that were created based on a function.

This decreases shader_translator binary size by about 6 KB on Windows.

BUG=angleproject:2267
BUG= chromium:823856 
TEST=angle_unittests

Change-Id: Idd5a00c772c6f26dd36fdbbfbe161d22ab27c2fe
Reviewed-on: https://chromium-review.googlesource.com/995372
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/RewriteUnaryMinusOperatorInt.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/Compiler.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/RemovePow.h
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/IntermNode.h
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_util/BuiltIn_autogen.h
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/BuiltInFunctionEmulator.h
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/tests/compiler_tests/IntermNode_test.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_util/IntermNode_util.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/SymbolUniqueId.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/BuiltInFunctionEmulatorHLSL.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/scripts/run_code_generation.py
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/UnfoldShortCircuitToIf.cpp
[delete] https://crrev.com/f3614370a8e3ba19fac29bf4dde4354f27c8d933/src/compiler/translator/ParamType.h
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/builtin_symbols_hash_autogen.txt
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/RewriteElseBlocks.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/RemovePow.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/SymbolUniqueId.h
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/BuiltInFunctionEmulator.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/gen_builtin_symbols.py
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/RewriteDoWhile.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/gen_emulated_builtin_function_tables.py
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/tree_ops/InitializeVariables.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/IntermNode.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/SymbolTable_autogen.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/emulated_builtin_functions_hlsl_autogen.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler.gypi
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/ParseContext.cpp
[modify] https://crrev.com/5fec7ab262e4c12e724e56f95a1a938c1412caa4/src/compiler/translator/ParseContext.h

I think that's the last big mitigation I'll do - all in all they didn't completely offset the increase, so still marking as WontFix, but the increase is much smaller now.

There are still some smaller cleanups that could be done, but they're not worth a lot of effort.

Thanks again for the added effort!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42033d6aed1cee70a0a57479b0b310c8c07a4428

commit 42033d6aed1cee70a0a57479b0b310c8c07a4428
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 06 07:12:16 2018

Roll src/third_party/angle/ 00af463e0..c3755fc56 (8 commits)

https://chromium.googlesource.com/angle/angle.git/+log/00af463e03ef..c3755fc56611

$ git log 00af463e0..c3755fc56 --date=short --no-merges --format='%ad %ae %s'
2018-04-05 jmadill Vulkan: Move Streaming data to VertexArrayVk.
2018-04-05 jmadill Context: Release surface first in onDestroy.
2018-04-05 jani.hautakangas GLIBC fix: size_t requires include <stddef.h>
2018-04-05 lucferron Vulkan: Bugfix in depth/stencil clearing
2018-04-04 oetuaho Guard traversers used during parsing against stack overflow
2018-04-04 lucferron Vulkan: Implement color mask and depth mask bits
2018-04-04 oetuaho Identify functions by unique id in BuiltInFunctionEmulator
2018-03-31 jmadill Vulkan: Rename StreamingBuffer to DynamicBuffer.

Created with:
  roll-dep src/third_party/angle
BUG= chromium:823856 


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=fjhenigman@chromium.org

Change-Id: I6c11e330653afa936d5ffe006c0d30515b0e6219
Reviewed-on: https://chromium-review.googlesource.com/999005
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#548702}
[modify] https://crrev.com/42033d6aed1cee70a0a57479b0b310c8c07a4428/DEPS