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

2018-10-31 Thread 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 :

>
>
> 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  >>>>> <mailto:tarc...@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
> >>>>>  >>>>> <mailto:vadym.shovkopl...@globallogic.com>>
> >>>>>
> >>>>> Reviewed-by: Timothy Arceri  >>>>> <mailto:tarc...@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
> >>>>>  >
> >>>>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Don't bother to call blorp if the blit is zero-sized

2018-10-29 Thread Vadim Shovkoplias 
Hi Jason,

Looks like there is an issue with the float comparison. It works perfectly
fine for me if it is compared with some precision:

if( (fabsf(*dstX1 - *dstX0) < 1e-8F) || (fabsf(*dstY1 - *dstY0) < 1e-8F) ) {
  return true;



пн, 1 окт. 2018 г. в 19:24, Eric Engestrom :

> On Monday, 2018-10-01 11:04:09 +0200, Juan A. Suarez Romero wrote:
> > On Tue, 2018-09-11 at 11:15 -0500, Jason Ekstrand wrote:
> > > Cc: mesa-sta...@lists.freedesktop.org
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107892
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_meta_util.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> >
> > This has been reviewed, but not pushed yet.
> >
> >
> >   J.A.
> >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
> b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > index 908b0989769..6714d96237c 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > @@ -247,6 +247,9 @@ brw_meta_mirror_clip_and_scissor(const struct
> gl_context *ctx,
> > >  clip_src_y1, clip_dst_y1, clip_dst_y0,
> > >  scaleY, false);
> > >
> > > +   if (*dstX0 == *dstX1 || *dstY0 == *dstY1)
> > > +  return true;
>
> This comes right after this code (few lines above in the same function):
>
>   float scaleX = (float) (*srcX1 - *srcX0) / (*dstX1 - *dstX0);
>   float scaleY = (float) (*srcY1 - *srcY0) / (*dstY1 - *dstY0);
>
> if *dstX0 == *dstX1, that would be a division by 0, so I don't think that
> this new `if *dstX0 == *dstX1` is reachable (same for *dstY0 == *dstY1)
>
> > > +
> > > /* Account for the fact that in the system framebuffer, the origin
> is at
> > >  * the lower left.
> > >  */
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-10-29 Thread Vadim Shovkoplias 
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 ?

Regards,
Vadym

сб, 27 окт. 2018 г. в 1:21, Timothy Arceri :

> 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 
>
> Reviewed-by: Timothy Arceri 
>
> 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
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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 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 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".
> >> So if I understand the spec correctly such restriction for the shader
> >> stage can be fixed only in linker. E.g. consider the use case when
> >> fragment shader is linked from
> >> two fragment shader objects. One object contains function foo() which
> >> is associated with subroutine type and second shader object has some
> >> regular foo() function
> >> with the same name.  I suppose compiler fix won't be able to detect
> this.
> >
> > Yes, that is correct, good point.
> >
> >> IMHO the best way to fix this is to implement 2 patches:
> >> 1) One patch for linker to implement restriction for the shader stages
> >> (already done in this patch)
> >> 2) Another one for compiler to check the restriction for distinct
> >> shader objects.
> >>
> >> What do you think ?
> >
> > Since we need to keep the link-time check, any compile-time checks we
> > add would be redundant with that, so I think we should just go with the
> > link-time check only (your original patch)
>
> The spec quotation is pretty clear: "A program will fail to compile ...
> if any shader ... contains two or more functions with the same name if
> the name is associated with a subroutine type."  To be strictly correct,
> both checks are necessary.  Before doing that extra work, someone should
> check that glslang has the compile-time check.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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
> was changed. This will help the review process. Otherwise it may not be
> obvious what got changed.
>
> On 10/3/18 11: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."
> >
> > Fixes:
> >  * no-overloads.vert
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108109
> > Signed-off-by: Vadym Shovkoplias 
> > ---
> >   src/compiler/glsl/linker.cpp | 40 
> >   1 file changed, 40 insertions(+)
> >
> > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> > index 3fde7e78d3..aca5488a1e 100644
> > --- a/src/compiler/glsl/linker.cpp
> > +++ b/src/compiler/glsl/linker.cpp
> > @@ -4639,6 +4639,45 @@ link_assign_subroutine_types(struct
> gl_shader_program *prog)
> >  }
> >   }
> >
> > +static void
> > +verify_subroutine_associated_funcs(struct gl_shader_program *prog)
> > +{
> > +   unsigned mask = prog->data->linked_stages;
> > +   while (mask) {
> > +  const int i = u_bit_scan();
> > +  gl_program *p = prog->_LinkedShaders[i]->Program;
> > +  glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
> > +
> > +  /*
> > +   * From OpenGL ES Shading Language 4.00 specification
> > +   * (6.1.2 Subroutines):
> > +   * "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."
> > +   */
> > +  for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
> > + unsigned definitions = 0;
> > + char *name = p->sh.SubroutineFunctions[j].name;
> > + ir_function *fn = symbols->get_function(name);
> > +
> > + /* Calculate number of function definitions with the same name
> */
> > + foreach_in_list(ir_function_signature, sig, >signatures) {
> > +if (sig->is_defined) {
> > +   if (++definitions > 1) {
> > +  linker_error(prog, "%s shader contains two or more
> function "
> > +"definitions with name `%s', which is "
> > +"associated with a subroutine type.\n",
> > +_mesa_shader_stage_to_string(i),
> > +fn->name);
> > +  return;
> > +   }
> > +}
> > + }
> > +  }
> > +   }
> > +}
> > +
> > +
> >   static void
> >   set_always_active_io(exec_list *ir, ir_variable_mode io_mode)
> >   {
> > @@ -5024,6 +5063,7 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
> >
> >  check_explicit_uniform_locations(ctx, prog);
> >  link_assign_subroutine_types(prog);
> > +   verify_subroutine_associated_funcs(prog);
> >
> >  if (!prog->data->LinkStatus)
> > goto done;
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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 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 the same name if the name
> is
> associated with a subroutine type".
> So if I understand the spec correctly such restriction for the shader
> stage can be fixed only in linker. E.g. consider the use case when fragment
> shader is linked from
> two fragment shader objects. One object contains function foo() which is
> associated with subroutine type and second shader object has some regular
> foo() function
> with the same name.  I suppose compiler fix won't be able to detect this.
>
>
> Yes, that is correct, good point.
>
> IMHO the best way to fix this is to implement 2 patches:
> 1) One patch for linker to implement restriction for the shader stages
> (already done in this patch)
> 2) Another one for compiler to check the restriction for distinct shader
> objects.
>
> What do you think ?
>
>
> Since we need to keep the link-time check, any compile-time checks we add
> would be redundant with that, so I think we should just go with the
> link-time check only (your original patch)
>
> Once you have addressed Tapani's comment feel free to add my:
>
> Reviewed-by: Iago Toral Quiroga 
>
> I have sent the patch to Jenkins for testing just in case, if that comes
> back clean as expected let me know if you need me to push the patch for you.
>
> Iago
>
> ср, 3 окт. 2018 г. в 13:59, 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 time.
>
> Iago
>
> On Wed, 2018-10-03 at 11:39 +0300, 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."
> >
> > Fixes:
> > * no-overloads.vert
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108109
> > Signed-off-by: Vadym Shovkoplias 
> > ---
> >  src/compiler/glsl/linker.cpp | 40
> > 
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/src/compiler/glsl/linker.cpp
> > b/src/compiler/glsl/linker.cpp
> > index 3fde7e78d3..aca5488a1e 100644
> > --- a/src/compiler/glsl/linker.cpp
> > +++ b/src/compiler/glsl/linker.cpp
> > @@ -4639,6 +4639,45 @@ link_assign_subroutine_types(struct
> > gl_shader_program *prog)
> > }
> >  }
> >
> > +static void
> > +verify_subroutine_associated_funcs(struct gl_shader_program *prog)
> > +{
> > +   unsigned mask = prog->data->linked_stages;
> > +   while (mask) {
> > +  const int i = u_bit_scan();
> > +  gl_program *p = prog->_LinkedShaders[i]->Program;
> > +  glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
> > +
> > +  /*
> > +   * From OpenGL ES Shading Language 4.00 specification
> > +   * (6.1.2 Subroutines):
> > +   * "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."
> > +   */
> > +  for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
> > + unsigned definitions = 0;
> > + char *name = p->sh.SubroutineFunctions[j].name;
> > + ir_function *fn = symbols->get_function(name);
> > +
> > + /* Calculate number of function definitions with the same
> > name */
> > + foreach_in_list(ir_function_signature, sig, 
> > >signatures) {
> > +if (sig->is_defined) {
> > +   if (++definitions > 1) {
> > +  linker_error(prog, "%s shader contains two or more
> > function "
> > +"definitions with name `%s', which is

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 the same name if the name is
associated with a subroutine type".
So if I understand the spec correctly such restriction for the shader stage
can be fixed only in linker. E.g. consider the use case when fragment
shader is linked from
two fragment shader objects. One object contains function foo() which is
associated with subroutine type and second shader object has some regular
foo() function
with the same name.  I suppose compiler fix won't be able to detect this.

IMHO the best way to fix this is to implement 2 patches:
1) One patch for linker to implement restriction for the shader stages
(already done in this patch)
2) Another one for compiler to check the restriction for distinct shader
objects.

What do you think ?

ср, 3 окт. 2018 г. в 13:59, 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 time.
>
> Iago
>
> On Wed, 2018-10-03 at 11:39 +0300, 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."
> >
> > Fixes:
> > * no-overloads.vert
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108109
> > Signed-off-by: Vadym Shovkoplias 
> > ---
> >  src/compiler/glsl/linker.cpp | 40
> > 
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/src/compiler/glsl/linker.cpp
> > b/src/compiler/glsl/linker.cpp
> > index 3fde7e78d3..aca5488a1e 100644
> > --- a/src/compiler/glsl/linker.cpp
> > +++ b/src/compiler/glsl/linker.cpp
> > @@ -4639,6 +4639,45 @@ link_assign_subroutine_types(struct
> > gl_shader_program *prog)
> > }
> >  }
> >
> > +static void
> > +verify_subroutine_associated_funcs(struct gl_shader_program *prog)
> > +{
> > +   unsigned mask = prog->data->linked_stages;
> > +   while (mask) {
> > +  const int i = u_bit_scan();
> > +  gl_program *p = prog->_LinkedShaders[i]->Program;
> > +  glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
> > +
> > +  /*
> > +   * From OpenGL ES Shading Language 4.00 specification
> > +   * (6.1.2 Subroutines):
> > +   * "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."
> > +   */
> > +  for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
> > + unsigned definitions = 0;
> > + char *name = p->sh.SubroutineFunctions[j].name;
> > + ir_function *fn = symbols->get_function(name);
> > +
> > + /* Calculate number of function definitions with the same
> > name */
> > + foreach_in_list(ir_function_signature, sig, 
> > >signatures) {
> > +if (sig->is_defined) {
> > +   if (++definitions > 1) {
> > +  linker_error(prog, "%s shader contains two or more
> > function "
> > +"definitions with name `%s', which is "
> > +"associated with a subroutine type.\n",
> > +_mesa_shader_stage_to_string(i),
> > +fn->name);
> > +  return;
> > +   }
> > +}
> > + }
> > +  }
> > +   }
> > +}
> > +
> > +
> >  static void
> >  set_always_active_io(exec_list *ir, ir_variable_mode io_mode)
> >  {
> > @@ -5024,6 +5063,7 @@ link_shaders(struct gl_context *ctx, struct
> > gl_shader_program *prog)
> >
> > check_explicit_uniform_locations(ctx, prog);
> > link_assign_subroutine_types(prog);
> > +   verify_subroutine_associated_funcs(prog);
> >
> > if (!prog->data->LinkStatus)
> >goto done;
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Don't bother to call blorp if the blit is zero-sized

2018-10-01 Thread Vadim Shovkoplias 
Hi Jason,

I'm still getting crash with this patch when run conformance test in the
firefox.
Assert is triggered during following call:

glBlitFramebuffer(srcX0 = -1, srcY0 = -1, srcX1 = 2147483646, srcY1 =
2147483646, dstX0 = 0, dstY0 = 0, dstX1 = 8, dstY1 = 8, mask =
GL_COLOR_BUFFER_BIT, filter = GL_NEAREST)

пн, 1 окт. 2018 г. в 12:50, Juan A. Suarez Romero :

> On Tue, 2018-09-11 at 11:15 -0500, Jason Ekstrand wrote:
> > Cc: mesa-sta...@lists.freedesktop.org
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107892
> > ---
> >  src/mesa/drivers/dri/i965/brw_meta_util.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
>
> This has been reviewed, but not pushed yet.
>
>
> J.A.
>
> > diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
> b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > index 908b0989769..6714d96237c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> > +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > @@ -247,6 +247,9 @@ brw_meta_mirror_clip_and_scissor(const struct
> gl_context *ctx,
> >  clip_src_y1, clip_dst_y1, clip_dst_y0,
> >  scaleY, false);
> >
> > +   if (*dstX0 == *dstX1 || *dstY0 == *dstY1)
> > +  return true;
> > +
> > /* Account for the fact that in the system framebuffer, the origin
> is at
> >  * the lower left.
> >  */
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl/linker: Link all out vars from a shader objects on a single stage

2018-08-28 Thread Vadim Shovkoplias 
Hi Timothy,

Thanks for the review! Space was removed. Can you please push this patch
since I haven't got write permissions ?

Regards,
Vadym

вт, 28 авг. 2018 г. в 10:32, Vadym Shovkoplias :

> From: "vadym.shovkoplias" 
>
> During intra stage linking some out variables can be dropped because
> it is not used in a shader with the main function. But these out vars
> can be referenced on later stages which can lead to further linking
> errors.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
> Signed-off-by: Vadym Shovkoplias 
> ---
>  src/compiler/glsl/linker.cpp | 37 
>  1 file changed, 37 insertions(+)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 3ce78fe642..dbd76b7fcc 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -2187,6 +2187,40 @@ link_cs_input_layout_qualifiers(struct
> gl_shader_program *prog,
> }
>  }
>
> +/**
> + * Link all out variables on a single stage which are not
> + * directly used in a shader with the main function.
> + */
> +static void
> +link_output_variables(struct gl_linked_shader *linked_shader,
> +  struct gl_shader **shader_list,
> +  unsigned num_shaders)
> +{
> +   struct glsl_symbol_table *symbols = linked_shader->symbols;
> +
> +   for (unsigned i = 0; i < num_shaders; i++) {
> +
> +  /* Skip shader object with main function */
> +  if (shader_list[i]->symbols->get_function("main"))
> + continue;
> +
> +  foreach_in_list (ir_instruction, ir, shader_list[i]->ir) {
> + if (ir->ir_type != ir_type_variable)
> +continue;
> +
> + ir_variable *const var = (ir_variable *) ir;
> +
> + if (var->data.mode == ir_var_shader_out &&
> +   !symbols->get_variable(var->name)) {
> +symbols->add_variable(var);
> +linked_shader->ir->push_head(var);
> + }
> +  }
> +   }
> +
> +   return;
> +}
> +
>
>  /**
>   * Combine a group of shaders for a single stage to generate a linked
> shader
> @@ -2352,6 +2386,9 @@ link_intrastage_shaders(void *mem_ctx,
>return NULL;
> }
>
> +   if (linked->Stage != MESA_SHADER_FRAGMENT)
> +  link_output_variables(linked, shader_list, num_shaders);
> +
> /* Make a pass over all variable declarations to ensure that arrays
> with
>  * unspecified sizes have a size specified.  The size is inferred from
> the
>  * max_array_access field.
> --
> 2.18.0
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Disable guardband clipping on SandyBridge for odd dimensions

2018-07-27 Thread Vadim Shovkoplias 
Hi Rafael,

Thanks a lot for reviewing the patch! Hopefully no one will have any
objections to push this.

Thanks,
Vadym

2018-07-26 18:11 GMT+03:00 Rafael Antognolli :

> Hi Vadym,
>
> Ken and Ian explained a bit the situation on this one to me, and it
> looks like neither of them are really against this patch. So unless
> someone else raise any concern, I'll ack and push the patch later today.
>
> Thanks for fixing this.
>
> Rafael
>
> On Thu, Jul 26, 2018 at 04:04:29PM +0300, Vadym Shovkoplias wrote:
> > ping
> >
> > On Tue, Jul 3, 2018 at 5:09 PM, Vadim Shovkoplias <
> vadim.shovkopl...@gmail.com>
> > wrote:
> >
> > Hi mesa devs,
> >
> > Can anyone please review this ?
> > This patch fixes following bugs:
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104388
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106158
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106667
> >
> >
> > 2018-06-07 18:27 GMT+03:00 Vadim Shovkoplias <
> vadim.shovkopl...@gmail.com>:
> >
> > Hi Kenneth,
> >
> > Can you please look at this patch ?
> >
> > 2018-06-07 15:30 GMT+03:00 Den :
> >
> > Hello. Found out that this patch also fixes 2 new issues:
> >
> > Bugzilla: https://bugs.freedesktop.org/
> show_bug.cgi?id=106158
> >
> > Bugzilla: https://bugs.freedesktop.org/
> show_bug.cgi?id=106667
> >
> > Tested-by: Denis 
> >
> >
> >
> > On 24.05.18 14:16, vadym.shovkoplias wrote:
> >
> > Bugzilla: https://bugs.freedesktop.org/
> show_bug.cgi?id=104388
> > Signed-off-by: Andriy Khulap <
> andriy.khu...@globallogic.com>
> > ---
> >   src/mesa/drivers/dri/i965/genX_state_upload.c | 11
> > +++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> b/
> > src/mesa/drivers/dri/i965/genX_state_upload.c
> > index b485e2c..5aa8033 100644
> > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > @@ -2473,6 +2473,17 @@ brw_calculate_guardband_size(
> uint32_t
> > fb_width, uint32_t fb_height,
> >   */
> >  const float gb_size = GEN_GEN >= 7 ? 16384.0f :
> 8192.0f;
> >   +   /* Workaround: prevent gpu hangs on SandyBridge
> > +* by disabling guardband clipping for odd
> dimensions.
> > +*/
> > +   if (GEN_GEN == 6 && (fb_width & 1 || fb_height & 1))
> {
> > +  *xmin = -1.0f;
> > +  *xmax =  1.0f;
> > +  *ymin = -1.0f;
> > +  *ymax =  1.0f;
> > +  return;
> > +   }
> > +
> >  if (m00 != 0 && m11 != 0) {
> > /* First, we compute the screen-space render
> area */
> > const float ss_ra_xmin = MIN3(0, m30 +
> m00, m30
> > - m00);
> >
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >
> >
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >
> >
> >
> > --
> >
> > Vadym Shovkoplias | Senior Software Engineer
> > GlobalLogic
> > P +380.57.766.7667  M +3.8050.931.7304  S vadym.shovkoplias
> > www.globallogic.com
> >
> > http://www.globallogic.com/email_disclaimer.txt
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Disable guardband clipping on SandyBridge for odd dimensions

2018-07-03 Thread Vadim Shovkoplias 
Hi mesa devs,

Can anyone please review this ?
This patch fixes following bugs:

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104388
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106158
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106667


2018-06-07 18:27 GMT+03:00 Vadim Shovkoplias :

> Hi Kenneth,
>
> Can you please look at this patch ?
>
> 2018-06-07 15:30 GMT+03:00 Den :
>
>> Hello. Found out that this patch also fixes 2 new issues:
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106158
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106667
>>
>> Tested-by: Denis 
>>
>>
>>
>> On 24.05.18 14:16, vadym.shovkoplias wrote:
>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104388
>>> Signed-off-by: Andriy Khulap 
>>> ---
>>>   src/mesa/drivers/dri/i965/genX_state_upload.c | 11 +++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
>>> b/src/mesa/drivers/dri/i965/genX_state_upload.c
>>> index b485e2c..5aa8033 100644
>>> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
>>> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
>>> @@ -2473,6 +2473,17 @@ brw_calculate_guardband_size(uint32_t fb_width,
>>> uint32_t fb_height,
>>>   */
>>>  const float gb_size = GEN_GEN >= 7 ? 16384.0f : 8192.0f;
>>>   +   /* Workaround: prevent gpu hangs on SandyBridge
>>> +* by disabling guardband clipping for odd dimensions.
>>> +*/
>>> +   if (GEN_GEN == 6 && (fb_width & 1 || fb_height & 1)) {
>>> +  *xmin = -1.0f;
>>> +  *xmax =  1.0f;
>>> +  *ymin = -1.0f;
>>> +  *ymax =  1.0f;
>>> +  return;
>>> +   }
>>> +
>>>  if (m00 != 0 && m11 != 0) {
>>> /* First, we compute the screen-space render area */
>>> const float ss_ra_xmin = MIN3(0, m30 + m00, m30 - m00);
>>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Disable guardband clipping on SandyBridge for odd dimensions

2018-06-07 Thread Vadim Shovkoplias 
Hi Kenneth,

Can you please look at this patch ?

2018-06-07 15:30 GMT+03:00 Den :

> Hello. Found out that this patch also fixes 2 new issues:
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106158
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106667
>
> Tested-by: Denis 
>
>
>
> On 24.05.18 14:16, vadym.shovkoplias wrote:
>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104388
>> Signed-off-by: Andriy Khulap 
>> ---
>>   src/mesa/drivers/dri/i965/genX_state_upload.c | 11 +++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
>> b/src/mesa/drivers/dri/i965/genX_state_upload.c
>> index b485e2c..5aa8033 100644
>> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
>> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
>> @@ -2473,6 +2473,17 @@ brw_calculate_guardband_size(uint32_t fb_width,
>> uint32_t fb_height,
>>   */
>>  const float gb_size = GEN_GEN >= 7 ? 16384.0f : 8192.0f;
>>   +   /* Workaround: prevent gpu hangs on SandyBridge
>> +* by disabling guardband clipping for odd dimensions.
>> +*/
>> +   if (GEN_GEN == 6 && (fb_width & 1 || fb_height & 1)) {
>> +  *xmin = -1.0f;
>> +  *xmax =  1.0f;
>> +  *ymin = -1.0f;
>> +  *ymax =  1.0f;
>> +  return;
>> +   }
>> +
>>  if (m00 != 0 && m11 != 0) {
>> /* First, we compute the screen-space render area */
>> const float ss_ra_xmin = MIN3(0, m30 + m00, m30 - m00);
>>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Don't try to disable render buffers for compute

2018-01-24 Thread Vadim Shovkoplias 
No worries :)

2018-01-24 11:36 GMT+02:00 Pohjolainen, Topi <topi.pohjolai...@gmail.com>:

> On Tue, Jan 16, 2018 at 03:58:03PM +0200, Vadim Shovkoplias wrote:
> > Tested-by: Vadym Shovkoplias <vadym.shovkopl...@globallogic.com>
>
> Just realized that I pushed without your Tested-by, I'm sorry about that!
>
> >
> > 2018-01-16 15:31 GMT+02:00 Topi Pohjolainen <topi.pohjolai...@gmail.com
> >:
> >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104546
> > > CC: xinghua@intel.com
> > > Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_draw.c | 6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> > > b/src/mesa/drivers/dri/i965/brw_draw.c
> > > index 7e29dcfd4e8..626cd3fdb70 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > > @@ -441,8 +441,10 @@ brw_predraw_resolve_inputs(struct brw_context
> *brw,
> > > bool rendering)
> > >  tex_obj = intel_texture_object(u->TexObj);
> > >
> > >  if (tex_obj && tex_obj->mt) {
> > > -   intel_disable_rb_aux_buffer(brw, tex_obj->mt, 0, ~0,
> > > -   "as a shader image");
> > > +   if (rendering) {
> > > +  intel_disable_rb_aux_buffer(brw, tex_obj->mt, 0,
> ~0,
> > > +  "as a shader image");
> > > +   }
> > >
> > > intel_miptree_prepare_image(brw, tex_obj->mt);
> > >
> > > --
> > > 2.14.1
> > >
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Don't try to disable render buffers for compute

2018-01-16 Thread Vadim Shovkoplias 
Tested-by: Vadym Shovkoplias 

2018-01-16 15:31 GMT+02:00 Topi Pohjolainen :

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104546
> CC: xinghua@intel.com
> Signed-off-by: Topi Pohjolainen 
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index 7e29dcfd4e8..626cd3fdb70 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -441,8 +441,10 @@ brw_predraw_resolve_inputs(struct brw_context *brw,
> bool rendering)
>  tex_obj = intel_texture_object(u->TexObj);
>
>  if (tex_obj && tex_obj->mt) {
> -   intel_disable_rb_aux_buffer(brw, tex_obj->mt, 0, ~0,
> -   "as a shader image");
> +   if (rendering) {
> +  intel_disable_rb_aux_buffer(brw, tex_obj->mt, 0, ~0,
> +  "as a shader image");
> +   }
>
> intel_miptree_prepare_image(brw, tex_obj->mt);
>
> --
> 2.14.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] loader/dri3: Avoid freeing renderbuffers in use

2018-01-11 Thread Vadim Shovkoplias 
Tested-by: Vadym Shovkoplias 

This patch also fixes similar issue with SHENZHEN I/O game (
https://bugs.freedesktop.org/show_bug.cgi?id=104392)
Tested on HP Zbook.

2018-01-11 11:53 GMT+02:00 Thomas Hellstrom :

> Upon reception of an event that lowered the number of active back buffers,
> the code would immediately try to free all back buffers with an id equal
> to or
> higher than the new number of active back buffers.
>
> However, that could lead to an active or to-be-active back buffer being
> freed,
> since the old number of back buffers was used when obtaining an idle back
> buffer for use.
>
> This lead to crashes when lowering the number of active back buffers by
> transitioning from page-flipping to non-page-flipping presents.
>
> Fix this by computing the number of active back buffers only when trying to
> obtain a new back buffer.
>
> Fixes: 15e208c4cc ("loader/dri3: Don't accidently free buffer holding new
> back content")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104214
> Cc: "17.3" 
> Signed-off-by: Thomas Hellstrom 
> ---
>  src/loader/loader_dri3_helper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_
> helper.c
> index cc890bc..c01b0ac 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -205,7 +205,6 @@ void
>  loader_dri3_set_swap_interval(struct loader_dri3_drawable *draw, int
> interval)
>  {
> draw->swap_interval = interval;
> -   dri3_update_num_back(draw);
>  }
>
>  /** dri3_free_render_buffer
> @@ -377,7 +376,6 @@ dri3_handle_present_event(struct loader_dri3_drawable
> *draw,
>  draw->flipping = false;
>  break;
>   }
> - dri3_update_num_back(draw);
>
>   if (draw->vtable->show_fps)
>  draw->vtable->show_fps(draw, ce->ust);
> @@ -402,7 +400,8 @@ dri3_handle_present_event(struct loader_dri3_drawable
> *draw,
>  buf->busy = 0;
>
>   if (buf && draw->num_back <= b && b < LOADER_DRI3_MAX_BACK &&
> - draw->cur_blit_source != b) {
> + draw->cur_blit_source != b &&
> + buf->busy == 0) {
>  dri3_free_render_buffer(draw, buf);
>  draw->buffers[b] = NULL;
>   }
> @@ -537,6 +536,7 @@ dri3_find_back(struct loader_dri3_drawable *draw)
> /* Check whether we need to reuse the current back buffer as new back.
>  * In that case, wait until it's not busy anymore.
>  */
> +   dri3_update_num_back(draw);
> num_to_consider = draw->num_back;
> if (!loader_dri3_have_image_blit(draw) && draw->cur_blit_source !=
> -1) {
>num_to_consider = 1;
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl/x11: Remove unneeded free() on always null string

2017-12-06 Thread Vadim Shovkoplias 
Hi Eric,

I used smatch (http://smatch.sourceforge.net/). It is mainly used for Linux
kernel.

2017-12-04 18:52 GMT+02:00 Eric Engestrom <eric.engest...@imgtec.com>:

> On Monday, 2017-12-04 12:48:55 +0200, Vadim Shovkoplias wrote:
> > Hi Eric,
>
> Hey, sorry, I forgot to hit "send" on the reply I wrote on friday :]
>
> >
> > Mostly by a static analysis tool. It found at least 7 issues with useless
> > free() calls and other problems that probably should be fixed.
>
> What tool? It would be interesting for others to know what tools exist,
> especially when they find issues other tools didn't :)
>
> > Suggest please should I create one cumulative commit for this or it
> should
> > be a separate commits ?
>
> Same kind of issues in the same module should be grouped, whereas
> different kind of issues or different modules should be separate.
>
> Don't worry too much about it though, if people ask you to merge or
> split commits, it's not that complicated to do for a v2 :)
>
> >
> > 2017-12-01 17:41 GMT+02:00 Eric Engestrom <eric.engest...@imgtec.com>:
> >
> > > On Friday, 2017-12-01 17:08:53 +0200, vadim.shovkopl...@gmail.com
> wrote:
> > > > From: Vadym Shovkoplias <vadym.shovkopl...@globallogic.com>
> > > >
> > > > In this condition dri2_dpy->driver_name string always equals
> > > > NULL, so call to free() is useless
> > > >
> > > > Signed-off-by: Vadym Shovkoplias <vadym.shovkopl...@globallogic.com>
> > >
> > > Reviewed and pushed :)
> > >
> > > Are you finding all of these by inspection, or are you using a tool?
> > >
> > > > ---
> > > >  src/egl/drivers/dri2/platform_x11.c | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/src/egl/drivers/dri2/platform_x11.c
> b/src/egl/drivers/dri2/
> > > platform_x11.c
> > > > index c49cb1f..8ede590b 100644
> > > > --- a/src/egl/drivers/dri2/platform_x11.c
> > > > +++ b/src/egl/drivers/dri2/platform_x11.c
> > > > @@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display
> *dri2_dpy)
> > > >
> > > > if (dri2_dpy->driver_name == NULL) {
> > > >close(dri2_dpy->fd);
> > > > -  free(dri2_dpy->driver_name);
> > > >free(connect);
> > > >return EGL_FALSE;
> > > > }
> > > > --
> > > > 2.7.4
> > > >
> > >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl/x11: Remove unneeded free() on always null string

2017-12-04 Thread Vadim Shovkoplias 
Hi Eric,

Mostly by a static analysis tool. It found at least 7 issues with useless
free() calls and other problems that probably should be fixed.
Suggest please should I create one cumulative commit for this or it should
be a separate commits ?

2017-12-01 17:41 GMT+02:00 Eric Engestrom :

> On Friday, 2017-12-01 17:08:53 +0200, vadim.shovkopl...@gmail.com wrote:
> > From: Vadym Shovkoplias 
> >
> > In this condition dri2_dpy->driver_name string always equals
> > NULL, so call to free() is useless
> >
> > Signed-off-by: Vadym Shovkoplias 
>
> Reviewed and pushed :)
>
> Are you finding all of these by inspection, or are you using a tool?
>
> > ---
> >  src/egl/drivers/dri2/platform_x11.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/
> platform_x11.c
> > index c49cb1f..8ede590b 100644
> > --- a/src/egl/drivers/dri2/platform_x11.c
> > +++ b/src/egl/drivers/dri2/platform_x11.c
> > @@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
> >
> > if (dri2_dpy->driver_name == NULL) {
> >close(dri2_dpy->fd);
> > -  free(dri2_dpy->driver_name);
> >free(connect);
> >return EGL_FALSE;
> > }
> > --
> > 2.7.4
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] util/disk_cache: Remove unneeded free() on always null string

2017-12-04 Thread vadim . shovkoplias 
From: Vadym Shovkoplias 

At this point dc_job->cache_item_metadata.keys always equals
NULL, so call to free() is useless

Fixes: b86ecea3446 ("util/disk_cache: write cache item metadata to disk")
Signed-off-by: Vadym Shovkoplias 

---
 src/util/disk_cache.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index e954065..7ebfa8c 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -811,7 +811,6 @@ create_put_job(struct disk_cache *cache, const cache_key 
key,
return dc_job;
 
 fail:
-   free(dc_job->cache_item_metadata.keys);
free(dc_job);
 
return NULL;
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] egl/x11: Remove unneeded free() on always null string

2017-12-01 Thread vadim . shovkoplias 
From: Vadym Shovkoplias 

In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless

Signed-off-by: Vadym Shovkoplias 

---
 src/egl/drivers/dri2/platform_x11.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
 
if (dri2_dpy->driver_name == NULL) {
   close(dri2_dpy->fd);
-  free(dri2_dpy->driver_name);
   free(connect);
   return EGL_FALSE;
}
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glx/dri3: Remove unused deviceName variable

2017-12-01 Thread vadim . shovkoplias 
From: Vadym Shovkoplias 

deviceName string is declared, assigned and freed but actually
never used in dri3_create_screen() function.

Fixes: 2d94601582e ("Add DRI3+Present loader")

Signed-off-by: Vadym Shovkoplias 

---
 src/glx/dri3_glx.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index a10306f..f280a8c 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -800,7 +800,7 @@ dri3_create_screen(int screen, struct glx_display * priv)
struct dri3_screen *psc;
__GLXDRIscreen *psp;
struct glx_config *configs = NULL, *visuals = NULL;
-   char *driverName, *deviceName, *tmp;
+   char *driverName, *tmp;
int i;
unsigned char disable;
 
@@ -830,7 +830,6 @@ dri3_create_screen(int screen, struct glx_display * priv)
}
 
psc->fd = loader_get_user_preferred_fd(psc->fd, >is_different_gpu);
-   deviceName = NULL;
 
driverName = loader_get_driver_for_fd(psc->fd);
if (!driverName) {
@@ -956,7 +955,6 @@ dri3_create_screen(int screen, struct glx_display * priv)
   __glXEnableDirectExtension(>base, "GLX_EXT_buffer_age");
 
free(driverName);
-   free(deviceName);
 
tmp = getenv("LIBGL_SHOW_FPS");
psc->show_fps_interval = tmp ? atoi(tmp) : 0;
@@ -983,7 +981,6 @@ handle_error:
   dlclose(psc->driver);
 
free(driverName);
-   free(deviceName);
glx_screen_cleanup(>base);
free(psc);
 
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel/blorp: Fix possible NULL pointer dereferencing

2017-11-28 Thread vadim . shovkoplias 
From: Vadym Shovkoplias 

Fix incomplete check of input params in blorp_surf_convert_to_uncompressed()
which can lead to NULL pointer dereferencing.

---
 src/intel/blorp/blorp_blit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
index 9b5c1f1..8abd0db 100644
--- a/src/intel/blorp/blorp_blit.c
+++ b/src/intel/blorp/blorp_blit.c
@@ -2343,7 +2343,7 @@ blorp_surf_convert_to_uncompressed(const struct 
isl_device *isl_dev,
 */
blorp_surf_convert_to_single_slice(isl_dev, info);
 
-   if (width || height) {
+   if (width && height) {
 #ifndef NDEBUG
   uint32_t right_edge_px = info->tile_x_sa + *x + *width;
   uint32_t bottom_edge_px = info->tile_y_sa + *y + *height;
@@ -2356,7 +2356,7 @@ blorp_surf_convert_to_uncompressed(const struct 
isl_device *isl_dev,
   *height = DIV_ROUND_UP(*height, fmtl->bh);
}
 
-   if (x || y) {
+   if (x && y) {
   assert(*x % fmtl->bw == 0);
   assert(*y % fmtl->bh == 0);
   *x /= fmtl->bw;
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel/blorp: Fix possible NULL pointer dereferencing

2017-11-27 Thread vadim . shovkoplias 
From: Vadym Shovkoplias 

Fix incomplete check of input params in blorp_surf_convert_to_uncompressed()
which can lead to NULL pointer dereferencing.
---
 src/intel/blorp/blorp_blit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
index 9b5c1f1..8abd0db 100644
--- a/src/intel/blorp/blorp_blit.c
+++ b/src/intel/blorp/blorp_blit.c
@@ -2343,7 +2343,7 @@ blorp_surf_convert_to_uncompressed(const struct 
isl_device *isl_dev,
 */
blorp_surf_convert_to_single_slice(isl_dev, info);
 
-   if (width || height) {
+   if (width && height) {
 #ifndef NDEBUG
   uint32_t right_edge_px = info->tile_x_sa + *x + *width;
   uint32_t bottom_edge_px = info->tile_y_sa + *y + *height;
@@ -2356,7 +2356,7 @@ blorp_surf_convert_to_uncompressed(const struct 
isl_device *isl_dev,
   *height = DIV_ROUND_UP(*height, fmtl->bh);
}
 
-   if (x || y) {
+   if (x && y) {
   assert(*x % fmtl->bw == 0);
   assert(*y % fmtl->bh == 0);
   *x /= fmtl->bw;
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev