| Summary: | Wrong line numbers expanded while compiling shaders | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Zhaowei Yuan <zhaowei.yuan> |
| Component: | glsl-compiler | Assignee: | mesa-dev |
| Status: | RESOLVED MOVED | QA Contact: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | 17.1 | ||
| Hardware: | ARM | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | patch that fix GLSL compiling issue | ||
Thanks for the patch! However you should sign up to the mesa mailing list [1] and send it there for review. [1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev See also: https://www.mesa3d.org/submittingpatches.html (In reply to Timothy Arceri from comment #1) > Thanks for the patch! However you should sign up to the mesa mailing list > [1] and send it there for review. > > [1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > See also: https://www.mesa3d.org/submittingpatches.html I have posted a patch here: https://patchwork.freedesktop.org/patch/225818/ but it hasn't been reviewed by anybody yet I posted patch v2 to cc more maintainer: https://patchwork.freedesktop.org/patch/240324/ A new patch to fix this issue: https://patchwork.freedesktop.org/patch/245027/ This seems like a duplicate of https://bugs.freedesktop.org/show_bug.cgi?id=106807 My faint recollection is that the specification of the __LINE__ macro is inconsistent. If, as Ken says, this has been removed from the specification, why are we looking at patches to change the implementation? (In reply to Mark Janes from comment #5) > This seems like a duplicate of > > https://bugs.freedesktop.org/show_bug.cgi?id=106807 > > My faint recollection is that the specification of the __LINE__ macro is > inconsistent. If, as Ken says, this has been removed from the > specification, why are we looking at patches to change the implementation? No, __LINE__ is perfectly well defined and not going away. #line directives to alter the current line using expressions is going away. I'm pretty sure Zhaowei Yuan's patch fixes a real bug we've had for a long time. (In reply to Kenneth Graunke from comment #6) > (In reply to Mark Janes from comment #5) > > This seems like a duplicate of > > > > https://bugs.freedesktop.org/show_bug.cgi?id=106807 > > > > My faint recollection is that the specification of the __LINE__ macro is > > inconsistent. If, as Ken says, this has been removed from the > > specification, why are we looking at patches to change the implementation? > > No, __LINE__ is perfectly well defined and not going away. #line directives > to alter the current line using expressions is going away. I'm pretty sure > Zhaowei Yuan's patch fixes a real bug we've had for a long time. Thanks. However, I'm not sure my patch is the best solution, hope someone can review it and give me some suggestion -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/816. |
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.
Created attachment 139657 [details] patch that fix GLSL compiling issue Following shader code is got from a CTS test case which is intend to check if the shader compiler can expand macro with right line number: 1 #version 300 es 2 precision mediump float; 3 #define BBB __LINE__, /* 4 */ __LINE__ 5 #define AAA(a,b) BBB, a, b 6 7 void main() 8 { 9 out0 = vec4(AAA(__LINE__, 10 __LINE__ 11 )); 12 } It expects to make out0 filled with (11, 11, 9, 10) values, But mesa gives (3,4, 9, 10). There are 2 problems: 1. Macro BBB should be expanded as the line number exactly where it is referenced, not declared 2. Macro BBB have a character '\n' within "/**/" which should be ignored and treat 2 "__LINE__" as the in the same line I fixed this problem with the attached patch, please review, thanks