Bug 26317 - glsl compiled wrong
Summary: glsl compiled wrong
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-29 05:54 UTC by Andre Maasikas
Modified: 2010-02-01 16:36 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
vdrift vertex shader (1.17 KB, text/plain)
2010-01-29 05:56 UTC, Andre Maasikas
Details
mesa program (1.55 KB, text/plain)
2010-01-29 05:58 UTC, Andre Maasikas
Details
patch to find multiple free temp regs (6.56 KB, patch)
2010-01-31 16:46 UTC, Brian Paul
Details | Splinter Review

Description Andre Maasikas 2010-01-29 05:54:28 UTC
vdrift data/shaders/full/vertex.glsl seems to compile wrong,
I'll attach the shader and output.

from this line:
refmapdir = vec3(gl_TextureMatrix[2] * vec4(reflect(viewdir, normal_eye),0));
reflect(viewdir, normal_eye) sequence starts with:

 20: DP3 TEMP[0].w, TEMP[1], TEMP[1];

Seems it's using same register for I and N.

also in the end we output 2 attribs from same temp:
 35: MOV OUTPUT[17], TEMP[3];
 36: MOV OUTPUT[18], TEMP[3];

Andre
Comment 1 Andre Maasikas 2010-01-29 05:56:45 UTC
Created attachment 32897 [details]
vdrift vertex shader
Comment 2 Andre Maasikas 2010-01-29 05:58:29 UTC
Created attachment 32898 [details]
mesa program
Comment 3 Andre Maasikas 2010-01-29 12:19:36 UTC
Looks like all the complex slang* things are right 
and it starts to go wrong anly in _mesa_remove_output_reads,
_mesa_find_free_register gives same temp register there:

Pre-remove output reads:
# Vertex Program/Shader 1
  9: MOV VARYING[1].xyz, TEMP[0];
..
 19: MOV VARYING[2].xyz, TEMP[1];
 20: DP3 TEMP[5].w, VARYING[1], VARYING[2];

..
 34: MOV VARYING[3].xyz, TEMP[1];
 35: END
Post-remove output reads:
# Vertex Program/Shader 1
  9: MOV TEMP[8].xyz, TEMP[0];
..
 19: MOV TEMP[8].xyz, TEMP[1];
 20: DP3 TEMP[5].w, TEMP[8], TEMP[8];
..
 34: MOV VARYING[3].xyz, TEMP[1];
 35: MOV VARYING[1], TEMP[8];
 36: MOV VARYING[2], TEMP[8];
 37: END

_mesa_remove_extra_move_use afterwards does some more stuff
and so it goes..


Comment 4 Brian Paul 2010-01-29 13:29:22 UTC
Which version of Mesa are you using?  This sounds a lot like an issue I fixed with commit 5076a4f53a2f34cc9116b45951037f639885c7a1 which, I believe, went into the Mesa 7.7 release.
Comment 5 Andre Maasikas 2010-01-29 23:05:49 UTC
using git master,
well, now looking some more it seems somewhat similar or might be caused by fix to that bug 

+ /* check dst reg first */
+ if (inst->DstReg.File == regFile) {
+ used[inst->DstReg.Index] = GL_TRUE;
+ }
+ else {
+ /* check src regs otherwise */

This does't check src regs if dest is TEMP register, and while rewriting
in _mesa_remove_output_reads our new temps are not yet used as dest register anywhere. So it returns same temp register it think's it's free?

Andre
Comment 6 Brian Paul 2010-01-31 08:41:13 UTC
Yes, the _mesa_find_free_register() function is returning the same register everytime.  I've got fix nearly ready but it may be a day or two until I finish it...
Comment 7 Brian Paul 2010-01-31 16:46:39 UTC
Created attachment 32956 [details] [review]
patch to find multiple free temp regs

Here's a patch that should fix this problem.
Comment 8 Andre Maasikas 2010-02-01 08:17:12 UTC
seems to work ok with the patch
Comment 9 Brian Paul 2010-02-01 16:36:35 UTC
Fixed on mesa 7.7 branch with commit e0d01c9d7f46ccd531f8dd1a04c5ac067200ef1e

A regression test has been added to glean/glsl1 test.


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.