Add/remove more functions to use shims.

Add/remove more functions to use shims.

Remove normalize and add cos instead to avoid a crash in Mac with ATI cards (angle bug 193, 202).

Also add atan and mod as it's also buggy on Mac/Win with NVIDIA cards.

Also, trying to minimize emulated functions by adding masks for fragment/vertex shaders.

ANGLEBUG=196
Committed: http://code.google.com/p/angleproject/source/detail?r=748

[zmo, 2011-09-09 21:21:58]

Please review.

[zmo, 2011-09-09 21:41:37]

On 2011/09/09 21:21:58, zmo wrote:
> Please review.

Wait, there is a bug.  Let me fix it first.

[dgkoch, 2011-09-09 23:25:43]

http://codereview.appspot.com/4992047/diff/1/src/common/version.h
File src/common/version.h (right):

http://codereview.appspot.com/4992047/diff/1/src/common/version.h#newcode4
src/common/version.h:4: #define BUILD_REVISION 746
748

[zmo, 2011-09-10 01:22:00]

Revised.  This one should work.  Please review.

[kbr1, 2011-09-10 22:10:27]

LGTM

It's scary to see how many driver bugs are being uncovered here. We definitely
need a way in Chrome to turn off these workarounds so that we can see the real
behavior of the underlying drivers, so that GPU vendors have an incentive to fix
their bugs. Please make that a priority. I'm happy to help implement that too.

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
File src/compiler/BuiltInFunctionEmulator.cpp (right):

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
src/compiler/BuiltInFunctionEmulator.cpp:19: "float webgl_atan_emu(float y,
float x) { float rt = atan(y, x); if (rt > 2.0) rt = 0.0;  return rt; }",
Did you try any other tricks not involving conditionals to fool the compiler?
For example, would the following work?

float rt = atan(y, x);
return clamp(rt, 0.0, 2.0);

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
src/compiler/BuiltInFunctionEmulator.cpp:335: // Right now all the emulated
functions with two parameters, the two
Small grammatical correction: all -> for all

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
src/compiler/BuiltInFunctionEmulator.cpp:339: return TFunctionUnknown;
Also check the nominal size and return early if it happens to be > 4. Or ASSERT
that it's <= 4.

http://codereview.appspot.com/4992047/diff/8001/src/compiler/intermediate.h
File src/compiler/intermediate.h (right):

http://codereview.appspot.com/4992047/diff/8001/src/compiler/intermediate.h#n...
src/compiler/intermediate.h:478: bool useEmulatedFunction; // if set to true,
replace the function call by an emulated one.
Could you expand the comment a bit? Perhaps "replace the built-in function call
with an emulated one to work around driver bugs".

[zmo, 2011-09-12 18:22:43]

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
File src/compiler/BuiltInFunctionEmulator.cpp (right):

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
src/compiler/BuiltInFunctionEmulator.cpp:19: "float webgl_atan_emu(float y,
float x) { float rt = atan(y, x); if (rt > 2.0) rt = 0.0;  return rt; }",
On 2011/09/10 22:10:27, kbr1 wrote:
> Did you try any other tricks not involving conditionals to fool the compiler?
> For example, would the following work?
> 
> float rt = atan(y, x);
> return clamp(rt, 0.0, 2.0);

I tried * 1.0 or + 0.0, and they all work.  The reason I choose a conditional
cause that will never be true is that 1) it's harder to be optimized out (by
current or future compilers) 2) hope it's really cheap.

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
src/compiler/BuiltInFunctionEmulator.cpp:335: // Right now all the emulated
functions with two parameters, the two
On 2011/09/10 22:10:27, kbr1 wrote:
> Small grammatical correction: all -> for all

Done.

http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
src/compiler/BuiltInFunctionEmulator.cpp:339: return TFunctionUnknown;
On 2011/09/10 22:10:27, kbr1 wrote:
> Also check the nominal size and return early if it happens to be > 4. Or
ASSERT
> that it's <= 4.

Done.

http://codereview.appspot.com/4992047/diff/8001/src/compiler/intermediate.h
File src/compiler/intermediate.h (right):

http://codereview.appspot.com/4992047/diff/8001/src/compiler/intermediate.h#n...
src/compiler/intermediate.h:478: bool useEmulatedFunction; // if set to true,
replace the function call by an emulated one.
On 2011/09/10 22:10:27, kbr1 wrote:
> Could you expand the comment a bit? Perhaps "replace the built-in function
call
> with an emulated one to work around driver bugs".

Done.

[zmo, 2011-09-12 18:26:55]

What's scarier is, we haven't tested them all yet.  Gregg only added tests for a
subset so far, so after he finished the task, we might have to work around a few
more functions.

[kbr1, 2011-09-12 18:34:31]

On 2011/09/12 18:22:43, zmo wrote:
>
http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
> File src/compiler/BuiltInFunctionEmulator.cpp (right):
> 
>
http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
> src/compiler/BuiltInFunctionEmulator.cpp:19: "float webgl_atan_emu(float y,
> float x) { float rt = atan(y, x); if (rt > 2.0) rt = 0.0;  return rt; }",
> On 2011/09/10 22:10:27, kbr1 wrote:
> > Did you try any other tricks not involving conditionals to fool the
compiler?
> > For example, would the following work?
> > 
> > float rt = atan(y, x);
> > return clamp(rt, 0.0, 2.0);
> 
> I tried * 1.0 or + 0.0, and they all work.  The reason I choose a conditional
> cause that will never be true is that 1) it's harder to be optimized out (by
> current or future compilers) 2) hope it's really cheap.

My experience has been that using conditionals in shaders is not cheap. I would
use the simplest possible expression that works around the problem.

>
http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
> src/compiler/BuiltInFunctionEmulator.cpp:335: // Right now all the emulated
> functions with two parameters, the two
> On 2011/09/10 22:10:27, kbr1 wrote:
> > Small grammatical correction: all -> for all
> 
> Done.
> 
>
http://codereview.appspot.com/4992047/diff/8001/src/compiler/BuiltInFunctionE...
> src/compiler/BuiltInFunctionEmulator.cpp:339: return TFunctionUnknown;
> On 2011/09/10 22:10:27, kbr1 wrote:
> > Also check the nominal size and return early if it happens to be > 4. Or
> ASSERT
> > that it's <= 4.
> 
> Done.
> 
> http://codereview.appspot.com/4992047/diff/8001/src/compiler/intermediate.h
> File src/compiler/intermediate.h (right):
> 
>
http://codereview.appspot.com/4992047/diff/8001/src/compiler/intermediate.h#n...
> src/compiler/intermediate.h:478: bool useEmulatedFunction; // if set to true,
> replace the function call by an emulated one.
> On 2011/09/10 22:10:27, kbr1 wrote:
> > Could you expand the comment a bit? Perhaps "replace the built-in function
> call
> > with an emulated one to work around driver bugs".
> 
> Done.