Bug 34843 - r600g: Evergreen piglit regression
Summary: r600g: Evergreen piglit regression
Status: CLOSED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r600 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-28 11:29 UTC by Rafael Monica
Modified: 2011-03-06 14:40 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
shader output from glsl-routing piglit test (414.54 KB, text/plain)
2011-03-01 17:25 UTC, Rafael Monica
Details
shader output from texrect-many piglit test (134.32 KB, text/plain)
2011-03-01 17:26 UTC, Rafael Monica
Details
Patch to dump the bytes of the fetch shader (1.15 KB, patch)
2011-03-02 13:59 UTC, Christian König
Details | Splinter Review
shader output from glsl-routing piglit test (before offending commit) (405.00 KB, text/plain)
2011-03-02 15:05 UTC, Rafael Monica
Details
shader output from texrect-many piglit test (before offending commit) (132.01 KB, text/plain)
2011-03-02 15:05 UTC, Rafael Monica
Details
shader output from glsl-routing (latest) (414.54 KB, text/plain)
2011-03-02 16:33 UTC, Rafael Monica
Details
shader output from texrect-many (latest) (134.32 KB, text/plain)
2011-03-02 16:33 UTC, Rafael Monica
Details
Possible fix (666 bytes, patch)
2011-03-04 07:03 UTC, Christian König
Details | Splinter Review
Possible fix (666 bytes, patch)
2011-03-04 07:07 UTC, Christian König
Details | Splinter Review
Use long long literal in calculation (564 bytes, patch)
2011-03-06 13:33 UTC, Rafael Monica
Details | Splinter Review

Description Rafael Monica 2011-02-28 11:29:50 UTC
Hi, the commit:

commit 96bbc627f369c0100b950f81531b1fe9ef586c34
Author: Christian König <deathsimple@vodafone.de>
Date:   Mon Feb 28 02:00:01 2011 +0100

    r600g: implement instanced drawing support



breaks the following two piglit tests on Evergreen:

shaders/glsl-routing
texturing/texrect-many


btw. The draw-instanced piglit tests also fail on Evergreen with

EE r600_asm.c:140 r600_bc_get_num_operands - Need instruction operand number for 0x9b.
Comment 2 Rafael Monica 2011-02-28 16:20:33 UTC
With that commit general/draw-instanced now passes clean on Evergreen.

general/draw-instanced-divisor fails with some other errors:

draw-instanced: instance 0 failed to draw correctly
draw-instanced: color instance divisor = 2

And shaders/glsl-routing and texturing/texrect-many are still regressed.
Comment 3 Christian König 2011-03-01 15:03:11 UTC
I don't have an evergreen card at hand to reproduce the problem.

So please run the following command:

R600_DUMP_SHADERS=1 shaders/glsl-routing 2> shaders.out

and attach the generated shader output.
Comment 4 Rafael Monica 2011-03-01 17:25:33 UTC
Created attachment 43993 [details]
shader output from glsl-routing piglit test
Comment 5 Rafael Monica 2011-03-01 17:26:51 UTC
Created attachment 43994 [details]
shader output from texrect-many piglit test
Comment 6 Christian König 2011-03-02 13:59:36 UTC
Created attachment 44035 [details] [review]
Patch to dump the bytes of the fetch shader

Ok please checkout the commit bce4f9ac395986ee0acae2702ed73448333d81b8 (the one before the offending commit) and apply the attached patch.
Then rerun the tests with R600_DUMP_SHADERS=1

Attach the newly generated shader output.

By comparing both we should be able to figure out what's going wrong here.

Christian.
Comment 7 Rafael Monica 2011-03-02 15:05:00 UTC
Created attachment 44037 [details]
shader output from glsl-routing piglit test (before offending commit)
Comment 8 Rafael Monica 2011-03-02 15:05:49 UTC
Created attachment 44038 [details]
shader output from texrect-many piglit test (before offending commit)
Comment 9 Christian König 2011-03-02 15:35:20 UTC
Ok, the mega_fetch_count was wrong, if've fixed this and pushed it to the mainline. Please fetch and try again.

The only problem is that i have no idea why this should affect only evergreen and not my RV710 for example.
Comment 10 Rafael Monica 2011-03-02 16:31:23 UTC
Hi, still not fixed with that commit. I'll attach the new shader output
Comment 11 Rafael Monica 2011-03-02 16:33:12 UTC
Created attachment 44041 [details]
shader output from glsl-routing (latest)
Comment 12 Rafael Monica 2011-03-02 16:33:50 UTC
Created attachment 44042 [details]
shader output from texrect-many (latest)
Comment 13 Christian König 2011-03-04 07:03:20 UTC
Created attachment 44128 [details] [review]
Possible fix

Beside the different formatting the shaders now look completely identical.

So it must be something else, please try the attached patch, if this still doesn't work there must be some other patch that interfere with this problem.
Comment 14 Christian König 2011-03-04 07:07:03 UTC
Created attachment 44129 [details] [review]
Possible fix

Ups, wrong patch, please use this one instead.
Comment 15 Rafael Monica 2011-03-04 19:07:06 UTC
Hi, the patch has no effect here but with latest git master the regression is fixed and draw-instanced-divisor also passes now. I guess it was your commit r600g: fix fragment shader size calculation that fixed it.

Thanks.
Comment 16 Christian König 2011-03-05 05:11:02 UTC
Yes, I finally figured out what was going wrong here. The alignment of the vertex fetch instructions weren't included in the shader size calculation.

Thanks for the help,
Christian.
Comment 17 Rafael Monica 2011-03-06 13:33:58 UTC
Created attachment 44182 [details] [review]
Use long long literal in calculation

Didn't feel like opening a new bug fur this. But your recent commit, r600g: simplify instance addr calculation, breaks the draw-instanced-divisor piglit test here. I also noticed a warning about overflow during compilation: 

r600_asm.c:2105: warning: left shift count >= width of type

Using a long long integer fixes the warning and the piglit test. Patch attached.
Comment 18 Christian König 2011-03-06 14:40:06 UTC
Thanks again, patch pushed.


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.