Re: [Mesa-dev] [PATCH v2 2/3] egl: return corresponding offset of EGLImage instead of 0.

2016-09-11 Thread Weng, Chuanbo
From functionality's perspective, version check is not necessary.
And I think the queryImage returns false if attribute is unsupported, so I 
prefer to not use version
Check.

Thanks,
Chuanbo Weng

-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
Axel Davy
Sent: Saturday, September 10, 2016 5:03 AM
To: Weng, Chuanbo ; Emil Velikov 

Cc: ML mesa-dev 
Subject: Re: [Mesa-dev] [PATCH v2 2/3] egl: return corresponding offset of 
EGLImage instead of 0.

I'm not sure calling queryImage with an unsupported attribute is legal, thus I 
think a small check doesn't hurt.

It'd give

if (offsets) {
offsets[0] = 0;
if (dri2_dpy->image->base.version >= 13) {
   EGLint img_offset = 0;
   bool ret = dri2_dpy->image->queryImage(dri2_img->dri_image,
  __DRI_IMAGE_ATTRIB_OFFSET, _offset);
   if (ret)
  offsets[0] = img_offset;
}
}
return EGL_TRUE;

or alternatively, if you think not being able to give the offset indicates an 
error,

if (offsets) {
offsets[0] = 0;
if (dri2_dpy->image->base.version >= 13) {
   EGLint img_offset = 0;
   bool ret = dri2_dpy->image->queryImage(dri2_img->dri_image,
  __DRI_IMAGE_ATTRIB_OFFSET, _offset);
   if (!ret)
  return EGL_FALSE;
   offsets[0] = img_offset;
}
}
return EGL_TRUE;

Axel Davy

On 09/09/2016 17:34, Weng, Chuanbo wrote:
> The comments from Emil can be summarized into the following code:
>
> if (offsets) {
>offsets[0] = 0;
>EGLint img_offset = 0;
>bool ret = dri2_dpy->image->queryImage(dri2_img->dri_image,
> __DRI_IMAGE_ATTRIB_OFFSET, _offset);
>if(ret == true)
>   offsets[0] = img_offset;
> }
>
> return EGL_TRUE;
>
> Emil, is it right?
>
> -Original Message-
> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> Sent: Friday, September 9, 2016 8:32 PM
> To: Weng, Chuanbo 
> Cc: ML mesa-dev ; Axel Davy 
> 
> Subject: Re: [PATCH v2 2/3] egl: return corresponding offset of EGLImage 
> instead of 0.
>
> On 9 September 2016 at 08:58, Chuanbo Weng  wrote:
>> The offset should not always be 0. For example, if EGLImage is 
>> created from a 2D texture with EGL_GL_TEXTURE_LEVEL=1, then the 
>> offset should be the actual start of miplevel 1 in bo.
>>
>> v2: Add version check of __DRIimageExtension implementation 
>> (Suggested by Axel Davy).
>>
> Just to elaborate what Axel was saying from another perspective:
>
> With current master the offset is explicitly zeroed, while with the
> (v2) patch you'll _only_ set the value when you have v13 driver.
> Thus using the loader + v12 driver you'll get a regression since a) 
> the offset will remain garbage and b) the function
> (dri2_export_dma_buf_image_mesa) will fail.
>
>
>> Signed-off-by: Chuanbo Weng 
>> ---
>>   src/egl/drivers/dri2/egl_dri2.c | 9 -
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.c 
>> b/src/egl/drivers/dri2/egl_dri2.c index 859612f..c529e55 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.c
>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>> @@ -2249,6 +2249,7 @@ dri2_export_dma_buf_image_mesa(_EGLDriver *drv, 
>> _EGLDisplay *disp, _EGLImage *im
>>  struct dri2_egl_image *dri2_img = dri2_egl_image(img);
>>
>>  (void) drv;
>> +   EGLint img_offset = 0;
>>
>>  /* rework later to provide multiple fds/strides/offsets */
>>  if (fds)
>> @@ -2259,8 +2260,14 @@ dri2_export_dma_buf_image_mesa(_EGLDriver *drv, 
>> _EGLDisplay *disp, _EGLImage *im
>> dri2_dpy->image->queryImage(dri2_img->dri_image,
>>__DRI_IMAGE_ATTRIB_STRIDE, 
>> strides);
>>
>> -   if (offsets)
>> +   if (offsets){
>> offsets[0] = 0;
>> +  if(dri2_dpy->image->base.version >= 13){
> Please move the variable declaration in local scope and add space 
> between ){
>
> Functionality wise we don't need the version check, esp. since you want to 
> set the offset only when queryImage() succeeds...
>
>> + dri2_dpy->image->queryImage(dri2_img->dri_image,
>> + __DRI_IMAGE_ATTRIB_OFFSET, _offset);
> ... which you don't seem to be checking here, so you'll effectively store/use 
> garbage.
>
> So unless Alex feels strongly for the version check I'd omit it, and fix the 
> rest of this patch.
>
> -Emil


___
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 01/20] gallium/radeon: derive buffer placement and flags only at initialization

2016-09-11 Thread Michel Dänzer
On 12/09/16 10:15 AM, Michel Dänzer wrote:
> On 30/08/16 12:28 AM, Marek Olšák wrote:
>> From: Marek Olšák 
>>
>> Invalidated buffers don't have to go through it.
>>
>> Split r600_init_resource into r600_init_resource_fields and
>> r600_alloc_resource.
> 
> This change caused Portal 2 to crash for me (with Kaveri using the
> radeon kernel driver),

On another machine using the amdgpu kernel driver (also with Kaveri), it
doesn't crash, but the intro videos and main menu background are
severely corrupted.


> see below. Let me know if you need more information.
> 
> 
> Thread 1 "portal2_linux" received signal SIGSEGV, Segmentation fault.
> 0xf4c3f500 in util_copy_rect (dst=0xe57fa000  address 0xe57fa000>, format=, dst_stride=, 
> dst_x=, dst_y=, width=640, height= out>, 
> src=0xe0fc000 '\200' ..., src_stride=, 
> src_x=, src_y=) at 
> ../../../../src/gallium/auxiliary/util/u_surface.c:105
> 105memcpy(dst, src, width);
> (gdb) bt
> #0  0xf4c3f500 in util_copy_rect (dst=0xe57fa000  at address 0xe57fa000>, format=, dst_stride=, 
> dst_x=, dst_y=, width=640, height= out>, 
> src=0xe0fc000 '\200' ..., src_stride=, 
> src_x=, src_y=) at 
> ../../../../src/gallium/auxiliary/util/u_surface.c:105
> #1  0xf4c3f616 in util_copy_box (dst=0xe57be000  at address 0xe57be000>, format=PIPE_FORMAT_L8_SRGB, dst_stride=768, 
> dst_slice_stride=276480, dst_x=0, dst_y=0, dst_z=0, width=640, height=360, 
> depth=1, 
> src=0xe0ca000 '\200' ..., src_stride=640, 
> src_slice_stride=230400, src_x=0, src_y=0, src_z=0) at 
> ../../../../src/gallium/auxiliary/util/u_surface.c:131
> #2  0xf4c4400e in u_default_texture_subdata (pipe=0xa18a000, 
> resource=0xd6d0800, level=0, usage=258, box=0xff826b38, data=0xe0ca000, 
> stride=640, layer_stride=230400) at 
> ../../../../src/gallium/auxiliary/util/u_transfer.c:67
> #3  0xf4a62123 in st_TexSubImage (ctx=0xa344000, dims=2, texImage=0xd666900, 
> xoffset=0, yoffset=0, zoffset=0, width=640, height=360, depth=1, format=6409, 
> type=5121, pixels=0xe0ca000, unpack=0xa35be3c) at 
> ../../../src/mesa/state_tracker/st_cb_texture.c:1400
> #4  0xf49ed48d in _mesa_texture_sub_image (ctx=0xa344000, dims=2, 
> texObj=0xd167b80, texImage=0xd666900, target=3553, level=0, xoffset=0, 
> yoffset=0, zoffset=0, width=640, height=360, depth=1, format=6409, type=5121, 
> pixels=0xe0ca000, dsa=false)
> at ../../../src/mesa/main/teximage.c:3239
> #5  0xf49ed696 in texsubimage (ctx=0xa344000, dims=2, target=3553, level=0, 
> xoffset=0, yoffset=0, zoffset=0, width=640, height=360, depth=1, format=6409, 
> type=5121, pixels=0xe0ca000, callerName=0xf4d9380a "glTexSubImage2D")
> at ../../../src/mesa/main/teximage.c:3297
> #6  0xf49ed98a in _mesa_TexSubImage2D (target=3553, level=0, xoffset=0, 
> yoffset=0, width=640, height=360, format=6409, type=5121, pixels=0xe0ca000) 
> at ../../../src/mesa/main/teximage.c:3438
> #7  0xf5792b98 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/libtogl.so
> #8  0xf5792db5 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/libtogl.so
> #9  0xf579f84b in IDirect3DSurface9::UnlockRect() () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/libtogl.so
> #10 0xed478ddb in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/shaderapidx9.so
> #11 0xed466cb3 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/shaderapidx9.so
> #12 0xedfdce2b in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/materialsystem.so
> #13 0xedfdda41 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/materialsystem.so
> #14 0xedfddbf8 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/materialsystem.so
> #15 0xedfddc62 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/materialsystem.so
> #16 0xed88050f in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/valve_avi.so
> #17 0xed8806c8 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/valve_avi.so
> #18 0xee87fd62 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #19 0xee87f316 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #20 0xee87ec53 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #21 0xee87ccf5 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #22 0xee87d5ff in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #23 0xee877536 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #24 0xee878890 in ?? () from 
> /home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/engine.so
> #25 0xf5d5ac00 in ?? ()
> #26 0xf5d5ac00 in ?? ()
> #27 0xf5d3e41a in ?? ()
> #28 0x08048564 in main ()
> 
> 
> 


-- 
Earthling Michel Dänzer   |   

Re: [Mesa-dev] Check about recent changes in amdgpu

2016-09-11 Thread Dave Airlie
On 10 September 2016 at 09:30, Mauro Rossi  wrote:
> Hi Dave,
>
> while checking mesa-dev with android build,
> I'm porting the necessary changes to to have amd/addrlib in Android build,
>
> but I have also encountered a building error related to commit
> https://cgit.freedesktop.org/mesa/mesa/commit/?id=1add3562e33f0234da50e54dda8cfa6dac613125
>
> In the attachment the full build log, under my signatue short error log.
>
> If I revert 1add3562e33f0234da50e54dda8cfa6dac613125
> and after that I modify src/amd/common/amdgpu_id.h
> in this way...
>
> -#include "util/u_endian.h"
> +#include "pipe/p_config.h"
>
> ...I can build mesa and complete Android 7.0, build,
> so there may be some part missing in the new util/u_endian.h
> header compared to the old one.

Does android have an endian.h? does the attached patch work?

Android seems to rely on the gallium hardcoding of architectures, which
I'd rather no port over to u_endian, but if the attached patch doesn't work,
then I'll do that.

Dave.


0001-u_endian-add-android-to-glibc-clause.patch
Description: application/binary
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/20] gallium/radeon: derive buffer placement and flags only at initialization

2016-09-11 Thread Michel Dänzer
On 30/08/16 12:28 AM, Marek Olšák wrote:
> From: Marek Olšák 
> 
> Invalidated buffers don't have to go through it.
> 
> Split r600_init_resource into r600_init_resource_fields and
> r600_alloc_resource.

This change caused Portal 2 to crash for me (with Kaveri using the
radeon kernel driver), see below. Let me know if you need more
information.


Thread 1 "portal2_linux" received signal SIGSEGV, Segmentation fault.
0xf4c3f500 in util_copy_rect (dst=0xe57fa000 , format=, dst_stride=, 
dst_x=, dst_y=, width=640, height=, 
src=0xe0fc000 '\200' ..., src_stride=, 
src_x=, src_y=) at 
../../../../src/gallium/auxiliary/util/u_surface.c:105
105  memcpy(dst, src, width);
(gdb) bt
#0  0xf4c3f500 in util_copy_rect (dst=0xe57fa000 , format=, dst_stride=, 
dst_x=, dst_y=, width=640, height=, 
src=0xe0fc000 '\200' ..., src_stride=, 
src_x=, src_y=) at 
../../../../src/gallium/auxiliary/util/u_surface.c:105
#1  0xf4c3f616 in util_copy_box (dst=0xe57be000 , format=PIPE_FORMAT_L8_SRGB, dst_stride=768, 
dst_slice_stride=276480, dst_x=0, dst_y=0, dst_z=0, width=640, height=360, 
depth=1, 
src=0xe0ca000 '\200' ..., src_stride=640, 
src_slice_stride=230400, src_x=0, src_y=0, src_z=0) at 
../../../../src/gallium/auxiliary/util/u_surface.c:131
#2  0xf4c4400e in u_default_texture_subdata (pipe=0xa18a000, 
resource=0xd6d0800, level=0, usage=258, box=0xff826b38, data=0xe0ca000, 
stride=640, layer_stride=230400) at 
../../../../src/gallium/auxiliary/util/u_transfer.c:67
#3  0xf4a62123 in st_TexSubImage (ctx=0xa344000, dims=2, texImage=0xd666900, 
xoffset=0, yoffset=0, zoffset=0, width=640, height=360, depth=1, format=6409, 
type=5121, pixels=0xe0ca000, unpack=0xa35be3c) at 
../../../src/mesa/state_tracker/st_cb_texture.c:1400
#4  0xf49ed48d in _mesa_texture_sub_image (ctx=0xa344000, dims=2, 
texObj=0xd167b80, texImage=0xd666900, target=3553, level=0, xoffset=0, 
yoffset=0, zoffset=0, width=640, height=360, depth=1, format=6409, type=5121, 
pixels=0xe0ca000, dsa=false)
at ../../../src/mesa/main/teximage.c:3239
#5  0xf49ed696 in texsubimage (ctx=0xa344000, dims=2, target=3553, level=0, 
xoffset=0, yoffset=0, zoffset=0, width=640, height=360, depth=1, format=6409, 
type=5121, pixels=0xe0ca000, callerName=0xf4d9380a "glTexSubImage2D")
at ../../../src/mesa/main/teximage.c:3297
#6  0xf49ed98a in _mesa_TexSubImage2D (target=3553, level=0, xoffset=0, 
yoffset=0, width=640, height=360, format=6409, type=5121, pixels=0xe0ca000) at 
../../../src/mesa/main/teximage.c:3438
#7  0xf5792b98 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/libtogl.so
#8  0xf5792db5 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/libtogl.so
#9  0xf579f84b in IDirect3DSurface9::UnlockRect() () from 
/home/daenzer/.steam/steam/steamapps/common/Portal 2/bin/libtogl.so
#10 0xed478ddb in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/shaderapidx9.so
#11 0xed466cb3 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/shaderapidx9.so
#12 0xedfdce2b in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/materialsystem.so
#13 0xedfdda41 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/materialsystem.so
#14 0xedfddbf8 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/materialsystem.so
#15 0xedfddc62 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/materialsystem.so
#16 0xed88050f in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/valve_avi.so
#17 0xed8806c8 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/valve_avi.so
#18 0xee87fd62 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#19 0xee87f316 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#20 0xee87ec53 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#21 0xee87ccf5 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#22 0xee87d5ff in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#23 0xee877536 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#24 0xee878890 in ?? () from /home/daenzer/.steam/steam/steamapps/common/Portal 
2/bin/engine.so
#25 0xf5d5ac00 in ?? ()
#26 0xf5d5ac00 in ?? ()
#27 0xf5d3e41a in ?? ()
#28 0x08048564 in main ()



-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 09/23] glsl: Convert constant_expression to the util hash table

2016-09-11 Thread Timothy Arceri
On Sun, 2016-09-11 at 15:31 +0200, Thomas Helland wrote:
> 2016-09-11 6:27 GMT+02:00 Timothy Arceri  m>:
> > 
> > On Sat, 2016-09-10 at 13:19 +0200, Thomas Helland wrote:
> > > 
> > > 2016-09-09 0:20 GMT+02:00 Thomas Helland  > > om>:
> > > > 
> > > > 
> > > > 2016-08-16 22:10 GMT+02:00 Thomas Helland  > > > l.co
> > > > m>:
> > > > > 
> > > > > 
> > > > > Signed-off-by: Thomas Helland 
> > > > > ---
> > > > >  src/compiler/glsl/ir_constant_expression.cpp | 24
> > > > > +-
> > > > > --
> > > > >  1 file changed, 13 insertions(+), 11 deletions(-)
> > > > > 
> > > > > diff --git a/src/compiler/glsl/ir_constant_expression.cpp
> > > > > b/src/compiler/glsl/ir_constant_expression.cpp
> > > > > index 6329acd..16c8fac 100644
> > > > > --- a/src/compiler/glsl/ir_constant_expression.cpp
> > > > > +++ b/src/compiler/glsl/ir_constant_expression.cpp
> > > > > @@ -39,7 +39,7 @@
> > > > >  #include "util/half_float.h"
> > > > >  #include "ir.h"
> > > > >  #include "compiler/glsl_types.h"
> > > > > -#include "program/hash_table.h"
> > > > > +#include "util/hash_table.h"
> > > > > 
> > > > >  static float
> > > > >  dot_f(ir_constant *op0, ir_constant *op1)
> > > > > @@ -457,7 +457,8 @@ constant_referenced(const ir_dereference
> > > > > *deref,
> > > > >    const ir_dereference_variable *const dv =
> > > > >   (const ir_dereference_variable *) deref;
> > > > > 
> > > > > -  store = (ir_constant *)
> > > > > hash_table_find(variable_context,
> > > > > dv->var);
> > > > > +  hash_entry *entry =
> > > > > _mesa_hash_table_search(variable_context, dv->var);
> > > > > +  store = (ir_constant *) entry->data;
> > > > >    break;
> > > > > }
> > > > > 
> > > > > @@ -1806,9 +1807,10 @@
> > > > > ir_dereference_variable::constant_expression_value(struct
> > > > > hash_table *variable_c
> > > > > 
> > > > > /* Give priority to the context hashtable, if it exists
> > > > > */
> > > > > if (variable_context) {
> > > > > -  ir_constant *value = (ir_constant
> > > > > *)hash_table_find(variable_context, var);
> > > > > -  if(value)
> > > > > - return value;
> > > > > +  hash_entry *entry =
> > > > > _mesa_hash_table_search(variable_context, var);
> > > > > +
> > > > > +  if(entry)
> > > > > + return (ir_constant *) entry->data;
> > > > > }
> > > > > 
> > > > > /* The constant_value of a uniform variable is its
> > > > > initializer,
> > > > > @@ -1926,7 +1928,7 @@ bool
> > > > > ir_function_signature::constant_expression_evaluate_expressio
> > > > > n_li
> > > > > st(const s
> > > > >   /* (declare () type symbol) */
> > > > >    case ir_type_variable: {
> > > > >   ir_variable *var = inst->as_variable();
> > > > > - hash_table_insert(variable_context,
> > > > > ir_constant::zero(this, var->type), var);
> > > > > + _mesa_hash_table_insert(variable_context, var,
> > > > > ir_constant::zero(this, var->type));
> > > > >   break;
> > > > >    }
> > > > > 
> > > > > @@ -2050,8 +2052,8 @@
> > > > > ir_function_signature::constant_expression_value(exec_list
> > > > > *actual_parameters, s
> > > > >  * We expect the correctness of the number of parameters
> > > > > to
> > > > > have
> > > > >  * been checked earlier.
> > > > >  */
> > > > > -   hash_table *deref_hash = hash_table_ctor(8,
> > > > > hash_table_pointer_hash,
> > > > > -hash_table_point
> > > > > er_c
> > > > > ompare);
> > > > > +   hash_table *deref_hash = _mesa_hash_table_create(NULL,
> > > > > _mesa_hash_pointer,
> > > > > +_mesa_ke
> > > > > y_po
> > > > > inter_equal);
> > > > > 
> > > > > /* If "origin" is non-NULL, then the function body is
> > > > > there.  So we
> > > > >  * have to use the variable objects from the object with
> > > > > the
> > > > > body,
> > > > > @@ -2062,13 +2064,13 @@
> > > > > ir_function_signature::constant_expression_value(exec_list
> > > > > *actual_parameters, s
> > > > > foreach_in_list(ir_rvalue, n, actual_parameters) {
> > > > >    ir_constant *constant = n-
> > > > > > 
> > > > > > constant_expression_value(variable_context);
> > > > >    if (constant == NULL) {
> > > > > - hash_table_dtor(deref_hash);
> > > > > + _mesa_hash_table_destroy(deref_hash, NULL);
> > > > >   return NULL;
> > > > >    }
> > > > > 
> > > > > 
> > > > >    ir_variable *var = (ir_variable *)parameter_info;
> > > > > -  hash_table_insert(deref_hash, constant, var);
> > > > > +  _mesa_hash_table_insert(deref_hash, constant, var);
> > > > 
> > > > This would be the cause of the regressions.
> > > > The API is inverted between the hash table implementations,
> > > > but the arguments here are not. No wonder weird things happen.
> > > > Will do a complete 

Re: [Mesa-dev] [PATCH 3/3] aubinator: only use program_invocation_short_name with glibc/cygwin

2016-09-11 Thread Timothy Arceri
On Thu, 2016-09-08 at 18:39 +0100, Emil Velikov wrote:
> On 1 September 2016 at 18:12, Jonathan Gray  wrote:
> > 
> > program_invocation_short_name is a gnu extension.  Limit use of it
> > to glibc and cygwin and otherwise use getprogname() which is
> > available
> > on BSD and OS X.
> > 
> > Signed-off-by: Jonathan Gray 
> > ---
> >  src/intel/tools/aubinator.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/intel/tools/aubinator.c
> > b/src/intel/tools/aubinator.c
> > index df84469..fe1f369 100644
> > --- a/src/intel/tools/aubinator.c
> > +++ b/src/intel/tools/aubinator.c
> > @@ -1014,6 +1014,12 @@ setup_pager(void)
> >  static void
> >  print_help(FILE *file)
> >  {
> > +   const char *progname;
> > +#if defined(__GLIBC__) || defined(__CYGWIN__)
> > +   progname = program_invocation_short_name;
> > +#else
> > +   progname = getprogname();
> > +#endif
> We could really fold the ~5 hunks somehow. Then again it shouldn't
> block this fix from getting it.
> 
> R-b and pushed the series. Thanks !
> Emil

This is causing a warning in GCC:

aubinator.c: In function ‘print_help’:
aubinator.c:1034:21: warning: passing argument 1 of ‘__xpg_basename’
discards ‘const’ qualifier from pointer target type [-Wdiscarded-
qualifiers]
basename(progname));
 ^~~~
In file included from aubinator.c:33:0:
/usr/include/libgen.h:34:14: note: expected ‘char *’ but argument is of
type ‘const char *’
 extern char *__xpg_basename (char *__path) __THROW;
  ^~
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] egl: fix gcc warning braces around scalar initializer

2016-09-11 Thread Timothy Arceri
---
 src/egl/main/eglcurrent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/egl/main/eglcurrent.c b/src/egl/main/eglcurrent.c
index 345f4cc..2a225bc 100644
--- a/src/egl/main/eglcurrent.c
+++ b/src/egl/main/eglcurrent.c
@@ -38,7 +38,7 @@
 
 /* This should be kept in sync with _eglInitThreadInfo() */
 #define _EGL_THREAD_INFO_INITIALIZER \
-   { EGL_SUCCESS, { NULL }, 0 }
+   { EGL_SUCCESS, NULL, 0 }
 
 /* a fallback thread info to guarantee that every thread always has one */
 static _EGLThreadInfo dummy_thread = _EGL_THREAD_INFO_INITIALIZER;
-- 
2.7.4

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


Re: [Mesa-dev] [PATCH] spirv/nir: Add support for OpAtomicLoad/Store

2016-09-11 Thread Timothy Arceri
On Wed, 2016-09-07 at 19:28 +0100, Lionel Landwerlin wrote:
> Hi Mark,
> 
> Thanks for the report.
> The assumption is that coord will be set on the image by a preceding
> SpvOpImageTexelPointer opcode (see top of the
> vtn_handle_image function).

I don't see how that can be right.

The image struct is declared on line 1663 after the handling of
the SpvOpImageTexelPointer opcode.

struct vtn_image_pointer image;

You then set image.image for the Atomic op codes. Neither image.coord
or image.sample are set for this code path.

As well as the Coverity issue there is also a GCC warning:

In file included from spirv/vtn_private.h:28:0,
 from spirv/spirv_to_nir.c:28:
spirv/spirv_to_nir.c: In function ‘vtn_handle_image’:
./nir/nir.h:571:11: warning: ‘image.coord’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
return src;
   ^~~
spirv/spirv_to_nir.c:1663:29: note: ‘image.coord’ was declared here
struct vtn_image_pointer image;
 ^
spirv/spirv_to_nir.c:1776:22: warning: ‘image.sample’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
   intrin->src[1] = nir_src_for_ssa(image.sample);


> 
> I don't think there are tests in the CTS exercising this at the
> moment
> :(
> 
> -
> Lionel
> 
> On Wed, 2016-09-07 at 10:14 -0700, Mark Janes wrote:
> > 
> > Hi Lionel,
> > 
> > This patch triggered CID 1372521 in Coverity.  Please let me know
> > if
> > my
> > the analysis is correct below:
> > 
> > Lionel Landwerlin  writes:
> > 
> > > 
> > > 
> > > Fixes new CTS tests :
> > > 
> > > dEQP-VK.spirv_assembly.instruction.compute.opatomic.load
> > > dEQP-VK.spirv_assembly.instruction.compute.opatomic.store
> > > 
> > > v2: don't handle images like ssbo/ubo (Jason)
> > > 
> > > Signed-off-by: Lionel Landwerlin 
> > > Cc: Jason Ekstrand 
> > > ---
> > >  src/compiler/spirv/spirv_to_nir.c | 124
> > > ++
> > >  1 file changed, 113 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/src/compiler/spirv/spirv_to_nir.c
> > > b/src/compiler/spirv/spirv_to_nir.c
> > > index fda38f9..1fcd70f 100644
> > > --- a/src/compiler/spirv/spirv_to_nir.c
> > > +++ b/src/compiler/spirv/spirv_to_nir.c
> > > @@ -1640,6 +1640,18 @@ vtn_handle_image(struct vtn_builder *b,
> > > SpvOp opcode,
> > >    image = *vtn_value(b, w[3], vtn_value_type_image_pointer)-
> > > > 
> > > > image;
> > >    break;
> > >  
> > > +   case SpvOpAtomicLoad: {
> > > +  image.image =
> > > + vtn_value(b, w[3], vtn_value_type_access_chain)-
> > > > 
> > > > access_chain;
> > > +  break;
> > > +   }
> > > +
> > > +   case SpvOpAtomicStore: {
> > > +  image.image =
> > > + vtn_value(b, w[1], vtn_value_type_access_chain)-
> > > > 
> > > > access_chain;
> > > +  break;
> > > +   }
> > 
> > These cases do not initialize image.coord, which is dereferenced on
> > line
> > 1773.  SpvOpImageQuerySize is a similar case which sets image.coord
> > to
> > NULL and is excepted from the code path at line 1773.
> > 
> > > 
> > > 
> > > case SpvOpImageQuerySize:
> > >    image.image =
> > >   vtn_value(b, w[3], vtn_value_type_access_chain)-
> > > > 
> > > > access_chain;
> > > @@ -1685,6 +1697,8 @@ vtn_handle_image(struct vtn_builder *b,
> > > SpvOp
> > > opcode,
> > > OP(ImageQuerySize, size)
> > > OP(ImageRead,  load)
> > > OP(ImageWrite, store)
> > > +   OP(AtomicLoad, load)
> > > +   OP(AtomicStore,store)
> > > OP(AtomicExchange, atomic_exchange)
> > > OP(AtomicCompareExchange,  atomic_comp_swap)
> > > OP(AtomicIIncrement,   atomic_add)
> > > @@ -1723,9 +1737,13 @@ vtn_handle_image(struct vtn_builder *b,
> > > SpvOp opcode,
> > > }
> > >  
> > > switch (opcode) {
> > > +   case SpvOpAtomicLoad:
> > > case SpvOpImageQuerySize:
> > > case SpvOpImageRead:
> > >    break;
> > > +   case SpvOpAtomicStore:
> > > +  intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[4])-
> > > > 
> > > > def);
> > > +  break;
> > > case SpvOpImageWrite:
> > >    intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[3])-
> > > > 
> > > > def);
> > >    break;
> > > @@ -1784,6 +1802,8 @@ static nir_intrinsic_op
> > >  get_ssbo_nir_atomic_op(SpvOp opcode)
> > >  {
> > > switch (opcode) {
> > > +   case SpvOpAtomicLoad:  return nir_intrinsic_load_ssbo;
> > > +   case SpvOpAtomicStore: return nir_intrinsic_store_ssbo;
> > >  #define OP(S, N) case SpvOp##S: return nir_intrinsic_ssbo_##N;
> > > OP(AtomicExchange, atomic_exchange)
> > > OP(AtomicCompareExchange,  atomic_comp_swap)
> > > @@ -1808,6 +1828,8 @@ static nir_intrinsic_op
> > >  get_shared_nir_atomic_op(SpvOp opcode)
> > >  {
> > > switch (opcode) {
> > > +   case SpvOpAtomicLoad:  return nir_intrinsic_load_var;
> > > + 

Re: [Mesa-dev] [PATCH 2/2] i965: remove unused variable at intel_miptree_create_for_teximage

2016-09-11 Thread Timothy Arceri
This patch is:

Reviewed-by: Timothy Arceri 

On Sun, 2016-09-11 at 00:07 +0200, Alejandro Piñeiro wrote:
> After commit "i965: Fix calculation of the image height at start
> level", it is
> not needed. This commit removes the "warning: unused variable ‘i’"
> warning.
> ---
>  src/mesa/drivers/dri/i965/intel_tex_image.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index a012927..f204db3 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -40,7 +40,6 @@ intel_miptree_create_for_teximage(struct
> brw_context *brw,
>  {
> GLuint lastLevel;
> int width, height, depth;
> -   GLuint i;
>  
> intel_get_image_dims(>base.Base, , ,
> );
>  
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 10/14] st/mesa: add support for dispatching a variable local size

2016-09-11 Thread Samuel Pitoiset
Signed-off-by: Samuel Pitoiset 
Reviewed-by: Marek Olšák 
---
 src/mesa/state_tracker/st_cb_compute.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_compute.c 
b/src/mesa/state_tracker/st_cb_compute.c
index 88c1ee2..ccc5dc2 100644
--- a/src/mesa/state_tracker/st_cb_compute.c
+++ b/src/mesa/state_tracker/st_cb_compute.c
@@ -36,6 +36,7 @@
 
 static void st_dispatch_compute_common(struct gl_context *ctx,
const GLuint *num_groups,
+   const GLuint *group_size,
struct pipe_resource *indirect,
GLintptr indirect_offset)
 {
@@ -56,7 +57,7 @@ static void st_dispatch_compute_common(struct gl_context *ctx,
   st_validate_state(st, ST_PIPELINE_COMPUTE);
 
for (unsigned i = 0; i < 3; i++) {
-  info.block[i] = prog->Comp.LocalSize[i];
+  info.block[i] = group_size ? group_size[i] : prog->Comp.LocalSize[i];
   info.grid[i]  = num_groups ? num_groups[i] : 0;
}
 
@@ -71,7 +72,7 @@ static void st_dispatch_compute_common(struct gl_context *ctx,
 static void st_dispatch_compute(struct gl_context *ctx,
 const GLuint *num_groups)
 {
-   st_dispatch_compute_common(ctx, num_groups, NULL, 0);
+   st_dispatch_compute_common(ctx, num_groups, NULL, NULL, 0);
 }
 
 static void st_dispatch_compute_indirect(struct gl_context *ctx,
@@ -80,11 +81,19 @@ static void st_dispatch_compute_indirect(struct gl_context 
*ctx,
struct gl_buffer_object *indirect_buffer = ctx->DispatchIndirectBuffer;
struct pipe_resource *indirect = st_buffer_object(indirect_buffer)->buffer;
 
-   st_dispatch_compute_common(ctx, NULL, indirect, indirect_offset);
+   st_dispatch_compute_common(ctx, NULL, NULL, indirect, indirect_offset);
+}
+
+static void st_dispatch_compute_group_size(struct gl_context *ctx,
+   const GLuint *num_groups,
+   const GLuint *group_size)
+{
+   st_dispatch_compute_common(ctx, num_groups, group_size, NULL, 0);
 }
 
 void st_init_compute_functions(struct dd_function_table *functions)
 {
functions->DispatchCompute = st_dispatch_compute;
functions->DispatchComputeIndirect = st_dispatch_compute_indirect;
+   functions->DispatchComputeGroupSize = st_dispatch_compute_group_size;
 }
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 12/14] nv50/ir: use 1024 threads/block for variable local size

2016-09-11 Thread Samuel Pitoiset
When a variable local size is defined as specified by
ARB_compute_variable_group_size, the fixed local size is set to 0
and a SIGFPE occurs when we compute the maximum number of regs.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_target.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h
index 4a701f7..0bb14ec 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target.h
@@ -174,7 +174,8 @@ public:
virtual void getBuiltinCode(const uint32_t **code, uint32_t *size) const = 
0;
 
virtual void parseDriverInfo(const struct nv50_ir_prog_info *info) {
-  threads = info->prop.cp.numThreads;
+  threads =
+ info->prop.cp.numThreads == 0 ? 1024 : info->prop.cp.numThreads;
}
 
virtual bool runLegalizePass(Program *, CGStage stage) const = 0;
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 08/14] gallium: add PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK

2016-09-11 Thread Samuel Pitoiset
Signed-off-by: Samuel Pitoiset 
---
 src/gallium/docs/source/screen.rst | 4 
 src/gallium/drivers/ilo/ilo_screen.c   | 2 ++
 src/gallium/drivers/nouveau/nv50/nv50_screen.c | 2 ++
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 2 ++
 src/gallium/drivers/radeon/r600_pipe_common.c  | 1 +
 src/gallium/drivers/softpipe/sp_screen.c   | 1 +
 src/gallium/include/pipe/p_defines.h   | 3 ++-
 7 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/gallium/docs/source/screen.rst 
b/src/gallium/docs/source/screen.rst
index 5dff650..cfc0a1b 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
@@ -498,6 +498,10 @@ pipe_screen::get_compute_param.
   threads. Also known as wavefront size, warp size or SIMD width.
 * ``PIPE_COMPUTE_CAP_ADDRESS_BITS``: The default compute device address space
   size specified as an unsigned integer value in bits.
+* ``PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK``: Maximum variable number
+  of threads that a single block can contain. This is similar to
+  PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK, except that the variable size is not
+  known a compile-time but at dispatch-time.
 
 .. _pipe_bind:
 
diff --git a/src/gallium/drivers/ilo/ilo_screen.c 
b/src/gallium/drivers/ilo/ilo_screen.c
index b9e5ad6..85357fa 100644
--- a/src/gallium/drivers/ilo/ilo_screen.c
+++ b/src/gallium/drivers/ilo/ilo_screen.c
@@ -303,6 +303,8 @@ ilo_get_compute_param(struct pipe_screen *screen,
   ptr = _size;
   size = sizeof(val.subgroup_size);
   break;
+   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK:
+  /* fallthrough */
default:
   ptr = NULL;
   size = 0;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c 
b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 1ec791d..6eb18ea 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -418,6 +418,8 @@ nv50_screen_get_compute_param(struct pipe_screen *pscreen,
   RET((uint32_t []) { 512 }); /* FIXME: arbitrary limit */
case PIPE_COMPUTE_CAP_ADDRESS_BITS:
   RET((uint32_t []) { 32 });
+   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK:
+  RET((uint64_t []) { 0 });
default:
   return 0;
}
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 1757cbb..df6c6af 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -478,6 +478,8 @@ nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   RET((uint32_t []) { 512 }); /* FIXME: arbitrary limit */
case PIPE_COMPUTE_CAP_ADDRESS_BITS:
   RET((uint32_t []) { 64 });
+   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK:
+  RET((uint64_t []) { 0 });
default:
   return 0;
}
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c 
b/src/gallium/drivers/radeon/r600_pipe_common.c
index 6d7cc1b..a7c6729 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.c
+++ b/src/gallium/drivers/radeon/r600_pipe_common.c
@@ -955,6 +955,7 @@ static int r600_get_compute_param(struct pipe_screen 
*screen,
}
return sizeof(uint32_t);
case PIPE_COMPUTE_CAP_MAX_PRIVATE_SIZE:
+   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK:
break; /* unused */
case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
if (ret) {
diff --git a/src/gallium/drivers/softpipe/sp_screen.c 
b/src/gallium/drivers/softpipe/sp_screen.c
index cd4269f..29e898b 100644
--- a/src/gallium/drivers/softpipe/sp_screen.c
+++ b/src/gallium/drivers/softpipe/sp_screen.c
@@ -522,6 +522,7 @@ softpipe_get_compute_param(struct pipe_screen *_screen,
case PIPE_COMPUTE_CAP_IMAGES_SUPPORTED:
case PIPE_COMPUTE_CAP_SUBGROUP_SIZE:
case PIPE_COMPUTE_CAP_ADDRESS_BITS:
+   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK;
   break;
}
return 0;
diff --git a/src/gallium/include/pipe/p_defines.h 
b/src/gallium/include/pipe/p_defines.h
index 88aa050..655995e 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -847,7 +847,8 @@ enum pipe_compute_cap
PIPE_COMPUTE_CAP_MAX_CLOCK_FREQUENCY,
PIPE_COMPUTE_CAP_MAX_COMPUTE_UNITS,
PIPE_COMPUTE_CAP_IMAGES_SUPPORTED,
-   PIPE_COMPUTE_CAP_SUBGROUP_SIZE
+   PIPE_COMPUTE_CAP_SUBGROUP_SIZE,
+   PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK,
 };
 
 /**
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 11/14] st/mesa: expose ARB_compute_variable_group_size

2016-09-11 Thread Samuel Pitoiset
This extension is only exposed if the underlying driver supports
ARB_compute_shader and if PIPE_COMPUTE_MAX_VARIABLE_THREADS_PER_BLOCK
is set.

v2: - expose the ext based on that new cap

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/state_tracker/st_extensions.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 807fbfb..ad7c637 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -1196,6 +1196,28 @@ void st_init_extensions(struct pipe_screen *screen,
  extensions->ARB_compute_shader =
   extensions->ARB_shader_image_load_store 
&&
   extensions->ARB_shader_atomic_counters;
+
+ if (extensions->ARB_compute_shader) {
+uint64_t max_variable_threads_per_block;
+
+screen->get_compute_param(screen, PIPE_SHADER_IR_TGSI,
+  
PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK,
+  _variable_threads_per_block);
+
+for (i = 0; i < 3; i++) {
+   /* Clamp the values to avoid having a local work group size
+* greater than the maximum number of invocations.
+*/
+   consts->MaxComputeVariableGroupSize[i] =
+  MIN2(consts->MaxComputeWorkGroupSize[i],
+   max_variable_threads_per_block);
+}
+consts->MaxComputeVariableGroupInvocations =
+   max_variable_threads_per_block;
+
+extensions->ARB_compute_variable_group_size =
+   max_variable_threads_per_block > 0;
+ }
   }
}
 
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 02/14] mesa/main: add support for ARB_compute_variable_groups_size

2016-09-11 Thread Samuel Pitoiset
v2: - update formatting spec quotations (Ian)
- move the total_invocations check outside of the loop (Ian)

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/api_validate.c | 96 
 src/mesa/main/api_validate.h |  4 ++
 src/mesa/main/compute.c  | 17 +++
 src/mesa/main/context.c  |  6 +++
 src/mesa/main/dd.h   |  9 
 src/mesa/main/extensions_table.h |  1 +
 src/mesa/main/get.c  | 12 +
 src/mesa/main/get_hash_params.py |  3 ++
 src/mesa/main/mtypes.h   | 24 +-
 src/mesa/main/shaderapi.c|  1 +
 src/mesa/main/shaderobj.c|  2 +
 11 files changed, 174 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index b35751e..62ac342 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -1096,6 +1096,7 @@ GLboolean
 _mesa_validate_DispatchCompute(struct gl_context *ctx,
const GLuint *num_groups)
 {
+   struct gl_shader_program *prog;
int i;
FLUSH_CURRENT(ctx, 0);
 
@@ -1128,6 +1129,88 @@ _mesa_validate_DispatchCompute(struct gl_context *ctx,
   }
}
 
+   /* The ARB_compute_variable_group_size spec says:
+*
+* "An INVALID_OPERATION error is generated by DispatchCompute if the active
+* program for the compute shader stage has a variable work group size."
+*/
+   prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
+   if (prog->Comp.LocalSizeVariable) {
+  _mesa_error(ctx, GL_INVALID_OPERATION,
+  "glDispatchCompute(variable work group size forbidden)");
+  return GL_FALSE;
+   }
+
+   return GL_TRUE;
+}
+
+GLboolean
+_mesa_validate_DispatchComputeGroupSizeARB(struct gl_context *ctx,
+   const GLuint *num_groups,
+   const GLuint *group_size)
+{
+   struct gl_shader_program *prog;
+   GLuint total_invocations = 1;
+   int i;
+
+   FLUSH_CURRENT(ctx, 0);
+
+   if (!check_valid_to_compute(ctx, "glDispatchComputeGroupSizeARB"))
+  return GL_FALSE;
+
+   for (i = 0; i < 3; i++) {
+  /* The ARB_compute_variable_group_size spec says:
+   *
+   * "An INVALID_VALUE error is generated by DispatchComputeGroupSizeARB if
+   * any of , , or  is less than
+   * or equal to zero or greater than the maximum local work group size for
+   * compute shaders with variable group size
+   * (MAX_COMPUTE_VARIABLE_GROUP_SIZE_ARB) in the corresponding dimension."
+   *
+   * However, the "less than" is a spec bug because they are declared as
+   * unsigned integers.
+   */
+  if (group_size[i] == 0 ||
+  group_size[i] > ctx->Const.MaxComputeVariableGroupSize[i]) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glDispatchComputeGroupSizeARB(group_size_%c)", 'x' + i);
+ return GL_FALSE;
+  }
+
+  /* The ARB_compute_variable_group_size spec says:
+   *
+   * "An INVALID_OPERATION error is generated by
+   * DispatchComputeGroupSizeARB if the active program for the compute
+   * shader stage has a fixed work group size."
+   */
+  prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
+  if (prog->Comp.LocalSize[i] != 0) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glDispatchComputeGroupSizeARB(fixed work group size "
+ "forbidden)");
+ return GL_FALSE;
+  }
+
+  total_invocations *= group_size[i];
+   }
+
+   /* The ARB_compute_variable_group_size spec says:
+*
+* "An INVALID_VALUE error is generated by DispatchComputeGroupSizeARB if
+* the product of , , and  exceeds
+* the implementation-dependent maximum local work group invocation count
+* for compute shaders with variable group size
+* (MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB)."
+*/
+   if (total_invocations > ctx->Const.MaxComputeVariableGroupInvocations) {
+  _mesa_error(ctx, GL_INVALID_VALUE,
+  "glDispatchComputeGroupSizeARB(product of local_sizes "
+  "exceeds MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB "
+  "(%d > %d))", total_invocations,
+  ctx->Const.MaxComputeVariableGroupInvocations);
+  return GL_FALSE;
+   }
+
return GL_TRUE;
 }
 
@@ -1137,6 +1220,7 @@ valid_dispatch_indirect(struct gl_context *ctx,
 GLsizei size, const char *name)
 {
const uint64_t end = (uint64_t) indirect + size;
+   struct gl_shader_program *prog;
 
if (!check_valid_to_compute(ctx, name))
   return GL_FALSE;
@@ -1182,6 +1266,18 @@ valid_dispatch_indirect(struct gl_context *ctx,
   return GL_FALSE;
}
 
+   /* The ARB_compute_variable_group_size spec says:
+*
+* "An INVALID_OPERATION error is generated if the active program for the
+* compute shader stage has 

[Mesa-dev] [PATCH v2 09/14] st/mesa: add mapping for SYSTEM_VALUE_LOCAL_GROUP_SIZE

2016-09-11 Thread Samuel Pitoiset
gl_LocalGroupSizeARB can be translated into TGSI_SEMANTIC_BLOCK_SIZE
which represents the block size in threads.

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Marek Olšák 
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 507a782..429f4b0 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -5235,6 +5235,8 @@ _mesa_sysval_to_semantic(unsigned sysval)
   return TGSI_SEMANTIC_BLOCK_ID;
case SYSTEM_VALUE_NUM_WORK_GROUPS:
   return TGSI_SEMANTIC_GRID_SIZE;
+   case SYSTEM_VALUE_LOCAL_GROUP_SIZE:
+  return TGSI_SEMANTIC_BLOCK_SIZE;
 
/* Unhandled */
case SYSTEM_VALUE_LOCAL_INVOCATION_INDEX:
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 13/14] nvc0: expose ARB_compute_variable_group_size

2016-09-11 Thread Samuel Pitoiset
Let's return the same number of threads per block for both fixed and
variable sizes.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index df6c6af..6540c31 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -446,6 +446,7 @@ nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   }
case PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE:
   RET(((uint64_t []) { 1024, 1024, 64 }));
+   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK:
case PIPE_COMPUTE_CAP_MAX_THREADS_PER_BLOCK:
   RET((uint64_t []) { 1024 });
case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE: /* g[] */
@@ -478,8 +479,6 @@ nvc0_screen_get_compute_param(struct pipe_screen *pscreen,
   RET((uint32_t []) { 512 }); /* FIXME: arbitrary limit */
case PIPE_COMPUTE_CAP_ADDRESS_BITS:
   RET((uint32_t []) { 64 });
-   case PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK:
-  RET((uint64_t []) { 0 });
default:
   return 0;
}
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 14/14] docs: mark ARB_compute_variable_group_size as done for nvc0

2016-09-11 Thread Samuel Pitoiset
Signed-off-by: Samuel Pitoiset 
---
 docs/features.txt | 2 +-
 docs/relnotes/12.1.0.html | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/features.txt b/docs/features.txt
index 690c160..3825943 100644
--- a/docs/features.txt
+++ b/docs/features.txt
@@ -279,7 +279,7 @@ Khronos, ARB, and OES extensions that are not part of any 
OpenGL or OpenGL ES ve
 
   GL_ARB_bindless_texture   started (airlied)
   GL_ARB_cl_event   not started
-  GL_ARB_compute_variable_group_sizenot started
+  GL_ARB_compute_variable_group_sizeDONE (nvc0)
   GL_ARB_ES3_2_compatibilitynot started
   GL_ARB_fragment_shader_interlock  not started
   GL_ARB_gpu_shader_int64   started (airlied for 
core and Gallium, idr for i965)
diff --git a/docs/relnotes/12.1.0.html b/docs/relnotes/12.1.0.html
index 3368ebc..cb78f35 100644
--- a/docs/relnotes/12.1.0.html
+++ b/docs/relnotes/12.1.0.html
@@ -47,6 +47,7 @@ Note: some of the new features are only available with 
certain drivers.
 OpenGL ES 3.1 on i965/hsw
 GL_ARB_ES3_1_compatibility on i965
 GL_ARB_clear_texture on r600, radeonsi
+GL_ARB_compute_variable_group_size on nvc0
 GL_ARB_cull_distance on radeonsi
 GL_ARB_enhanced_layouts on i965
 GL_ARB_indirect_parameters on radeonsi
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 00/14] add support for ARB_compute_variable_group_size

2016-09-11 Thread Samuel Pitoiset
v2: - update formatting spec quotations
- add PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK
- expose the ext based on that new cap
- add missing relnotes
- various cosmetic changes

From original cover-letter:

Hi,

This series implements ARB_compute_variable_group_size written against GL 4.3.
This extension allows to dispatch variable work group size via a new function
called glDispatchComputeGroupSizeARB().

Because this extension is pretty similar to ARB_compute_shader, all Gallium
drivers which already support compute shaders will expose
ARB_compute_variable_group_size with that series.

I did write a bunch of piglit tests, have a look here if you want:
https://lists.freedesktop.org/archives/piglit/2016-September/020755.html

All tests pass on Fermi (GF119) as well as all previous compute shaders tests.

Marek, Nicolai and other AMD folks, I don't know if radeonsi will need a fix
somewhere for handling a variable work group size, but as I don't have the
hardware, I can't test. Let me know if something needs to be slighty updated.

Please review,
Thanks!

Samuel Pitoiset (14):
  glapi: add entry points for GL_ARB_compute_variable_group_size
  mesa/main: add support for ARB_compute_variable_groups_size
  glsl: add enable flags for ARB_compute_variable_group_size
  glsl: process local_size_variable input qualifier
  glsl: reject compute shaders with fixed and variable local size
  glsl/linker: handle errors when a variable local size is used
  glsl: add gl_LocalGroupSizeARB as a system value
  gallium: add PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK
  st/mesa: add mapping for SYSTEM_VALUE_LOCAL_GROUP_SIZE
  st/mesa: add support for dispatching a variable local size
  st/mesa: expose ARB_compute_variable_group_size
  nv50/ir: use 1024 threads/block for variable local size
  nvc0: expose ARB_compute_variable_group_size
  docs: mark ARB_compute_variable_group_size as done for nvc0

 docs/features.txt  |  2 +-
 docs/relnotes/12.1.0.html  |  1 +
 src/compiler/glsl/ast.h|  5 ++
 src/compiler/glsl/ast_to_hir.cpp   | 14 
 src/compiler/glsl/ast_type.cpp |  6 ++
 src/compiler/glsl/builtin_variables.cpp|  6 ++
 src/compiler/glsl/glsl_parser.yy   | 13 +++
 src/compiler/glsl/glsl_parser_extras.cpp   |  7 ++
 src/compiler/glsl/glsl_parser_extras.h |  8 ++
 src/compiler/glsl/linker.cpp   | 25 +-
 src/compiler/glsl/standalone.cpp   |  4 +
 src/compiler/glsl/standalone_scaffolding.cpp   |  5 ++
 src/compiler/shader_enums.h|  1 +
 src/gallium/docs/source/screen.rst |  4 +
 src/gallium/drivers/ilo/ilo_screen.c   |  2 +
 .../drivers/nouveau/codegen/nv50_ir_target.h   |  3 +-
 src/gallium/drivers/nouveau/nv50/nv50_screen.c |  2 +
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c |  1 +
 src/gallium/drivers/radeon/r600_pipe_common.c  |  1 +
 src/gallium/drivers/softpipe/sp_screen.c   |  1 +
 src/gallium/include/pipe/p_defines.h   |  3 +-
 .../glapi/gen/ARB_compute_variable_group_size.xml  | 25 ++
 src/mapi/glapi/gen/Makefile.am |  1 +
 src/mapi/glapi/gen/gl_API.xml  |  4 +-
 src/mesa/main/api_validate.c   | 96 ++
 src/mesa/main/api_validate.h   |  4 +
 src/mesa/main/compute.c| 25 ++
 src/mesa/main/compute.h|  5 ++
 src/mesa/main/context.c|  6 ++
 src/mesa/main/dd.h |  9 ++
 src/mesa/main/extensions_table.h   |  1 +
 src/mesa/main/get.c| 12 +++
 src/mesa/main/get_hash_params.py   |  3 +
 src/mesa/main/mtypes.h | 24 +-
 src/mesa/main/shaderapi.c  |  1 +
 src/mesa/main/shaderobj.c  |  2 +
 src/mesa/main/tests/dispatch_sanity.cpp|  3 +
 src/mesa/state_tracker/st_cb_compute.c | 15 +++-
 src/mesa/state_tracker/st_extensions.c | 22 +
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  2 +
 40 files changed, 364 insertions(+), 10 deletions(-)
 create mode 100644 src/mapi/glapi/gen/ARB_compute_variable_group_size.xml

-- 
2.9.3

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


[Mesa-dev] [PATCH v2 05/14] glsl: reject compute shaders with fixed and variable local size

2016-09-11 Thread Samuel Pitoiset
The ARB_compute_variable_group_size specification explains that
when a compute shader includes both a fixed and a variable local
size, a compile-time error occurs.

v2: - update formatting spec quotations (Ian)

Signed-off-by: Samuel Pitoiset 
---
 src/compiler/glsl/ast_to_hir.cpp | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 4fc4c5c..7e737c0 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -8013,6 +8013,20 @@ ast_cs_input_layout::hir(exec_list *instructions,
   }
}
 
+   /* The ARB_compute_variable_group_size spec says:
+*
+* If a compute shader including a *local_size_variable* qualifier also
+* declares a fixed local group size using the *local_size_x*,
+* *local_size_y*, or *local_size_z* qualifiers, a compile-time error
+* results
+*/
+   if (state->cs_input_local_size_variable_specified) {
+  _mesa_glsl_error(, state,
+   "compute shader can't include both a variable and a "
+   "fixed local group size");
+  return NULL;
+   }
+
state->cs_input_local_size_specified = true;
for (int i = 0; i < 3; i++)
   state->cs_input_local_size[i] = qual_local_size[i];
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 07/14] glsl: add gl_LocalGroupSizeARB as a system value

2016-09-11 Thread Samuel Pitoiset
v2: - only add it if the ext is enabled (Ilia)

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Ian Romanick 
---
 src/compiler/glsl/builtin_variables.cpp | 6 ++
 src/compiler/shader_enums.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/src/compiler/glsl/builtin_variables.cpp 
b/src/compiler/glsl/builtin_variables.cpp
index f47daab..3a339a7 100644
--- a/src/compiler/glsl/builtin_variables.cpp
+++ b/src/compiler/glsl/builtin_variables.cpp
@@ -1236,6 +1236,12 @@ builtin_variable_generator::generate_cs_special_vars()
 "gl_LocalInvocationID");
add_system_value(SYSTEM_VALUE_WORK_GROUP_ID, uvec3_t, "gl_WorkGroupID");
add_system_value(SYSTEM_VALUE_NUM_WORK_GROUPS, uvec3_t, "gl_NumWorkGroups");
+
+   if (state->ARB_compute_variable_group_size_enable) {
+  add_system_value(SYSTEM_VALUE_LOCAL_GROUP_SIZE,
+   uvec3_t, "gl_LocalGroupSizeARB");
+   }
+
if (state->ctx->Const.LowerCsDerivedVariables) {
   add_variable("gl_GlobalInvocationID", uvec3_t, ir_var_auto, 0);
   add_variable("gl_LocalInvocationIndex", uint_t, ir_var_auto, 0);
diff --git a/src/compiler/shader_enums.h b/src/compiler/shader_enums.h
index c3a62e0..b6e048e 100644
--- a/src/compiler/shader_enums.h
+++ b/src/compiler/shader_enums.h
@@ -472,6 +472,7 @@ typedef enum
SYSTEM_VALUE_GLOBAL_INVOCATION_ID,
SYSTEM_VALUE_WORK_GROUP_ID,
SYSTEM_VALUE_NUM_WORK_GROUPS,
+   SYSTEM_VALUE_LOCAL_GROUP_SIZE,
/*@}*/
 
/**
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 04/14] glsl: process local_size_variable input qualifier

2016-09-11 Thread Samuel Pitoiset
This is the new layout qualifier introduced by
ARB_compute_variable_group_size which allows to use a variable work
group size.

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Ian Romanick 
---
 src/compiler/glsl/ast.h  |  5 +
 src/compiler/glsl/ast_type.cpp   |  6 ++
 src/compiler/glsl/glsl_parser.yy | 13 +
 src/compiler/glsl/glsl_parser_extras.cpp |  6 ++
 src/compiler/glsl/glsl_parser_extras.h   |  6 ++
 5 files changed, 36 insertions(+)

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index 4c648d0..55f009a 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -553,6 +553,11 @@ struct ast_type_qualifier {
   */
  unsigned local_size:3;
 
+/** \name Layout qualifiers for ARB_compute_variable_group_size. */
+/** \{ */
+unsigned local_size_variable:1;
+/** \} */
+
 /** \name Layout and memory qualifiers for 
ARB_shader_image_load_store. */
 /** \{ */
 unsigned early_fragment_tests:1;
diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
index f3f6b29..3f19f1f 100644
--- a/src/compiler/glsl/ast_type.cpp
+++ b/src/compiler/glsl/ast_type.cpp
@@ -503,6 +503,7 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc,
  state->in_qualifier->flags.q.local_size == 0;
 
   valid_in_mask.flags.q.local_size = 7;
+  valid_in_mask.flags.q.local_size_variable = 1;
   break;
default:
   _mesa_glsl_error(loc, state,
@@ -580,6 +581,10 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc,
   this->point_mode = q.point_mode;
}
 
+   if (q.flags.q.local_size_variable) {
+  state->cs_input_local_size_variable_specified = true;
+   }
+
if (create_node) {
   if (create_gs_ast) {
  node = new(mem_ctx) ast_gs_input_layout(*loc, q.prim_type);
@@ -653,6 +658,7 @@ ast_type_qualifier::validate_flags(YYLTYPE *loc,
 bad.flags.q.prim_type ? " prim_type" : "",
 bad.flags.q.max_vertices ? " max_vertices" : "",
 bad.flags.q.local_size ? " local_size" : "",
+bad.flags.q.local_size_variable ? " local_size_variable" : 
"",
 bad.flags.q.early_fragment_tests ? " early_fragment_tests" 
: "",
 bad.flags.q.explicit_image_format ? " image_format" : "",
 bad.flags.q.coherent ? " coherent" : "",
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index 9e1fd9e..38cbd3f 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -1491,6 +1491,19 @@ layout_qualifier_id:
  }
   }
 
+  /* Layout qualifiers for ARB_compute_variable_group_size. */
+  if (!$$.flags.i) {
+ if (match_layout_qualifier($1, "local_size_variable", state) == 0) {
+$$.flags.q.local_size_variable = 1;
+ }
+
+ if ($$.flags.i && !state->ARB_compute_variable_group_size_enable) {
+_mesa_glsl_error(& @1, state,
+ "qualifier `local_size_variable` requires "
+ "ARB_compute_variable_group_size");
+ }
+  }
+
   if (!$$.flags.i) {
  _mesa_glsl_error(& @1, state, "unrecognized layout identifier "
   "`%s'", $1);
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index bcbe623..44d241d 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -297,6 +297,8 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
gl_context *_ctx,
   sizeof(this->atomic_counter_offsets));
this->allow_extension_directive_midshader =
   ctx->Const.AllowGLSLExtensionDirectiveMidShader;
+
+   this->cs_input_local_size_variable_specified = false;
 }
 
 /**
@@ -1648,6 +1650,7 @@ set_shader_inout_layout(struct gl_shader *shader,
if (shader->Stage != MESA_SHADER_COMPUTE) {
   /* Should have been prevented by the parser. */
   assert(!state->cs_input_local_size_specified);
+  assert(!state->cs_input_local_size_variable_specified);
}
 
if (shader->Stage != MESA_SHADER_FRAGMENT) {
@@ -1763,6 +1766,9 @@ set_shader_inout_layout(struct gl_shader *shader,
  for (int i = 0; i < 3; i++)
 shader->info.Comp.LocalSize[i] = 0;
   }
+
+  shader->info.Comp.LocalSizeVariable =
+ state->cs_input_local_size_variable_specified;
   break;
 
case MESA_SHADER_FRAGMENT:
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index 8e0dafe..c1cf789 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -405,6 +405,12 @@ struct _mesa_glsl_parse_state {
unsigned cs_input_local_size[3];
 
/**
+* True if a compute shader input 

[Mesa-dev] [PATCH v2 01/14] glapi: add entry points for GL_ARB_compute_variable_group_size

2016-09-11 Thread Samuel Pitoiset
v2: - correctly sort that new extension (Ian)
- fix up the comment (Ian)

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Ian Romanick 
---
 .../glapi/gen/ARB_compute_variable_group_size.xml  | 25 ++
 src/mapi/glapi/gen/Makefile.am |  1 +
 src/mapi/glapi/gen/gl_API.xml  |  4 +++-
 src/mesa/main/compute.c|  8 +++
 src/mesa/main/compute.h|  5 +
 src/mesa/main/tests/dispatch_sanity.cpp|  3 +++
 6 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 src/mapi/glapi/gen/ARB_compute_variable_group_size.xml

diff --git a/src/mapi/glapi/gen/ARB_compute_variable_group_size.xml 
b/src/mapi/glapi/gen/ARB_compute_variable_group_size.xml
new file mode 100644
index 000..b21c52f
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_compute_variable_group_size.xml
@@ -0,0 +1,25 @@
+
+
+
+
+
+
+
+
+
+  
+  
+  
+  
+
+  
+
+
+
+
+
+
+  
+
+
+
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 0d7c338..49fdfe3 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -117,6 +117,7 @@ API_XML = \
ARB_color_buffer_float.xml \
ARB_compressed_texture_pixel_storage.xml \
ARB_compute_shader.xml \
+   ARB_compute_variable_group_size.xml \
ARB_copy_buffer.xml \
ARB_copy_image.xml \
ARB_debug_output.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index c39aa22..4d2d851 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8254,7 +8254,9 @@
 
 
 
-
+
+
+http://www.w3.org/2001/XInclude"/>
 
 http://www.w3.org/2001/XInclude"/>
 
diff --git a/src/mesa/main/compute.c b/src/mesa/main/compute.c
index b71430f..b052bae 100644
--- a/src/mesa/main/compute.c
+++ b/src/mesa/main/compute.c
@@ -60,3 +60,11 @@ _mesa_DispatchComputeIndirect(GLintptr indirect)
 
ctx->Driver.DispatchComputeIndirect(ctx, indirect);
 }
+
+void GLAPIENTRY
+_mesa_DispatchComputeGroupSizeARB(GLuint num_groups_x, GLuint num_groups_y,
+  GLuint num_groups_z, GLuint group_size_x,
+  GLuint group_size_y, GLuint group_size_z)
+{
+
+}
diff --git a/src/mesa/main/compute.h b/src/mesa/main/compute.h
index 0cc034f..8018bbb 100644
--- a/src/mesa/main/compute.h
+++ b/src/mesa/main/compute.h
@@ -35,4 +35,9 @@ _mesa_DispatchCompute(GLuint num_groups_x,
 extern void GLAPIENTRY
 _mesa_DispatchComputeIndirect(GLintptr indirect);
 
+extern void GLAPIENTRY
+_mesa_DispatchComputeGroupSizeARB(GLuint num_groups_x, GLuint num_groups_y,
+  GLuint num_groups_z, GLuint group_size_x,
+  GLuint group_size_y, GLuint group_size_z);
+
 #endif
diff --git a/src/mesa/main/tests/dispatch_sanity.cpp 
b/src/mesa/main/tests/dispatch_sanity.cpp
index 42fe61a..7faeabe 100644
--- a/src/mesa/main/tests/dispatch_sanity.cpp
+++ b/src/mesa/main/tests/dispatch_sanity.cpp
@@ -942,6 +942,9 @@ const struct function common_desktop_functions_possible[] = 
{
{ "glDispatchCompute", 43, -1 },
{ "glDispatchComputeIndirect", 43, -1 },
 
+   /* GL_ARB_compute_variable_group_size */
+   { "glDispatchComputeGroupSizeARB", 43, -1 },
+
/* GL_EXT_polygon_offset_clamp */
{ "glPolygonOffsetClampEXT", 11, -1 },
 
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 06/14] glsl/linker: handle errors when a variable local size is used

2016-09-11 Thread Samuel Pitoiset
Compute shaders can now include a fixed local size as defined by
ARB_compute_shader or a variable size as defined by
ARB_compute_variable_group_size.

v2: - update formatting spec quotations (Ian)
- various cosmetic changes (Ian)

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Ian Romanick 
---
 src/compiler/glsl/linker.cpp | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index c95edf3..f740de1 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -2075,6 +2075,8 @@ link_cs_input_layout_qualifiers(struct gl_shader_program 
*prog,
for (int i = 0; i < 3; i++)
   linked_shader->info.Comp.LocalSize[i] = 0;
 
+   linked_shader->info.Comp.LocalSizeVariable = false;
+
/* This function is called for all shader stages, but it only has an effect
 * for compute shaders.
 */
@@ -2109,6 +2111,20 @@ link_cs_input_layout_qualifiers(struct gl_shader_program 
*prog,
 linked_shader->info.Comp.LocalSize[i] =
shader->info.Comp.LocalSize[i];
  }
+  } else if (shader->info.Comp.LocalSizeVariable) {
+ if (linked_shader->info.Comp.LocalSize[0] != 0) {
+/* The ARB_compute_variable_group_size spec says:
+ *
+ * If one compute shader attached to a program declares a
+ * variable local group size and a second compute shader
+ * attached to the same program declares a fixed local group
+ * size, a link-time error results.
+ */
+linker_error(prog, "compute shader defined with both fixed and "
+ "variable local group size\n");
+return;
+ }
+ linked_shader->info.Comp.LocalSizeVariable = true;
   }
}
 
@@ -2116,12 +2132,17 @@ link_cs_input_layout_qualifiers(struct 
gl_shader_program *prog,
 * since we already know we're in the right type of shader program
 * for doing it.
 */
-   if (linked_shader->info.Comp.LocalSize[0] == 0) {
-  linker_error(prog, "compute shader didn't declare local size\n");
+   if (linked_shader->info.Comp.LocalSize[0] == 0 &&
+   !linked_shader->info.Comp.LocalSizeVariable) {
+  linker_error(prog, "compute shader must contain a fixed or a variable "
+ "local group size\n");
   return;
}
for (int i = 0; i < 3; i++)
   prog->Comp.LocalSize[i] = linked_shader->info.Comp.LocalSize[i];
+
+   prog->Comp.LocalSizeVariable =
+  linked_shader->info.Comp.LocalSizeVariable;
 }
 
 
-- 
2.9.3

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


[Mesa-dev] [PATCH v2 03/14] glsl: add enable flags for ARB_compute_variable_group_size

2016-09-11 Thread Samuel Pitoiset
This also initializes the default values for the standalone compiler.

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Ian Romanick 
---
 src/compiler/glsl/glsl_parser_extras.cpp | 1 +
 src/compiler/glsl/glsl_parser_extras.h   | 2 ++
 src/compiler/glsl/standalone.cpp | 4 
 src/compiler/glsl/standalone_scaffolding.cpp | 5 +
 4 files changed, 12 insertions(+)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 436ddd0..bcbe623 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -580,6 +580,7 @@ static const _mesa_glsl_extension 
_mesa_glsl_supported_extensions[] = {
EXT(ARB_ES3_2_compatibility),
EXT(ARB_arrays_of_arrays),
EXT(ARB_compute_shader),
+   EXT(ARB_compute_variable_group_size),
EXT(ARB_conservative_depth),
EXT(ARB_cull_distance),
EXT(ARB_derivative_control),
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index e146fe1..8e0dafe 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -576,6 +576,8 @@ struct _mesa_glsl_parse_state {
bool ARB_arrays_of_arrays_warn;
bool ARB_compute_shader_enable;
bool ARB_compute_shader_warn;
+   bool ARB_compute_variable_group_size_enable;
+   bool ARB_compute_variable_group_size_warn;
bool ARB_conservative_depth_enable;
bool ARB_conservative_depth_warn;
bool ARB_cull_distance_enable;
diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp
index 88fe5fd..cb2da03 100644
--- a/src/compiler/glsl/standalone.cpp
+++ b/src/compiler/glsl/standalone.cpp
@@ -58,6 +58,10 @@ initialize_context(struct gl_context *ctx, gl_api api)
ctx->Const.MaxComputeWorkGroupSize[2] = 64;
ctx->Const.MaxComputeWorkGroupInvocations = 1024;
ctx->Const.MaxComputeSharedMemorySize = 32768;
+   ctx->Const.MaxComputeVariableGroupSize[0] = 512;
+   ctx->Const.MaxComputeVariableGroupSize[1] = 512;
+   ctx->Const.MaxComputeVariableGroupSize[2] = 64;
+   ctx->Const.MaxComputeVariableGroupInvocations = 512;
ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits = 16;
ctx->Const.Program[MESA_SHADER_COMPUTE].MaxUniformComponents = 1024;
ctx->Const.Program[MESA_SHADER_COMPUTE].MaxCombinedUniformComponents = 1024;
diff --git a/src/compiler/glsl/standalone_scaffolding.cpp 
b/src/compiler/glsl/standalone_scaffolding.cpp
index b0fb4b7..decff5f 100644
--- a/src/compiler/glsl/standalone_scaffolding.cpp
+++ b/src/compiler/glsl/standalone_scaffolding.cpp
@@ -144,6 +144,7 @@ void initialize_context_to_defaults(struct gl_context *ctx, 
gl_api api)
ctx->Extensions.dummy_false = false;
ctx->Extensions.dummy_true = true;
ctx->Extensions.ARB_compute_shader = true;
+   ctx->Extensions.ARB_compute_variable_group_size = true;
ctx->Extensions.ARB_conservative_depth = true;
ctx->Extensions.ARB_draw_instanced = true;
ctx->Extensions.ARB_ES2_compatibility = true;
@@ -207,6 +208,10 @@ void initialize_context_to_defaults(struct gl_context 
*ctx, gl_api api)
ctx->Const.MaxComputeWorkGroupSize[1] = 1024;
ctx->Const.MaxComputeWorkGroupSize[2] = 64;
ctx->Const.MaxComputeWorkGroupInvocations = 1024;
+   ctx->Const.MaxComputeVariableGroupSize[0] = 512;
+   ctx->Const.MaxComputeVariableGroupSize[1] = 512;
+   ctx->Const.MaxComputeVariableGroupSize[2] = 64;
+   ctx->Const.MaxComputeVariableGroupInvocations = 512;
ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits = 16;
ctx->Const.Program[MESA_SHADER_COMPUTE].MaxUniformComponents = 1024;
ctx->Const.Program[MESA_SHADER_COMPUTE].MaxInputComponents = 0; /* not used 
*/
-- 
2.9.3

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


Re: [Mesa-dev] [PATCH 02/11] mesa/main: add support for ARB_compute_variable_groups_size

2016-09-11 Thread Samuel Pitoiset



On 09/08/2016 10:58 PM, Ian Romanick wrote:

On 09/08/2016 01:31 PM, Samuel Pitoiset wrote:

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/api_validate.c | 94 
 src/mesa/main/api_validate.h |  4 ++
 src/mesa/main/compute.c  | 17 
 src/mesa/main/context.c  |  6 +++
 src/mesa/main/dd.h   |  9 
 src/mesa/main/extensions_table.h |  1 +
 src/mesa/main/get.c  | 12 +
 src/mesa/main/get_hash_params.py |  3 ++
 src/mesa/main/mtypes.h   | 23 +-
 src/mesa/main/shaderapi.c|  1 +
 src/mesa/main/shaderobj.c|  2 +
 11 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index b35751e..9379015 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -1096,6 +1096,7 @@ GLboolean
 _mesa_validate_DispatchCompute(struct gl_context *ctx,
const GLuint *num_groups)
 {
+   struct gl_shader_program *prog;
int i;
FLUSH_CURRENT(ctx, 0);

@@ -1128,6 +1129,86 @@ _mesa_validate_DispatchCompute(struct gl_context *ctx,
   }
}

+   /* From the ARB_compute_variable_group_size specification:
+*
+* "An INVALID_OPERATION error is generated by DispatchCompute if the active
+* program for the compute shader stage has a variable work group size."
+*/


There has been a lot of debate about formatting spec quotations.  The
one thing where I think everyone agrees is formatting the first like.
Please use

The ARB_compute_variable_group_size spec says:

That makes it much easier to grep the code for spec quotations.

This comment applies to the spec quotations below too.


+   prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
+   if (prog->Comp.LocalSizeVariable) {
+  _mesa_error(ctx, GL_INVALID_OPERATION,
+  "glDispatchCompute(variable work group size forbidden)");
+  return GL_FALSE;
+   }
+
+   return GL_TRUE;
+}
+
+GLboolean
+_mesa_validate_DispatchComputeGroupSizeARB(struct gl_context *ctx,
+   const GLuint *num_groups,
+   const GLuint *group_size)
+{
+   struct gl_shader_program *prog;
+   GLuint64 total_invocations = 1;
+   int i;
+
+   FLUSH_CURRENT(ctx, 0);
+
+   if (!check_valid_to_compute(ctx, "glDispatchComputeGroupSizeARB"))
+  return GL_FALSE;
+
+   for (i = 0; i < 3; i++) {
+  /* From the ARB_compute_variable_group_size specification:
+   *
+   * "An INVALID_VALUE error is generated by DispatchComputeGroupSizeARB if
+   * any of , , or  is less than
+   * or equal to zero or greater than the maximum local work group size for
+   * compute shaders with variable group size
+   * (MAX_COMPUTE_VARIABLE_GROUP_SIZE_ARB) in the corresponding dimension."
+   *
+   * However, the "less than" is a spec bug because they are declared as
+   * unsigned integers.
+   */
+  if (group_size[i] == 0 ||
+  group_size[i] > ctx->Const.MaxComputeVariableGroupSize[i]) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glDispatchComputeGroupSizeARB(group_size_%c)", 'x' + i);
+ return GL_FALSE;
+  }
+
+  /* From the ARB_compute_variable_group_size specification:
+   *
+   * "An INVALID_VALUE error is generated by DispatchComputeGroupSizeARB if
+   * the product of , , and 
+   * exceeds the implementation-dependent maximum local work group
+   * invocation count for compute shaders with variable group size
+   * (MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB)."
+   */
+  total_invocations *= group_size[i];
+  if (total_invocations > ctx->Const.MaxComputeVariableGroupInvocations) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glDispatchComputeGroupSizeARB(product of local_sizes "
+ "exceeds MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB 
(%d))",
+ ctx->Const.MaxComputeVariableGroupInvocations);
+ return GL_FALSE;
+  }


This check should happen after the loop, and you should also log the
value of total_invocations.  Something like:

   _mesa_error(ctx, GL_INVALID_VALUE,
   "glDispatchComputeGroupSizeARB(product of local_sizes "
   "exceeds MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB (%d < %d))",
total_invocations,
ctx->Const.MaxComputeVariableGroupInvocations);


+
+  /* From the ARB_compute_variable_group_size specification:
+   *
+   * "An INVALID_OPERATION error is generated by
+   * DispatchComputeGroupSizeARB if the active program for the compute
+   * shader stage has a fixed work group size."
+   */
+  prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
+  if (prog->Comp.LocalSize[i] != 0) {


Does LocalSize[i] == 0 imply 

Re: [Mesa-dev] Proposal: move the Mesa documentation to readthedocs.org

2016-09-11 Thread Nicholas Bishop
Good questions,

> 1. How can we minimize the effort of maintaining two repositories? (I don't
> think anyone here would support dropping the HTML docs, as they feed into
> the main Mesa website).

I agree that maintaining two repos would be bad, but we don't need to
do that. Sphinx can generate HTML from RST in combination with a style
template to provide the look 'n feel. This is what Read The Docs does:
http://docs.readthedocs.io/en/latest/builds.html#sphinx

For Mesa's website we could either pre-generate the HTML and store it
in the mesa git repo alongside the RST sources, or it could be done in
a separate update-the-website script. I'm not sure how Mesa's website
is currently deployed.

> 2. How easy it is to automate the additional tweaks you mention? Scriptable?

Assuming that RST becomes the canonical source for the HTML it'll be a
one time deal; we use pandoc to automate the initial conversion as
shown in my github repo, then manually fix any formatting issues. I'm
not sure how long it would take, but I'm happy to take on that work if
there's buy-in from the project maintainers.

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


Re: [Mesa-dev] Proposal: move the Mesa documentation to readthedocs.org

2016-09-11 Thread Rhys Kidd
On 27 August 2016 at 01:09, Nicholas Bishop  wrote:

> Hi,
>
> I'd like to propose a conversion of Mesa's documentation to
> reStructuredText (RST) and hosting the result on readthedocs.org. The
> intent is to make Mesa's documentation more accessible, searchable, and
> easier to edit.
>
> I put together a quick proof-of-concept here:
>
> http://mesa-docs.readthedocs.io/en/latest/intro.html
>
> (For comparison: http://mesa3d.org/intro.html)
>
> Readthedocs.org essentially renders either Markdown or RST from a git
> repository (or other VCS) into a nice pretty set of HTML pages. (I'm more
> familiar with Markdown than RST, but I saw the Gallium docs are already
> using RST on readthedocs.org.)
>
> Putting the content in RST and on readthedocs.org makes formatting and
> searching easy, and it's a familiar platform for many developers.
>
> For the linked proof-of-concept I used pandoc (http://pandoc.org/) to
> convert all the HTML pages to RST, on top of which I did some minor editing
> to make the table of contents look reasonable. It's by no means a finished
> conversion, but I hope having something to look at can make discussion
> easier. This is all just a few hours work, but of course proofing and
> correcting all the text/formatting would take somewhat longer.
>
> Here's the git repo that readthedocs.org is pulling from:
> https://github.com/nicholasbishop/mesa-docs/tree/master/docs
>
> Thoughts?
>

Hello Nicholas,

Although not a core Mesa developer, I have recently done work on Mesa
documentation.

So FWIW, in my view this is a worthwhile objective to work towards
readthedocs.org formatted documentation. It is clear and well presented. We
also have precedence within this project for using that service, for the
Gallium docs in RST.

Worth considering however:

1. How can we minimize the effort of maintaining two repositories? (I don't
think anyone here would support dropping the HTML docs, as they feed into
the main Mesa website).
2. How easy it is to automate the additional tweaks you mention? Scriptable?

Rhys


>
> -Nicholas
>
>
> ___
> 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] [PATCH] st/mesa: fix is_scissor_enabled when X/Y are negative

2016-09-11 Thread Ilia Mirkin
Similar to commit 49c24d8a24 ("i965: fix noop_scissor range issue on
width/height") - take the X/Y into account to determine whether the
scissor covers the whole area or not.

Fixes the recently-added gl-1.0-scissor-depth-clear-negative-xy piglit
test.

Signed-off-by: Ilia Mirkin 
Cc: 
---
 src/mesa/state_tracker/st_cb_clear.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_clear.c 
b/src/mesa/state_tracker/st_cb_clear.c
index e70cba6..813ba9b 100644
--- a/src/mesa/state_tracker/st_cb_clear.c
+++ b/src/mesa/state_tracker/st_cb_clear.c
@@ -313,11 +313,13 @@ clear_with_quad(struct gl_context *ctx, unsigned 
clear_buffers)
 static inline GLboolean
 is_scissor_enabled(struct gl_context *ctx, struct gl_renderbuffer *rb)
 {
+   const struct gl_scissor_rect *scissor = >Scissor.ScissorArray[0];
+
return (ctx->Scissor.EnableFlags & 1) &&
-  (ctx->Scissor.ScissorArray[0].X > 0 ||
-   ctx->Scissor.ScissorArray[0].Y > 0 ||
-   (unsigned) ctx->Scissor.ScissorArray[0].Width < rb->Width ||
-   (unsigned) ctx->Scissor.ScissorArray[0].Height < rb->Height);
+  (scissor->X > 0 ||
+   scissor->Y > 0 ||
+   scissor->X + scissor->Width < rb->Width ||
+   scissor->Y + scissor->Height < rb->Height);
 }
 
 /**
-- 
2.7.3

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


Re: [Mesa-dev] [PATCH 09/23] glsl: Convert constant_expression to the util hash table

2016-09-11 Thread Thomas Helland
2016-09-11 6:27 GMT+02:00 Timothy Arceri :
> On Sat, 2016-09-10 at 13:19 +0200, Thomas Helland wrote:
>> 2016-09-09 0:20 GMT+02:00 Thomas Helland :
>> >
>> > 2016-08-16 22:10 GMT+02:00 Thomas Helland > > m>:
>> > >
>> > > Signed-off-by: Thomas Helland 
>> > > ---
>> > >  src/compiler/glsl/ir_constant_expression.cpp | 24 +-
>> > > --
>> > >  1 file changed, 13 insertions(+), 11 deletions(-)
>> > >
>> > > diff --git a/src/compiler/glsl/ir_constant_expression.cpp
>> > > b/src/compiler/glsl/ir_constant_expression.cpp
>> > > index 6329acd..16c8fac 100644
>> > > --- a/src/compiler/glsl/ir_constant_expression.cpp
>> > > +++ b/src/compiler/glsl/ir_constant_expression.cpp
>> > > @@ -39,7 +39,7 @@
>> > >  #include "util/half_float.h"
>> > >  #include "ir.h"
>> > >  #include "compiler/glsl_types.h"
>> > > -#include "program/hash_table.h"
>> > > +#include "util/hash_table.h"
>> > >
>> > >  static float
>> > >  dot_f(ir_constant *op0, ir_constant *op1)
>> > > @@ -457,7 +457,8 @@ constant_referenced(const ir_dereference
>> > > *deref,
>> > >const ir_dereference_variable *const dv =
>> > >   (const ir_dereference_variable *) deref;
>> > >
>> > > -  store = (ir_constant *) hash_table_find(variable_context,
>> > > dv->var);
>> > > +  hash_entry *entry =
>> > > _mesa_hash_table_search(variable_context, dv->var);
>> > > +  store = (ir_constant *) entry->data;
>> > >break;
>> > > }
>> > >
>> > > @@ -1806,9 +1807,10 @@
>> > > ir_dereference_variable::constant_expression_value(struct
>> > > hash_table *variable_c
>> > >
>> > > /* Give priority to the context hashtable, if it exists */
>> > > if (variable_context) {
>> > > -  ir_constant *value = (ir_constant
>> > > *)hash_table_find(variable_context, var);
>> > > -  if(value)
>> > > - return value;
>> > > +  hash_entry *entry =
>> > > _mesa_hash_table_search(variable_context, var);
>> > > +
>> > > +  if(entry)
>> > > + return (ir_constant *) entry->data;
>> > > }
>> > >
>> > > /* The constant_value of a uniform variable is its
>> > > initializer,
>> > > @@ -1926,7 +1928,7 @@ bool
>> > > ir_function_signature::constant_expression_evaluate_expression_li
>> > > st(const s
>> > >   /* (declare () type symbol) */
>> > >case ir_type_variable: {
>> > >   ir_variable *var = inst->as_variable();
>> > > - hash_table_insert(variable_context,
>> > > ir_constant::zero(this, var->type), var);
>> > > + _mesa_hash_table_insert(variable_context, var,
>> > > ir_constant::zero(this, var->type));
>> > >   break;
>> > >}
>> > >
>> > > @@ -2050,8 +2052,8 @@
>> > > ir_function_signature::constant_expression_value(exec_list
>> > > *actual_parameters, s
>> > >  * We expect the correctness of the number of parameters to
>> > > have
>> > >  * been checked earlier.
>> > >  */
>> > > -   hash_table *deref_hash = hash_table_ctor(8,
>> > > hash_table_pointer_hash,
>> > > -hash_table_pointer_c
>> > > ompare);
>> > > +   hash_table *deref_hash = _mesa_hash_table_create(NULL,
>> > > _mesa_hash_pointer,
>> > > +_mesa_key_po
>> > > inter_equal);
>> > >
>> > > /* If "origin" is non-NULL, then the function body is
>> > > there.  So we
>> > >  * have to use the variable objects from the object with the
>> > > body,
>> > > @@ -2062,13 +2064,13 @@
>> > > ir_function_signature::constant_expression_value(exec_list
>> > > *actual_parameters, s
>> > > foreach_in_list(ir_rvalue, n, actual_parameters) {
>> > >ir_constant *constant = n-
>> > > >constant_expression_value(variable_context);
>> > >if (constant == NULL) {
>> > > - hash_table_dtor(deref_hash);
>> > > + _mesa_hash_table_destroy(deref_hash, NULL);
>> > >   return NULL;
>> > >}
>> > >
>> > >
>> > >ir_variable *var = (ir_variable *)parameter_info;
>> > > -  hash_table_insert(deref_hash, constant, var);
>> > > +  _mesa_hash_table_insert(deref_hash, constant, var);
>> >
>> > This would be the cause of the regressions.
>> > The API is inverted between the hash table implementations,
>> > but the arguments here are not. No wonder weird things happen.
>> > Will do a complete piglit run (except deqp, etc) and send
>> > an updated patch to the list likely sometime tomorrow.
>> >
>>
>> So, I did a complete piglit run, and the only changes after fixing
>> this issue are some likely spurious changes in some timing tests.
>> Some gl_arb_sync_control tests goes from warn to pass, and some goes
>> the opposite way, from pass to warn. ext-timer-query time-elapsed
>> goes
>> from pass to fail. I don't see how these are related to this change
>> though.
>
> Yeah don't worry about those. Do you have a branch somewhere that I can
> 

[Mesa-dev] [Bug 97766] Multiple EGL displays with multiple window systems leads to a crash

2016-09-11 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=97766

Bug ID: 97766
   Summary: Multiple EGL displays with multiple window systems
leads to a crash
   Product: Mesa
   Version: 12.0
  Hardware: All
OS: All
Status: NEW
  Severity: minor
  Priority: medium
 Component: EGL
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: legrand.cedric...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 126460
  --> https://bugs.freedesktop.org/attachment.cgi?id=126460=edit
mesa-12.0.2-multiple-egl-displays.patch

When opening multiple displays from multiple window systems, an application
will crash. The second display is opened with the first one driver, leading to
a crash. I know this is a side case, but I could not find it forbidden in the
EGL spec.

This may be usefull for example when opening an X11 display and then a Wayland
one to render Wayland applications in X11. I know it can be rendered directly,
but my use case is more complex than this.

This bug was reported against Mir some years ago, but it is still not fixed in
Mesa 12.0.2. You may find the bug here :
https://bugs.launchpad.net/mir/+bug/1122394

I updated the Mir patch to apply against the latest release, please find it
attached. It seems to work after a quick test, but I did not deeply tested it
for the main reason that I don't know how. I don't have a simple example, but I
can make one if needed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev