Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-09 Thread Vadim Shovkoplias
Hi Ian, Thanks for the explanation. I've just sent a patch for review with the compile time check https://patchwork.freedesktop.org/patch/255542/ Regards, Vadym пт, 5 окт. 2018 г. в 16:55, Ian Romanick : > On 10/03/2018 11:52 PM, Iago Toral wrote: > > On Wed, 2018-10-03 at 16:24 +0300, Vadim Sh

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-05 Thread Ian Romanick
On 10/03/2018 11:52 PM, Iago Toral wrote: > On Wed, 2018-10-03 at 16:24 +0300, Vadim Shovkoplias wrote: >> Hi Iago, >> >> I also think that compiler is the best place for the fix. But from my >> understanding compiler fix will be limited to distinct shader objects >> (not the shader stage). >> In G

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-05 Thread Ian Romanick
On 10/03/2018 01:39 AM, Vadym Shovkoplias wrote: > From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification > > "A program will fail to compile or link if any shader > or stage contains two or more functions with the same > name if the name is associated with a subroutine type

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-04 Thread Iago Toral
Jenkins is good, I have pushed the patch with a small style fix and a v2 tag in the commit log. Iago On Thu, 2018-10-04 at 10:15 +0300, Vadim Shovkoplias wrote: > Thanks, I'll appreciate if you will push the patch once test will be > finished. > чт, 4 окт. 2018 г. в 9:52, Iago Toral : > > On Wed, 2

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-04 Thread Vadim Shovkoplias
Sure, my fault. чт, 4 окт. 2018 г. в 10:27, Tapani Pälli : > Reviewed-by: Tapani Pälli > > In the future, please remember update patch version when making changes > and posting patch again, easiest way is to give '-v2' to > git-format-patch and then update commit message with information what >

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-04 Thread Tapani Pälli
Reviewed-by: Tapani Pälli In the future, please remember update patch version when making changes and posting patch again, easiest way is to give '-v2' to git-format-patch and then update commit message with information what was changed. This will help the review process. Otherwise it may not

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-04 Thread Vadim Shovkoplias
Thanks, I'll appreciate if you will push the patch once test will be finished. чт, 4 окт. 2018 г. в 9:52, Iago Toral : > On Wed, 2018-10-03 at 16:24 +0300, Vadim Shovkoplias wrote: > > Hi Iago, > > I also think that compiler is the best place for the fix. But from my > understanding compiler fix

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-03 Thread Iago Toral
On Wed, 2018-10-03 at 16:24 +0300, Vadim Shovkoplias wrote: > Hi Iago, > I also think that compiler is the best place for the fix. But from my > understanding compiler fix will be limited to distinct shader objects > (not the shader stage). > In GLSL spec mentioned: "A program will fail to compile

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-03 Thread Vadim Shovkoplias
Hi Iago, I also think that compiler is the best place for the fix. But from my understanding compiler fix will be limited to distinct shader objects (not the shader stage). In GLSL spec mentioned: "A program will fail to compile or link if any *shader or stage* contains two or more functions with

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-03 Thread Iago Toral
Hi Vadym, I think this looks correct, but I was wondering if you considered implementing this check in ir_reader::read_function_sig (ir_reader.cpp) instead, which runs at compile time. My rationale is that given the option, I think it is preferable to push work to compile time rather than link tim

[Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-03 Thread Vadym Shovkoplias
From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification "A program will fail to compile or link if any shader or stage contains two or more functions with the same name if the name is associated with a subroutine type." Fixes: * no-overloads.vert Bugzilla: https://bugs.

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-02 Thread Tapani Pälli
On 10/2/18 7:38 PM, Vadym Shovkoplias wrote: Hi Tapani, Thanks for the review! Completely agree with the first comment, I'll change that and resend the patch. Regarding second comment. I'm not sure if it is possible to do this check after the optimization loop. From my observations compiler

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-02 Thread Vadym Shovkoplias
Hi Tapani, Thanks for the review! Completely agree with the first comment, I'll change that and resend the patch. Regarding second comment. I'm not sure if it is possible to do this check after the optimization loop. From my observations compiler inlines everything and only after that it removes

Re: [Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-02 Thread Tapani Pälli
On 10/1/18 5:03 PM, Vadym Shovkoplias wrote: From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification "A program will fail to compile or link if any shader or stage contains two or more functions with the same name if the name is associated with a subroutine type." Fix

[Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

2018-10-01 Thread Vadym Shovkoplias
From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification "A program will fail to compile or link if any shader or stage contains two or more functions with the same name if the name is associated with a subroutine type." Fixes: * no-overloads.vert Bugzilla: https://bugs.