Bug 51506 - [Bisected]Piglit glslparsertest/glsl2/pragma-08.frag regressed
Summary: [Bisected]Piglit glslparsertest/glsl2/pragma-08.frag regressed
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: All Linux (All)
: high major
Assignee: Carl Worth
QA Contact:
URL:
Whiteboard:
Keywords:
: 51802 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-06-28 00:18 UTC by lu hua
Modified: 2012-11-19 08:53 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch to fix problem (2.54 KB, patch)
2012-07-24 14:21 UTC, Fabian Bieler
Details | Splinter Review
patch (6.03 KB, patch)
2012-11-07 00:09 UTC, Matt Turner
Details | Splinter Review

Description lu hua 2012-06-28 00:18:40 UTC
System Environment:
--------------------------
Arch:    i386
Platform:Ironlake
Libdrm:	(master)libdrm-2.4.35-12-gae137f4669ccdbc615d18facebdb804a9af9846b
Mesa:	(master)3bc39414ab960ecd77662e41c8df751c2c9c8984
Xserver:(master)xorg-server-1.12.0-233-gdae317e7265007b38012244722e3b3a06e904ed5
Xf86_video_intel:(master)2.19.0-342-ga072ab506569ecff5b4c57fa90f7a417db69f33b
Libva:	(staging)f12f80371fb534e6bbf248586b3c17c298a31f4e
Libva_intel_driver:(staging)82fa52510a37ab645daaa3bb7091ff5096a20d0b
Kernel:	(drm-intel-next-queued) 3e52259264832dc8c8b94c91561f072ab1d192d8

Bug detailed description:
-----------------------------
It fails on Ironlake, Pineview, Sandybridge and Ivybridge with mesa master branch.It doesn't happen on mesa 8.0 branch.

output:
Failed to compile fragment shader /GFX/Test/Piglit/piglit/tests/glslparsertest/glsl2/pragma-08.frag: 0:17(1): error: syntax error, unexpected $undefined

Shader source:
// [config]
// expect_result: pass
// glsl_version: 1.10
//
// # NOTE: Config section was auto-generated from file
// # NOTE: 'glslparser.tests' at git revision
// # NOTE: 6cc17ae70b70d150aa1751f8e28db7b2a9bd50f0
// [end config]

/* PASS
 *
 * Based on the Regnum Online shader in bugzilla #28138.
 * */
#version 120

#line 336
#extension GL_ARB_texture_rectangle : enable
#pragma optionNV(fastmath on)
#pragma optionNV(fastprecision on)

varying vec4 clrAdd;
varying vec4 clrAddSmooth;
uniform float g_fBloomAddSmoothFactor;

void main()
{
  gl_FragColor.xyz = mix(clrAdd, clrAddSmooth, g_fBloomAddSmoothFactor).xyz;
  gl_FragColor.w = 1.0;
}

PIGLIT: {'result': 'fail' }

Bisect shows:aac78ce8234d96932c38b3f48b1d828077bc0027 is the first bad commit.
commit aac78ce8234d96932c38b3f48b1d828077bc0027
Author:     Carl Worth <cworth@cworth.org>
AuthorDate: Sat Jun 9 16:31:06 2012 -0700
Commit:     Carl Worth <cworth@cworth.org>
CommitDate: Tue Jun 26 15:23:49 2012 -0700

    glsl: glcpp: Move handling of #line directives from lexer to parser.

    The GLSL specification requires that #line directives be interpreted
    after macro expansion. Our existing implementation of #line macros in
    the lexer prevents conformance on this point.

    Moving the handling of #line from the lexer to the parser gives us the
    macro expansion we need. An additional benefit is that the
    preprocessor also now supports comments on the same line as #line
    directives.

    Finally, the preprocessor now emits the (fully-macro-expanded) #line
    directives into the output. This allows the full GLSL compiler to also
    see and interpret these directives so it can also generate correct
    line numbers in error messages.

    Signed-off-by: Carl Worth <cworth@cworth.org>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Reproduce steps:
----------------------------
1. start X
2. ./bin/glslparsertest pragma-08.frag pass 1.10
Comment 1 Ian Romanick 2012-07-03 15:27:55 UTC
I am able to reproduce this.  It looks like something goes wrong after processing the #line directive.  If I comment that line out, the test passes.
Comment 2 Carl Worth 2012-07-09 20:45:20 UTC
(In reply to comment #1)
> I am able to reproduce this.  It looks like something goes wrong after
> processing the #line directive.  If I comment that line out, the test passes.

My bug, sorry.

It looks like the current processing of "#line" directives is
swallowing the trailing newline. So these directives will work if they
are followed by a blank line, but will cause this problem otherwise.

I'll augment our test suite and cook up a fix as soon as I can.

-Carl
Comment 3 Ian Romanick 2012-07-10 18:31:27 UTC
*** Bug 51802 has been marked as a duplicate of this bug. ***
Comment 4 scrawl 2012-07-19 21:59:35 UTC
Is this related to bug 51958 ?
Comment 5 Fabian Bieler 2012-07-24 14:21:35 UTC
Created attachment 64616 [details] [review]
patch to fix problem

the patch also modifies the preprocessor test 091
Comment 6 Kenneth Graunke 2012-07-28 20:39:30 UTC
Since it's been almost three weeks, I've gone ahead and pushed a patch to add \n to the #line directives, since this is actually breaking real games like Regnum Online and Overgrowth.  It's not the right solution, as line numbers in error messages will likely be off by one in certain circumstances, but it gets those games working again.  Carl, feel free to revert when you come up with the proper fix.
Comment 7 lu hua 2012-07-30 05:38:49 UTC
It fixed by commit:dcf8754cce1af09547a5976a74ba807bc6f2657c.

commit dcf8754cce1af09547a5976a74ba807bc6f2657c
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Sat Jul 28 13:04:53 2012 -0700

    glcpp: Add a newline to expanded #line directives.

    Otherwise, the preprocessor happily outputs

        #line 2 4 <your next line of code>

    and the main compiler gets horribly confused and fails to compile.

    This is not the right solution (line numbers in error messages will
    likely be off-by-one in certain circumstances), but until Carl comes
    up with a proper fix, this gets programs running again.
Comment 8 lu hua 2012-07-30 05:39:33 UTC
Verified.It fixed.
Comment 9 Gordon Jin 2012-07-31 01:38:54 UTC
let's keep this open so maybe Carl will come up with a real fix.
Comment 10 Matt Turner 2012-11-07 00:09:09 UTC
Created attachment 69648 [details] [review]
patch

Carl and I came up with this patch to fix glcpp's test 091 and determined that we needed to modify the test in an identical way to Fabian's patch.

I think since Fabian's patch is much simpler, it's what we'll go with. I'll send it to the list.

In any case, this bug is fixed.
Comment 11 lu hua 2012-11-19 08:53:44 UTC
Verified.Fixed.


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.