[Mesa-dev] [PATCH] glsl/linker: Fix out variables linking during single stage

[Vadim Shovkoplias]

Hi Tapani, Timothy,

I also did some testing on my SNB laptop and didn't noticed any issues. CI
also didn't show that issues.

Regards,
Vadym



ср, 31 окт. 2018 г. в 9:13, Tapani Pälli <tapani.palli at intel.com>:

>
>
> On 10/31/18 7:28 AM, Tapani Pälli wrote:
> >
> >
> > On 10/31/18 12:57 AM, Timothy Arceri wrote:
> >> On 30/10/18 10:14 pm, Tapani Pälli wrote:
> >>> Hi;
> >>>
> >>> On 10/30/18 1:28 AM, Timothy Arceri wrote:
> >>>> On 29/10/18 10:05 pm, Vadim Shovkoplias wrote:
> >>>>> Hi Timothy,
> >>>>>
> >>>>> Thanks for the review. Piglit patch is updated with the additional
> >>>>> out var: https://patchwork.freedesktop.org/patch/258899/
> >>>>> Original reporter confirmed that issue is finally fixed with the
> >>>>> current patch and closed it.
> >>>>>
> >>>>> Can I ask to push the patch please ?
> >>>>
> >>>> I've pushed both. Thanks for the patches!
> >>>
> >>> Unfortunately it seems this patch introduced regression in Intel CI,
> >>> I'm getting following failures after this commit (7d66eddbbde):
> >>>
> >>> piglit.spec.glsl-1_30.execution.tex-miplevel-selection
> >>> texturelodoffset 2dshadow.snbm64
> >>> piglit.spec.glsl-1_30.execution.tex-miplevel-selection texturelod
> >>> 2dshadow.snbm64
> >>> piglit.spec.glsl-1_30.execution.tex-miplevel-selection texturelod
> >>> cube.snbm64
> >>> generated.gpu-hang-otc-gfxtest-snbgt2-02.snbm64
> >>
> >> I don't have a Sandy Bridge to test with but its really strange that
> >> this would break only one generation of hardware. Are you sure it was
> >> this patch?
> >
> > Well .. not 100% sure but it looks like previous commit won't show these
> > fails, it is strange. I'll do some runs today and try to figure it out.
>
> Yeah, it passes now so it seems this was some kind of 'CI noise'.
>
> >
> >>>
> >>>
> >>>>>
> >>>>> Regards,
> >>>>> Vadym
> >>>>>
> >>>>> сб, 27 окт. 2018 г. в 1:21, Timothy Arceri <tarceri at itsqueeze.com
> >>>>> <mailto:tarceri at itsqueeze.com>>:
> >>>>>
> >>>>>     On Wed, Oct 24, 2018, at 3:28 AM, Vadym Shovkoplias wrote:
> >>>>>      > Since out variables are copied from shader objects instruction
> >>>>>      > streams to linked shader instruction steam it should be cloned
> >>>>>      > at first to keep source instruction steam unaltered.
> >>>>>      >
> >>>>>      > Fixes: 966a797e433 glsl/linker: Link all out vars from a
> shader
> >>>>>      > objects on a single stage
> >>>>>      > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
> >>>>>      > Signed-off-by: Vadym Shovkoplias
> >>>>>     <vadym.shovkoplias at globallogic.com
> >>>>>     <mailto:vadym.shovkoplias at globallogic.com>>
> >>>>>
> >>>>>     Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com
> >>>>>     <mailto:tarceri at itsqueeze.com>>
> >>>>>
> >>>>>     Also please either update the existing piglit or add a new one to
> >>>>>     better cover the use of the freed vars. From the bug report it
> >>>>> seems
> >>>>>     adding another out var is enough to do it. Thanks.
> >>>>>
> >>>>>      > ---
> >>>>>      >  src/compiler/glsl/linker.cpp | 3 ++-
> >>>>>      >  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>      >
> >>>>>      > diff --git a/src/compiler/glsl/linker.cpp
> >>>>>     b/src/compiler/glsl/linker.cpp
> >>>>>      > index 7db34ebf95..8b1b03322a 100644
> >>>>>      > --- a/src/compiler/glsl/linker.cpp
> >>>>>      > +++ b/src/compiler/glsl/linker.cpp
> >>>>>      > @@ -2269,10 +2269,11 @@ link_output_variables(struct
> >>>>>     gl_linked_shader
> >>>>>      > *linked_shader,
> >>>>>      >           if (ir->ir_type != ir_type_variable)
> >>>>>      >              continue;
> >>>>>      >
> >>>>>      > -         ir_variable *const var = (ir_variable *) ir;
> >>>>>      > +         ir_variable *var = (ir_variable *) ir;
> >>>>>      >
> >>>>>      >           if (var->data.mode == ir_var_shader_out &&
> >>>>>      >                 !symbols->get_variable(var->name)) {
> >>>>>      > +            var = var->clone(linked_shader, NULL);
> >>>>>      >              symbols->add_variable(var);
> >>>>>      >              linked_shader->ir->push_head(var);
> >>>>>      >           }
> >>>>>      > --
> >>>>>      > 2.18.0
> >>>>>      >
> >>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181031/73118eaf/attachment-0001.html>