Bug 49713 - piglit glsl-const-folding-01 regression
Summary: piglit glsl-const-folding-01 regression
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Ian Romanick
QA Contact:
URL:
Whiteboard:
Keywords: bisected, regression
Depends on:
Blocks:
 
Reported: 2012-05-09 19:28 UTC by Vinson Lee
Modified: 2016-07-09 00:10 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
glsl: Improve the accuracy of the asin() builtin function. (4.30 KB, patch)
2013-04-23 17:21 UTC, Fabian Bieler
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Vinson Lee 2012-05-09 19:28:16 UTC
mesa: 788fd04dacb9eb1e32010050c57cd2f49779311b (master)

$ ./bin/shader_runner tests/shaders/glsl-const-folding-01.shader_test -auto
Mesa warning: failed to remap index 173
Failed to link:
error: unresolved reference to function `bad_constant_folding'

PIGLIT: {'result': 'fail' }
Comment 1 Vinson Lee 2012-05-09 19:43:04 UTC
363c14ae0cd2baa624d85e8c9db12cd1677190ea is the first bad commit
commit 363c14ae0cd2baa624d85e8c9db12cd1677190ea
Author: Olivier Galibert <galibert@pobox.com>
Date:   Wed May 2 23:11:42 2012 +0200

    glsl: Change built-in constant expression evaluation to run the IR.
    
    This removes code duplication with
    ir_expression::constant_expression_value and builtins/ir/*.
    
    Signed-off-by: Olivier Galibert <galibert@pobox.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

:040000 040000 b5544fbb108c323c6f87b67cf2ed88b52c80acd3 554bf308d67557ed0ac39762d1b4e9f59b28e032 M	src
bisect run success
Comment 2 Kenneth Graunke 2012-05-10 00:32:09 UTC
Yeah, this is due to a slight difference in precision.  We concluded that either:

1. Our built-ins need to become more precise, or
2. The test needs to be made less stringent.

I'd prefer the former, and it's probably not actually very hard to do...
Comment 3 lu hua 2012-05-10 02:06:14 UTC
Following cases fail on Ironlake, Sandybridge and Ivybridge with mesa master branch, and have same bisect commit.
Piglit 
shaders_glsl-const-folding-01
Oglc
glsl-constexpr(positive.asin)
glsl-constexpr(positive.acos)
glsl-constexpr(positive.atan)
glsl-constexpr(positive.atan_y_over_x)
glsl-autointconv(function.builtIn.trigonometry)

Oglc cases 'glsl-constexpr(positive.tanh)' and 'glsl-bif-trig(advanced.tanh.vec2)' fail on Sandybridge and Ivybridge with mesa master branch, and have same bisect commit.
Comment 4 Olivier Galibert 2012-05-10 21:42:25 UTC
Piglit patch for atan(1):
  http://lists.freedesktop.org/archives/piglit/2012-May/002342.html

Mesa patch for acos(1):
  http://lists.freedesktop.org/archives/mesa-dev/2012-May/021586.html

The test passes once those two are in.

These were sent before the patch series second version.

  OG.
Comment 5 Pavel Ondračka 2012-06-12 11:29:13 UTC
The glsl-const-builtin-normalize piglit failure with r300g driver is also caused by the aforementioned commit, it looks like some precision problem too. Should I open a new bug for that?

./bin/shader_runner tests/shaders/glsl-const-builtin-normalize.shader_test -auto -fbo
r300: DRM version: 2.15.0, Name: ATI RV530, ID: 0x71c5, GB: 1, Z: 2
r300: GART size: 509 MB, VRAM size: 256 MB
r300: AA compression RAM: YES, Z compression RAM: YES, HiZ RAM: YES
Probe at (15,15)
  Expected: 0.707107 0.000000 0.000000
  Observed: 0.705882 1.000000 1.000000
PIGLIT: {'result': 'fail' }
Comment 6 Olivier Galibert 2012-06-12 12:06:01 UTC
(In reply to comment #5)
> The glsl-const-builtin-normalize piglit failure with r300g driver is also
> caused by the aforementioned commit, it looks like some precision problem too.
> Should I open a new bug for that?
> 
> ./bin/shader_runner tests/shaders/glsl-const-builtin-normalize.shader_test
> -auto -fbo
> r300: DRM version: 2.15.0, Name: ATI RV530, ID: 0x71c5, GB: 1, Z: 2
> r300: GART size: 509 MB, VRAM size: 256 MB
> r300: AA compression RAM: YES, Z compression RAM: YES, HiZ RAM: YES
> Probe at (15,15)
>   Expected: 0.707107 0.000000 0.000000
>   Observed: 0.705882 1.000000 1.000000
> PIGLIT: {'result': 'fail' }

Well, the constant folding gives:
      (assign  (xyzw) (var_ref gl_FragColor)  (constant vec4 (0.707107 -nan -nan 1.000000)) ) 

The initial 0.707107 is sqrt(2), i.e. the result of normalizing (1, 1).  Then it gets multiplied by 255, truncated, then re-divided by 255, giving 0.705882.  So that slot is good.

The other two are the result of normalizing (0, 0), which is undefined (the test itself says so), because it does a pair of 0/0.  I'm not convinced pessimizing normalize just to return (0, 0) in that case is worth it.
Comment 7 lu hua 2012-09-04 03:13:15 UTC
It also happens on mesa 9.0 branch(commit:6886da783ac2fc549b4ffc1f4)
Comment 8 Vinson Lee 2013-01-26 09:20:00 UTC
mesa: 1a316af0343b1c1b345d6209a687ce858b47c438 (master)

glsl-const-folding-01 regression is still present.
Comment 9 Fabian Bieler 2013-04-23 17:21:41 UTC
Created attachment 78390 [details] [review]
glsl: Improve the accuracy of the asin() builtin function.

I extended the Series by two degrees to 

asin(x) = sign(x) * asin_pos(abs(x))

asin_pos(x) = pi/2 - sqrt(1 - x) * T5(x)

T5(x) = (a0 + x * (a1 + x * (a2 + x * (a3 + x * (a4 + x * a5)))))

a0 = pi/2
a1 = (pi/4 - 1)
a2, a3, a4 and a5 found by numerical fitting.

This approximation has a worst case absolute error <4e-6. Unfortunately, due to round off errors of single precision floats the relative error is of the order of 100% near 0 (for |x|<1e-3). Otherwise the relative error is <2e-5.

The current implementation has similar problems with round of errors near 0. A possible way to fix this would be a piecewise linear approximation near 0.
Comment 10 Vinson Lee 2014-03-02 03:42:14 UTC
mesa: fc25956badb8e1932cc19d8c97b4be16e92dfc65 (master 10.2.0-devel)

glsl-const-folding-01 regression is still present.
Comment 11 David Heidelberg (okias) 2014-05-19 02:00:37 UTC
Mesa 10.3.0-devel (git-5b8f1a0), r600g

Failed to link:
error: unresolved reference to function `bad_constant_folding'
Comment 12 Vinson Lee 2014-09-12 22:43:38 UTC
mesa: d13d2fd16132f351ec7c8184f165faeac3b31bb4 (master 10.4.0-devel)

Regression is still present.
Comment 13 Timothy Arceri 2014-10-10 22:11:11 UTC
Should be fixed by 326e303175ab700d59602f0e344e8d84e15f1aa8


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.