Results of % are undefined in the GLSL ES spec if one or both operands are negative. ANGLE already warns about this in case negative operands are hit during constant folding, though the warning message could be a bit clearer.

The undefined results are somewhat unfortunate for portability, but I don't think it's worth it to work around this. Developers should be able to expect % to be reasonably fast too.

fair. and I guess even a pre-cast to uint would introduce some extra cost.

suggestion: 
for all these numerous situations of cost vs better portability, 
what about having a flag that let programmer choose between both priority ?
But I don't know how Angle could introduce a GLSL flag. Does the policy allows for special defines like ANGLE_HINT_BEST-PORTABILITY  = true/false ?

It would be possible to create a JS library that would do transformations on shaders to eliminate some undefined results. You could even emscripten-compile ANGLE's shader translator and use that as a basis for it.

I'd be okay with introducing flags to enable "trade performance for better portability" mode into upstream ANGLE as well if someone were to implement that. Don't know what other people on the project think. It would be a bit similar to the floating-point precision emulation functionality that is already implemented in ANGLE, though that's positioned purely as a feature to use for testing portability and certainly not meant to be used in production. That feature also tests for a whole category of common bugs, whereas the other cases of undefined results are quite different from each other and need different kinds of handling.

We could also consider implementing a "test for portability issues" mode that would for example discard fragments in case un-portable calculations are detected in shaders at run time. A developer could run their app in that mode and be alerted about any possible issues that could show up.

Currently I don't think either of these modes is valuable enough that I could personally invest time in them though.

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

commit 67d2cd07edd0dee620e180383ff350c5b7e30320
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Wed May 09 16:26:32 2018

Issue a clearer warning about negative modulus operands

This also adds unit tests for negative modulus operands.

BUG= chromium:839468 
TEST=angle_unittests

Change-Id: I6ab5959ba4f7045d2bde71d246695ef0983c5608
Reviewed-on: https://chromium-review.googlesource.com/1046055
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/67d2cd07edd0dee620e180383ff350c5b7e30320/src/tests/compiler_tests/ConstantFolding_test.cpp
[modify] https://crrev.com/67d2cd07edd0dee620e180383ff350c5b7e30320/src/compiler/translator/IntermNode.cpp

Olli, thanks for getting to the bottom of this.

We should work more on the WebAssembly port of ANGLE's shader translator so that developers can do pre-passes on their shaders. Some sort of "portability over performance" mode could be added then and invoked, similar to your emulation of low-precision devices. I don't think we should be doing either in the browser by default.

fabrice.neyret@: the above change about warning in this case is all we're going to do for this bug. Closing as Fixed.

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

commit 30105dba1ca0c34c367e7b5a5d117c3a42631eb1
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 09 20:27:52 2018

Roll src/third_party/angle/ e95a7f077..ce07f967c (5 commits)

https://chromium.googlesource.com/angle/angle.git/+log/e95a7f077e8b..ce07f967c21e

$ git log e95a7f077..ce07f967c --date=short --no-merges --format='%ad %ae %s'
2018-05-09 geofflang Merge the Framebuffer onDestroy and onDestroyDefault methods.
2018-05-09 geofflang Pass a context pointer to Framebuffer[Impl]::getSamplePosition
2018-05-09 lucferron Vulkan: Clear to opaque black instead of transparent
2018-05-08 lucferron Vulkan: Support EXT_texture_storage
2018-05-07 oetuaho Issue a clearer warning about negative modulus operands

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


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;luci.chromium.try:win_optional_gpu_tests_rel
TBR=fjhenigman@chromium.org

Change-Id: I364e15878f2c36d45e308f81659df42bdc43f354
Reviewed-on: https://chromium-review.googlesource.com/1052680
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@{#557304}
[modify] https://crrev.com/30105dba1ca0c34c367e7b5a5d117c3a42631eb1/DEPS