Bug 26317

Summary:	glsl compiled wrong
Product:	Mesa	Reporter:	Andre Maasikas <amaasikas>
Component:	Mesa core	Assignee:	mesa-dev
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium
Version:	git
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	vdrift vertex shader mesa program patch to find multiple free temp regs

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.