Bug 51631 - piglit if-arg-must-be-defined-01 regression
Summary: piglit if-arg-must-be-defined-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: Carl Worth
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-01 15:28 UTC by Vinson Lee
Modified: 2013-03-01 22:13 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Proposed fix for piglit (3.53 KB, text/plain)
2013-02-22 21:29 UTC, Carl Worth
Details

Description Vinson Lee 2012-07-01 15:28:54 UTC
mesa: 1d21bd057a6b1701dd44a79e82259c0f3ded2b70

Run piglit if-arg-must-be-defined-01 on softpipe.

$ ./bin/glslparsertest tests/spec/glsl-1.30/preprocessor/if/if-arg-must-be-defined-01.frag fail 1.30
Successfully compiled fragment shader tests/spec/glsl-1.30/preprocessor/if/if-arg-must-be-defined-01.frag: 
Shader source:
// [config]
// expect_result: fail
// glsl_version: 1.30
// [end config]
//
// Check that the compiler raises an error when an undefined macro is used as
// an argument to the #if directive.
//
// From page 11 (17 of pdf) of the GLSL 1.30 spec:
//     "It is an error to use #if or #elif on expressions containing
//     undefined macro names, other than as arguments to the
//     defined operator."

#version 130

#if UNDEFINED_MACRO
#	define SHOULD_NOT_REACH_HERE 1
#endif

int f()
{
	return 0;
}

PIGLIT: {'result': 'fail' }


c96b8302a398a6db27f1bb6070cdc088c7ee0fba is the first bad commit
commit c96b8302a398a6db27f1bb6070cdc088c7ee0fba
Author: Carl Worth <cworth@cworth.org>
Date:   Fri Jun 8 15:00:49 2012 -0700

    glsl: glcpp: Allow "#if undefined-macro' to evaluate to false.
    
    A strict reading of the GLSL specification would have this be an
    error, but we've received reports from users who expect the
    preprocessor to interepret undefined macros as 0. This is the standard
    behavior of the rpeprocessor for C, and according to these user
    reports is also the behavior of other OpenGL implementations.
    
    So here's one of those cases where we can make our users happier by
    ignoring the specification. And it's hard to imagine users who really,
    really want to see an error for this case.
    
    The two affected tests cases are updated to reflect the new behavior.
    
    Signed-off-by: Carl Worth <cworth@cworth.org>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

:040000 040000 473886a232991765329410db71aff081dbc7129c b4960ffd2dbdc2f05ad5847b6084b90393c61f40 M	src
bisect run success
Comment 1 Ian Romanick 2012-07-02 17:21:20 UTC
This change was made to Mesa after the ARB agreed to change the spec.  The commit message says, "The two affected tests cases are updated to reflect the new behavior."  I don't see any recent changes to piglit, so maybe the test changes didn't get pushed?

I'm reassigning to Carl for follow-up.
Comment 2 Carl Worth 2012-07-10 00:52:46 UTC
(In reply to comment #1)
> This change was made to Mesa after the ARB agreed to change the spec.  The
> commit message says, "The two affected tests cases are updated to reflect the
> new behavior."  I don't see any recent changes to piglit, so maybe the test
> changes didn't get pushed?

I updated the tests in the glcpp suite, but neglected to fix piglit, oops!

> I'm reassigning to Carl for follow-up.

I'll take care of this. Feel free to ping me if I neglect it.

-Carl
Comment 3 Vinson Lee 2012-08-13 00:15:20 UTC
(In reply to comment #2)
> 
> I'll take care of this. Feel free to ping me if I neglect it.
> 
> -Carl

Ping.
Comment 4 lu hua 2012-09-05 05:38:00 UTC
It also happens on mesa 9.0 branch(commit:284fe975159981789).

Case 'spec_glsl-1.30_preprocessor_if_if-arg-must-be-defined-02.frag' also fails.

./bin/glslparsertest tests/spec/glsl-1.30/preprocessor/if/if-arg-must-be-defined-02.frag fail 1.30
libGL: OpenDriver: trying /opt/X11R7/lib/dri/i965_dri.so
Successfully compiled fragment shader tests/spec/glsl-1.30/preprocessor/if/if-arg-must-be-defined-02.frag:
Shader source:
// [config]
// expect_result: fail
// glsl_version: 1.30
// [end config]
//
// Check that the compiler raises an error when an undefined macro is used as
// an argument to the #elif directive.
//
// From page 11 (17 of pdf) of the GLSL 1.30 spec:
//     "It is an error to use #if or #elif on expressions containing
//     undefined macro names, other than as arguments to the
//     defined operator."

#version 130

#if 0
#       define JUNK 1
#elif UNDEFINED_MACRO
#       define JUNK 1
#endif

int f()
{
        return 0;
}

PIGLIT: {'result': 'fail' }
Comment 5 Vinson Lee 2013-01-22 01:55:03 UTC
mesa: 3a91e7955ace2885cfb23089852018a8037ca134 (master)
piglit: 8eae69515cb54ab0a3f3b5eed331851432790baa (master)

This regression is still present in the latest versions of piglit and mesa.
Comment 6 Carl Worth 2013-02-22 21:29:20 UTC
Created attachment 75371 [details]
Proposed fix for piglit

I apologize sincerely that I neglected this issue, even with the
various pings.

My lame excuse is that I had misconfigured some email filters in an
attempt to deal with the huge quanitity of bug spam I was getting from
bugzilla. I had been a bit too aggressive with those filters so I
failed to see any of the comments on this bug report until quite
recently when Ian mentioned them to me out of band.

Anyway, I've attached a piglit patch here to fix this bug, (by making
the tests expect the new behavior implemented in mesa). I've sent this
patch to piglit's mailing list for review, and also asked for avice
from Ian on the mesa mailing list.

If review comes back in favor of this change, I expect to push it soon
and close this bug.
Comment 7 Carl Worth 2013-03-01 22:13:33 UTC
(In reply to comment #6)
> If review comes back in favor of this change, I expect to push it soon
> and close this bug.

Pushed.

Thanks again for your patience.

-Carl


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.