Re: [Mesa-dev] [PATCH 00/11] i965 shader debug through KHR_debug

[Jason Ekstrand]

Did this get dropped on the ground?

On Fri, Dec 7, 2018 at 12:07 PM Mark Janes  wrote:

> Ilia Mirkin  writes:
>
> > On Thu, Dec 6, 2018 at 7:36 PM Mark Janes 
> wrote:
> >>
> >> This series provides Intel shader compilation debug information via
> >> KHR_debug.  Previously, shader assembly and related compilation
> >> artifacts were dumped to stderr.  Tools associating compilation
> >> artifacts with programs (e.g. FrameRetrace*) parsed stderr, which was
> >> error prone.  Changes to the shader debug formats and the addition of
> >> shader cache assembly dumps further complicate the task of parsing
> >> stderr.
> >>
> >> KHR_debug provides synchronous callbacks with stable identifiers,
> >> simplifying the task of matching debug artifacts with the originating
> >> GLSL.
> >
> > One observation is that while the identifiers may be stable within a
> > single execution, they will not be stable across different
> > applications / traces. id's are set on a first-come first-serve basis,
> > so depending on the exact order, the id's will end up different.
> >
> > Is that OK for frameretrace? Another approach would be to create
> > globally-hardcoded id's for these, and start the auto-allocation in a
> > higher range.
>
> I did take a few steps down the path of globally hard-coded id's.  I
> agree with Eric's assessment in debug_output.c that a giant enum list of
> all debug message id's is unworkable.
>
> We could divide id's into regions based on the high bits, and allocate
> the lower bits to components to declare and manage.  This would break up
> the monolithic declaration of id's, but you still have the issue of id
> stability as the driver changes over time.  Refactoring functionality to
> a location where it can be shared would involve changing the id's of
> emitted debug messages.  Also, any taxonomy used to split up the id's
> will probably look shortsighted in 5 years time.
>
> I personally haven't seen a great solution for sharing a 32 debug id
> within a large project.  The idea of exporting the id's to external
> tools is even more problematic.
>
> Rather than try to solve that problem, I decided to associate unknown
> id's with the semantic meaning by parsing the first few words in
> frameretrace.  A stable id means subsequent messages can be handled
> without parsing.  Unknown messages are turned of via KHR_debug.  This is
> a much better parsing stderr, where it is difficult to determine message
> boundaries.
> ___
> 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] gallium: set PIPE_CAP_MAX_FRAMES_IN_FLIGHT to 2 for all drivers

[Kenneth Graunke]

On Tuesday, April 23, 2019 6:23:32 PM PDT Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  src/gallium/auxiliary/util/u_screen.c| 4 +++-
>  src/gallium/drivers/freedreno/freedreno_screen.c | 3 ---
>  src/gallium/drivers/i915/i915_screen.c   | 3 ---
>  src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 3 ---
>  src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 3 ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 3 ---
>  src/gallium/drivers/r300/r300_screen.c   | 3 ---
>  src/gallium/drivers/r600/r600_pipe.c | 3 ---
>  src/gallium/drivers/radeonsi/si_get.c| 3 ---
>  src/gallium/drivers/svga/svga_screen.c   | 2 --
>  10 files changed, 3 insertions(+), 27 deletions(-)

This seems like a good idea.  The old default drm config specified 2,
so this is basically the equivalent now that it's a PIPE_CAP.  If
some driver wants a different value, they can override it.

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Timothy Arceri]

On 24/4/19 10:59 am, Jason Ekstrand wrote:
On Tue, Apr 23, 2019 at 7:39 PM Timothy Arceri > wrote:


On 24/4/19 1:45 am, Samuel Pitoiset wrote:
 >
 > On 4/23/19 5:16 PM, Jason Ekstrand wrote:
 >> On Tue, Apr 23, 2019 at 7:46 AM Samuel Pitoiset
 >> mailto:samuel.pitoi...@gmail.com>
>> wrote:
 >>
 >>
 >>     On 4/23/19 10:45 AM, Bas Nieuwenhuizen wrote:
 >>     > On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
 >>     > mailto:samuel.pitoi...@gmail.com> >>
 >>     wrote:
 >>     >> Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>
 >>     >>
 >>     >> ---
 >>     >>   src/amd/vulkan/radv_shader.c                 |  2 +-
 >>     >>   src/compiler/nir/nir.h                       |  3 ++-
 >>     >>   src/compiler/nir/nir_opt_if.c                | 17
 >>     ++---
 >>     >>   src/freedreno/ir3/ir3_nir.c                  |  2 +-
 >>     >>   src/gallium/auxiliary/nir/tgsi_to_nir.c      |  2 +-
 >>     >>   src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
 >>     >>   src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
 >>     >>   src/intel/compiler/brw_nir.c                 |  2 +-
 >>     >>   src/mesa/state_tracker/st_glsl_to_nir.cpp    |  2 +-
 >>     >>   9 files changed, 19 insertions(+), 15 deletions(-)
 >>     >>
 >>     >> diff --git a/src/amd/vulkan/radv_shader.c
 >>     b/src/amd/vulkan/radv_shader.c
 >>     >> index 13f1f9aa9dc..54a4e732230 100644
 >>     >> --- a/src/amd/vulkan/radv_shader.c
 >>     >> +++ b/src/amd/vulkan/radv_shader.c
 >>     >> @@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader
 >>     *shader, bool optimize_conservatively,
 >>     >>                          NIR_PASS(progress, shader,
 >>     nir_opt_remove_phis);
 >>     >>                           NIR_PASS(progress, shader,
nir_opt_dce);
 >>     >>                   }
 >>     >> -                NIR_PASS(progress, shader, nir_opt_if,
true);
 >>     >> +                NIR_PASS(progress, shader, nir_opt_if, true,
 >>     false);
 >>     >>                   NIR_PASS(progress, shader,
nir_opt_dead_cf);
 >>     >>                   NIR_PASS(progress, shader, nir_opt_cse);
 >>     >>                   NIR_PASS(progress, shader,
 >>     nir_opt_peephole_select, 8, true, true);
 >>     >> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
 >>     >> index 7d2062d3691..d7506d6ddd1 100644
 >>     >> --- a/src/compiler/nir/nir.h
 >>     >> +++ b/src/compiler/nir/nir.h
 >>     >> @@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader
*shader, bool
 >>     value_number);
 >>     >>
 >>     >>   bool nir_opt_idiv_const(nir_shader *shader, unsigned
 >>     min_bit_size);
 >>     >>
 >>     >> -bool nir_opt_if(nir_shader *shader, bool
 >>     aggressive_last_continue);
 >>     >> +bool nir_opt_if(nir_shader *shader, bool
aggressive_last_continue,
 >>     >> +                bool skip_alu_of_phi);
 >>     > Can we have a flag for this instead (e.g. something like
 >>     > nir_opt_if_skip_alu_of_phi)? I think have a function with
a bunch of
 >>     > bools is less than ideal as you can't see at the calling site
 >>     what is
 >>     > for what arg.
 >>     Yes, that seems better to me.
 >>
 >>
 >> This is the worst kind of hack all around.  We're making NIR more
 >> complicated and adding a flag to disable a useful and correct
piece of
 >> an optimization, not because it causes a perf regression but
because
 >> the back-end compiler is broken and this is easier than fixing it
 >> properly. Seriously?  Can't we just fix the LLVM back-end?  Or, if
 >> this optimization is actually doing something wrong, fix it?  Or
maybe
 >> actually figure out what pattern is causing LLVM to fall over
and have
 >> a hack in your NIR -> LLVM pass?  On the list of "good ways to fix
 >> this problem", this seems to be pretty far down if it hasn't fallen
 >> off the bottom.
 >
 > Best hack of the month? :-)
 >
 > As discussed over IRC, this is definitely not the best solution,
I don't
 > like it either as I said.
 >
 > I will work on a different solution maybe in our NIR->LLVM pass.
 >
 > The aggressive_last_continue option should also be removed (make it
 > default?).

The aggressive_last_continue option is not a hack to avoid a bug in a
driver backend it is to avoid perf regressions. That said we may be
able
to remove it now that Jason has landed

[Mesa-dev] [PATCH] gallium: set PIPE_CAP_MAX_FRAMES_IN_FLIGHT to 2 for all drivers

[Marek Olšák]

From: Marek Olšák 

---
 src/gallium/auxiliary/util/u_screen.c| 4 +++-
 src/gallium/drivers/freedreno/freedreno_screen.c | 3 ---
 src/gallium/drivers/i915/i915_screen.c   | 3 ---
 src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 3 ---
 src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 3 ---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 3 ---
 src/gallium/drivers/r300/r300_screen.c   | 3 ---
 src/gallium/drivers/r600/r600_pipe.c | 3 ---
 src/gallium/drivers/radeonsi/si_get.c| 3 ---
 src/gallium/drivers/svga/svga_screen.c   | 2 --
 10 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_screen.c 
b/src/gallium/auxiliary/util/u_screen.c
index a6c7e086c32..27f51e0898e 100644
--- a/src/gallium/auxiliary/util/u_screen.c
+++ b/src/gallium/auxiliary/util/u_screen.c
@@ -346,23 +346,25 @@ u_pipe_screen_get_param_defaults(struct pipe_screen 
*pscreen,
case PIPE_CAP_DEST_SURFACE_SRGB_CONTROL:
   return 1;
 
case PIPE_CAP_MAX_VARYINGS:
   return 8;
 
case PIPE_CAP_COMPUTE_GRID_INFO_LAST_BLOCK:
   return 0;
 
case PIPE_CAP_COMPUTE_SHADER_DERIVATIVES:
-   case PIPE_CAP_MAX_FRAMES_IN_FLIGHT:
   return 0;
 
+   case PIPE_CAP_MAX_FRAMES_IN_FLIGHT:
+  return 2;
+
case PIPE_CAP_DMABUF:
 #ifdef PIPE_OS_LINUX
   return 1;
 #else
   return 0;
 #endif
 
default:
   unreachable("bad PIPE_CAP_*");
}
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
b/src/gallium/drivers/freedreno/freedreno_screen.c
index 38a459e73aa..fdf26390718 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -373,23 +373,20 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
return is_a3xx(screen) ? 1 : 0;
 
/* Queries. */
case PIPE_CAP_OCCLUSION_QUERY:
return is_a3xx(screen) || is_a4xx(screen) || is_a5xx(screen) || 
is_a6xx(screen);
case PIPE_CAP_QUERY_TIMESTAMP:
case PIPE_CAP_QUERY_TIME_ELAPSED:
/* only a4xx, requires new enough kernel so we know max_freq: */
return (screen->max_freq > 0) && (is_a4xx(screen) || 
is_a5xx(screen) || is_a6xx(screen));
 
-   case PIPE_CAP_MAX_FRAMES_IN_FLIGHT:
-   return 2;
-
case PIPE_CAP_VENDOR_ID:
return 0x5143;
case PIPE_CAP_DEVICE_ID:
return 0x;
case PIPE_CAP_ACCELERATED:
return 1;
case PIPE_CAP_VIDEO_MEMORY:
DBG("FINISHME: The value returned is incorrect\n");
return 10;
case PIPE_CAP_UMA:
diff --git a/src/gallium/drivers/i915/i915_screen.c 
b/src/gallium/drivers/i915/i915_screen.c
index 62ff5f89198..78707c66e62 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -334,23 +334,20 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap 
cap)
case PIPE_CAP_MAX_COMBINED_SHADER_OUTPUT_RESOURCES:
case PIPE_CAP_FRAMEBUFFER_MSAA_CONSTRAINTS:
case PIPE_CAP_SIGNED_VERTEX_BUFFER_OFFSET:
case PIPE_CAP_CONTEXT_PRIORITY_MASK:
case PIPE_CAP_FENCE_SIGNAL:
case PIPE_CAP_CONSTBUF0_FLAGS:
case PIPE_CAP_PACKED_UNIFORMS:
case PIPE_CAP_PROGRAMMABLE_SAMPLE_LOCATIONS:
   return 0;
 
-   case PIPE_CAP_MAX_FRAMES_IN_FLIGHT:
-  return 2;
-
case PIPE_CAP_MAX_GS_INVOCATIONS:
   return 32;
 
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
   return 1 << 27;
 
case PIPE_CAP_MAX_VIEWPORTS:
   return 1;
 
case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT:
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c 
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index e6e7fac2a44..b5dc033bd2d 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -242,23 +242,20 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_CONSERVATIVE_RASTER_POST_SNAP_TRIANGLES:
case PIPE_CAP_CONSERVATIVE_RASTER_POST_SNAP_POINTS_LINES:
case PIPE_CAP_CONSERVATIVE_RASTER_PRE_SNAP_TRIANGLES:
case PIPE_CAP_CONSERVATIVE_RASTER_PRE_SNAP_POINTS_LINES:
case PIPE_CAP_CONSERVATIVE_RASTER_POST_DEPTH_COVERAGE:
case PIPE_CAP_MAX_CONSERVATIVE_RASTER_SUBPIXEL_PRECISION_BIAS:
case PIPE_CAP_PROGRAMMABLE_SAMPLE_LOCATIONS:
case PIPE_CAP_IMAGE_LOAD_FORMATTED:
   return 0;
 
-   case PIPE_CAP_MAX_FRAMES_IN_FLIGHT:
-  return 2;
-
case PIPE_CAP_MAX_GS_INVOCATIONS:
   return 32;
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
   return 1 << 27;
case PIPE_CAP_VENDOR_ID:
   return 0x10de;
case PIPE_CAP_DEVICE_ID: {
   uint64_t device_id;
   if (nouveau_getparam(dev, NOUVEAU_GETPARAM_PCI_DEVICE, _id)) {
  NOUVEAU_ERR("NOUVEAU_GETPARAM_PCI_DEVICE failed.\n");
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c 

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Jason Ekstrand]

On Tue, Apr 23, 2019 at 7:39 PM Timothy Arceri 
wrote:

> On 24/4/19 1:45 am, Samuel Pitoiset wrote:
> >
> > On 4/23/19 5:16 PM, Jason Ekstrand wrote:
> >> On Tue, Apr 23, 2019 at 7:46 AM Samuel Pitoiset
> >> mailto:samuel.pitoi...@gmail.com>> wrote:
> >>
> >>
> >> On 4/23/19 10:45 AM, Bas Nieuwenhuizen wrote:
> >> > On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
> >> > mailto:samuel.pitoi...@gmail.com>>
> >> wrote:
> >> >> Signed-off-by: Samuel Pitoiset  >> >
> >> >> ---
> >> >>   src/amd/vulkan/radv_shader.c |  2 +-
> >> >>   src/compiler/nir/nir.h   |  3 ++-
> >> >>   src/compiler/nir/nir_opt_if.c| 17
> >> ++---
> >> >>   src/freedreno/ir3/ir3_nir.c  |  2 +-
> >> >>   src/gallium/auxiliary/nir/tgsi_to_nir.c  |  2 +-
> >> >>   src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
> >> >>   src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
> >> >>   src/intel/compiler/brw_nir.c |  2 +-
> >> >>   src/mesa/state_tracker/st_glsl_to_nir.cpp|  2 +-
> >> >>   9 files changed, 19 insertions(+), 15 deletions(-)
> >> >>
> >> >> diff --git a/src/amd/vulkan/radv_shader.c
> >> b/src/amd/vulkan/radv_shader.c
> >> >> index 13f1f9aa9dc..54a4e732230 100644
> >> >> --- a/src/amd/vulkan/radv_shader.c
> >> >> +++ b/src/amd/vulkan/radv_shader.c
> >> >> @@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader
> >> *shader, bool optimize_conservatively,
> >> >>  NIR_PASS(progress, shader,
> >> nir_opt_remove_phis);
> >> >>   NIR_PASS(progress, shader,
> nir_opt_dce);
> >> >>   }
> >> >> -NIR_PASS(progress, shader, nir_opt_if, true);
> >> >> +NIR_PASS(progress, shader, nir_opt_if, true,
> >> false);
> >> >>   NIR_PASS(progress, shader, nir_opt_dead_cf);
> >> >>   NIR_PASS(progress, shader, nir_opt_cse);
> >> >>   NIR_PASS(progress, shader,
> >> nir_opt_peephole_select, 8, true, true);
> >> >> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >> >> index 7d2062d3691..d7506d6ddd1 100644
> >> >> --- a/src/compiler/nir/nir.h
> >> >> +++ b/src/compiler/nir/nir.h
> >> >> @@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool
> >> value_number);
> >> >>
> >> >>   bool nir_opt_idiv_const(nir_shader *shader, unsigned
> >> min_bit_size);
> >> >>
> >> >> -bool nir_opt_if(nir_shader *shader, bool
> >> aggressive_last_continue);
> >> >> +bool nir_opt_if(nir_shader *shader, bool
> aggressive_last_continue,
> >> >> +bool skip_alu_of_phi);
> >> > Can we have a flag for this instead (e.g. something like
> >> > nir_opt_if_skip_alu_of_phi)? I think have a function with a bunch
> of
> >> > bools is less than ideal as you can't see at the calling site
> >> what is
> >> > for what arg.
> >> Yes, that seems better to me.
> >>
> >>
> >> This is the worst kind of hack all around.  We're making NIR more
> >> complicated and adding a flag to disable a useful and correct piece of
> >> an optimization, not because it causes a perf regression but because
> >> the back-end compiler is broken and this is easier than fixing it
> >> properly. Seriously?  Can't we just fix the LLVM back-end?  Or, if
> >> this optimization is actually doing something wrong, fix it?  Or maybe
> >> actually figure out what pattern is causing LLVM to fall over and have
> >> a hack in your NIR -> LLVM pass?  On the list of "good ways to fix
> >> this problem", this seems to be pretty far down if it hasn't fallen
> >> off the bottom.
> >
> > Best hack of the month? :-)
> >
> > As discussed over IRC, this is definitely not the best solution, I don't
> > like it either as I said.
> >
> > I will work on a different solution maybe in our NIR->LLVM pass.
> >
> > The aggressive_last_continue option should also be removed (make it
> > default?).
>
> The aggressive_last_continue option is not a hack to avoid a bug in a
> driver backend it is to avoid perf regressions. That said we may be able
> to remove it now that Jason has landed
> cd4ffb376f2aeefdd6a1b80d69a1580c4e569778
>

I don't think anyone was claiming it was.  The only claim made was that
it's probably an ultimately unnecessary option and we should try to apply
it universally.

--Jason



> >
> >>
> >> --Jason
> >>
> >> >>   bool nir_opt_intrinsics(nir_shader *shader);
> >> >>
> >> >> diff --git a/src/compiler/nir/nir_opt_if.c
> >> b/src/compiler/nir/nir_opt_if.c
> >> >> index f674185f1e2..149b3bd1659 100644
> >> >> --- a/src/compiler/nir/nir_opt_if.c
> >> >> +++ b/src/compiler/nir/nir_opt_if.c
> >> >> @@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Timothy Arceri]

On 24/4/19 1:45 am, Samuel Pitoiset wrote:


On 4/23/19 5:16 PM, Jason Ekstrand wrote:
On Tue, Apr 23, 2019 at 7:46 AM Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:



On 4/23/19 10:45 AM, Bas Nieuwenhuizen wrote:
> On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
> mailto:samuel.pitoi...@gmail.com>>
wrote:
>> Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
>> ---
>>   src/amd/vulkan/radv_shader.c                 |  2 +-
>>   src/compiler/nir/nir.h                       |  3 ++-
>>   src/compiler/nir/nir_opt_if.c                | 17
++---
>>   src/freedreno/ir3/ir3_nir.c                  |  2 +-
>>   src/gallium/auxiliary/nir/tgsi_to_nir.c      |  2 +-
>>   src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
>>   src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
>>   src/intel/compiler/brw_nir.c                 |  2 +-
>>   src/mesa/state_tracker/st_glsl_to_nir.cpp    |  2 +-
>>   9 files changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_shader.c
b/src/amd/vulkan/radv_shader.c
>> index 13f1f9aa9dc..54a4e732230 100644
>> --- a/src/amd/vulkan/radv_shader.c
>> +++ b/src/amd/vulkan/radv_shader.c
>> @@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader
*shader, bool optimize_conservatively,
>>                          NIR_PASS(progress, shader,
nir_opt_remove_phis);
>>                           NIR_PASS(progress, shader, nir_opt_dce);
>>                   }
>> -                NIR_PASS(progress, shader, nir_opt_if, true);
>> +                NIR_PASS(progress, shader, nir_opt_if, true,
false);
>>                   NIR_PASS(progress, shader, nir_opt_dead_cf);
>>                   NIR_PASS(progress, shader, nir_opt_cse);
>>                   NIR_PASS(progress, shader,
nir_opt_peephole_select, 8, true, true);
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index 7d2062d3691..d7506d6ddd1 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool
value_number);
>>
>>   bool nir_opt_idiv_const(nir_shader *shader, unsigned
min_bit_size);
>>
>> -bool nir_opt_if(nir_shader *shader, bool
aggressive_last_continue);
>> +bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
>> +                bool skip_alu_of_phi);
> Can we have a flag for this instead (e.g. something like
> nir_opt_if_skip_alu_of_phi)? I think have a function with a bunch of
> bools is less than ideal as you can't see at the calling site
what is
> for what arg.
Yes, that seems better to me.


This is the worst kind of hack all around.  We're making NIR more 
complicated and adding a flag to disable a useful and correct piece of 
an optimization, not because it causes a perf regression but because 
the back-end compiler is broken and this is easier than fixing it 
properly. Seriously?  Can't we just fix the LLVM back-end?  Or, if 
this optimization is actually doing something wrong, fix it?  Or maybe 
actually figure out what pattern is causing LLVM to fall over and have 
a hack in your NIR -> LLVM pass?  On the list of "good ways to fix 
this problem", this seems to be pretty far down if it hasn't fallen 
off the bottom.


Best hack of the month? :-)

As discussed over IRC, this is definitely not the best solution, I don't 
like it either as I said.


I will work on a different solution maybe in our NIR->LLVM pass.

The aggressive_last_continue option should also be removed (make it 
default?).


The aggressive_last_continue option is not a hack to avoid a bug in a 
driver backend it is to avoid perf regressions. That said we may be able 
to remove it now that Jason has landed 
cd4ffb376f2aeefdd6a1b80d69a1580c4e569778






--Jason

>>   bool nir_opt_intrinsics(nir_shader *shader);
>>
>> diff --git a/src/compiler/nir/nir_opt_if.c
b/src/compiler/nir/nir_opt_if.c
>> index f674185f1e2..149b3bd1659 100644
>> --- a/src/compiler/nir/nir_opt_if.c
>> +++ b/src/compiler/nir/nir_opt_if.c
>> @@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct
exec_list *cf_list,
>>    * not do anything to cause the metadata to become invalid.
>>    */
>>   static bool
>> -opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
>> +opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list,
>> +                    bool skip_alu_of_phi)
>>   {
>>      bool progress = false;
>>      foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
>> @@ -1395,16 +1396,17 @@ opt_if_safe_cf_list(nir_builder *b,
struct exec_list *cf_list)
>>
>>         case nir_cf_node_if: {
>>            nir_if *nif = nir_cf_node_as_if(cf_node);
>> -         progress |= 

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Marek Olšák]

Adam, did you notice my original suggestion "If there is at least 1 drm
device, swrast won't be in the list."? which means swrast would be in the
list for your "dumb" GPUs.

Marek

On Tue, Apr 23, 2019 at 7:52 PM Adam Jackson  wrote:

> On Tue, 2019-04-23 at 19:21 -0400, Marek Olšák wrote:
>
> > Then I think a separate EGL extension that exposes swrast would be
> > better. The thing with the device extensions is that swrast is not a
> > device.
>
> Enh. I've got dumb GPUs I need to support, they need to run OpenGL, and
> if they were running swrast-on-otherwise-dumb-gem, I can make posting
> the front buffer to the compositor be (at least sometimes) zero-copy,
> instead of the XPutImage thing that drisw currently has to do. So I
> don't think it's necessarily the case that swrast doesn't have
> "hardware" support behind it.
>
> More generally, if the term in the extension spec was "renderer" not
> "device" I don't think we'd be having this discussion. And I think it's
> worth having renderer selection be orthogonal in an API sense.
>
> - ajax
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Adam Jackson]

On Tue, 2019-04-23 at 19:21 -0400, Marek Olšák wrote:

> Then I think a separate EGL extension that exposes swrast would be
> better. The thing with the device extensions is that swrast is not a
> device.

Enh. I've got dumb GPUs I need to support, they need to run OpenGL, and
if they were running swrast-on-otherwise-dumb-gem, I can make posting
the front buffer to the compositor be (at least sometimes) zero-copy,
instead of the XPutImage thing that drisw currently has to do. So I
don't think it's necessarily the case that swrast doesn't have
"hardware" support behind it.

More generally, if the term in the extension spec was "renderer" not
"device" I don't think we'd be having this discussion. And I think it's
worth having renderer selection be orthogonal in an API sense.

- ajax

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

[Mesa-dev] [Bug 110455] Land adaptive_sync support

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110455

Bas Nieuwenhuizen  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

--- Comment #1 from Bas Nieuwenhuizen  ---
Landed https://gitlab.freedesktop.org/mesa/mesa/merge_requests/672 .

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

[Mesa-dev] [Bug 110439] [TRACKER] Mesa 19.1 feature tracker

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110439
Bug 110439 depends on bug 110455, which changed state.

Bug 110455 Summary: Land adaptive_sync support
https://bugs.freedesktop.org/show_bug.cgi?id=110455

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

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

Re: [Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers

[Marek Olšák]

On Tue, Apr 23, 2019 at 7:30 PM Marek Olšák  wrote:

> On Tue, Apr 23, 2019 at 4:49 PM Kenneth Graunke 
> wrote:
>
>> On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
>> > From: Marek Olšák 
>> >
>> > for better CPU-GPU parallelism
>> > ---
>> >  src/gallium/state_trackers/dri/dri_drawable.c | 20 +--
>> >  1 file changed, 10 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/src/gallium/state_trackers/dri/dri_drawable.c
>> b/src/gallium/state_trackers/dri/dri_drawable.c
>> > index 26bfdbecc53..c1de3bed9dd 100644
>> > --- a/src/gallium/state_trackers/dri/dri_drawable.c
>> > +++ b/src/gallium/state_trackers/dri/dri_drawable.c
>> > @@ -555,33 +555,33 @@ dri_flush(__DRIcontext *cPriv,
>> > *
>> > * This pulls a fence off the throttling queue and waits for it
>> if the
>> > * number of fences on the throttling queue has reached the
>> desired
>> > * number.
>> > *
>> > * Then flushes to insert a fence at the current rendering
>> position, and
>> > * pushes that fence on the queue. This requires that the
>> st_context_iface
>> > * flush method returns a fence even if there are no commands to
>> flush.
>> > */
>> >struct pipe_screen *screen = drawable->screen->base.screen;
>> > -  struct pipe_fence_handle *fence;
>> > +  struct pipe_fence_handle *oldest_fence, *new_fence = NULL;
>> >
>> > -  fence = swap_fences_pop_front(drawable);
>> > -  if (fence) {
>> > - (void) screen->fence_finish(screen, NULL, fence,
>> PIPE_TIMEOUT_INFINITE);
>> > - screen->fence_reference(screen, , NULL);
>> > -  }
>> > +  st->flush(st, flush_flags, _fence);
>> >
>> > -  st->flush(st, flush_flags, );
>> > +  oldest_fence = swap_fences_pop_front(drawable);
>> > +  if (oldest_fence) {
>> > + screen->fence_finish(screen, NULL, oldest_fence,
>> PIPE_TIMEOUT_INFINITE);
>> > + screen->fence_reference(screen, _fence, NULL);
>> > +  }
>> >
>> > -  if (fence) {
>> > - swap_fences_push_back(drawable, fence);
>> > - screen->fence_reference(screen, , NULL);
>> > +  if (new_fence) {
>> > + swap_fences_push_back(drawable, new_fence);
>> > + screen->fence_reference(screen, _fence, NULL);
>> >}
>> > }
>> > else if (flags & (__DRI2_FLUSH_DRAWABLE | __DRI2_FLUSH_CONTEXT)) {
>> >st->flush(st, flush_flags, NULL);
>> > }
>> >
>> > if (drawable) {
>> >drawable->flushing = FALSE;
>> > }
>>
>> It seems like this will let us submit one more batch before throttling,
>> which is a little like increasing the throttle value from 2 to 3...but
>> not exactly.  I'm not sure I have an opinion on which is better...
>>
>
> "Wait then flush" adds latency (input lag) to the unflushed frame.
> "Flush then wait" doesn't add latency.
>

Actually, if it has to wait, there is already large input lag, so I guess
this patch doesn't change anything other than increasing the number. I'll
drop it.

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

Re: [Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers

[Marek Olšák]

On Tue, Apr 23, 2019 at 4:49 PM Kenneth Graunke 
wrote:

> On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
> > From: Marek Olšák 
> >
> > for better CPU-GPU parallelism
> > ---
> >  src/gallium/state_trackers/dri/dri_drawable.c | 20 +--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/dri/dri_drawable.c
> b/src/gallium/state_trackers/dri/dri_drawable.c
> > index 26bfdbecc53..c1de3bed9dd 100644
> > --- a/src/gallium/state_trackers/dri/dri_drawable.c
> > +++ b/src/gallium/state_trackers/dri/dri_drawable.c
> > @@ -555,33 +555,33 @@ dri_flush(__DRIcontext *cPriv,
> > *
> > * This pulls a fence off the throttling queue and waits for it
> if the
> > * number of fences on the throttling queue has reached the
> desired
> > * number.
> > *
> > * Then flushes to insert a fence at the current rendering
> position, and
> > * pushes that fence on the queue. This requires that the
> st_context_iface
> > * flush method returns a fence even if there are no commands to
> flush.
> > */
> >struct pipe_screen *screen = drawable->screen->base.screen;
> > -  struct pipe_fence_handle *fence;
> > +  struct pipe_fence_handle *oldest_fence, *new_fence = NULL;
> >
> > -  fence = swap_fences_pop_front(drawable);
> > -  if (fence) {
> > - (void) screen->fence_finish(screen, NULL, fence,
> PIPE_TIMEOUT_INFINITE);
> > - screen->fence_reference(screen, , NULL);
> > -  }
> > +  st->flush(st, flush_flags, _fence);
> >
> > -  st->flush(st, flush_flags, );
> > +  oldest_fence = swap_fences_pop_front(drawable);
> > +  if (oldest_fence) {
> > + screen->fence_finish(screen, NULL, oldest_fence,
> PIPE_TIMEOUT_INFINITE);
> > + screen->fence_reference(screen, _fence, NULL);
> > +  }
> >
> > -  if (fence) {
> > - swap_fences_push_back(drawable, fence);
> > - screen->fence_reference(screen, , NULL);
> > +  if (new_fence) {
> > + swap_fences_push_back(drawable, new_fence);
> > + screen->fence_reference(screen, _fence, NULL);
> >}
> > }
> > else if (flags & (__DRI2_FLUSH_DRAWABLE | __DRI2_FLUSH_CONTEXT)) {
> >st->flush(st, flush_flags, NULL);
> > }
> >
> > if (drawable) {
> >drawable->flushing = FALSE;
> > }
>
> It seems like this will let us submit one more batch before throttling,
> which is a little like increasing the throttle value from 2 to 3...but
> not exactly.  I'm not sure I have an opinion on which is better...
>

"Wait then flush" adds latency (input lag) to the unflushed frame.
"Flush then wait" doesn't add latency.

Where we should go from here is a separate issue. Both 1 and 0 would work
here, but 0 currently disables the throttling.

Marek



>
> The rest of the series is:
> Reviewed-by: Kenneth Graunke 
>
> It looks like iris ends up with PIPE_CAP_MAX_FRAMES_IN_FLIGHT == 0 at
> the end of the series.  It was 2 before I botched the DRM_CONF config
> when adding driconf XML support, so I'd probably like it to be that way
> again.  Feel free to just fix that in your patch that introduces the
> cap, or I can push a patch after your series lands.
>
> Thanks for cleaning this up, it's nice to have one fewer place to
> configure this sort of stuff.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Marek Olšák]

On Tue, Apr 23, 2019 at 6:30 PM Eric Anholt  wrote:

> Marek Olšák  writes:
>
> > On Tue, Apr 23, 2019 at 4:39 PM Mathias Fröhlich <
> mathias.froehl...@gmx.net>
> > wrote:
> >
> >> Hi,
> >>
> >> On Tuesday, 23 April 2019 22:23:45 CEST Marek Olšák wrote:
> >> > On Tue, Apr 23, 2019 at 4:05 PM Mathias Fröhlich <
> >> mathias.froehl...@gmx.net>
> >> > wrote:
> >> >
> >> > > Hi Marek,
> >> > >
> >> > > On Tuesday, 23 April 2019 20:22:15 CEST Marek Olšák wrote:
> >> > > > I'd like to remove swrast from devices. It doesn't work
> >> (eglInitialize
> >> > > > fails) and I don't think I like swrast there. Any objections?
> >> > >
> >> > > Yes, how do you guarantee that at least one device can be returned
> >> > > in any case? Even if no drm device is found?
> >> > > The egl device query extension guarantees that there is at least one
> >> > > device available.
> >> > > Beside that swrast may make sense for itself, that is the reason
> >> > > the swrast device is there.
> >> > >
> >> >
> >> > If no device can be returned, the extension won't be exposed.
> >> Yes, I was triggering this solution back in the days. There was (is?) no
> >> infrastructure to enable
> >> or disable an egl extension just past initialization or at runtime.
> >> Finally Emil took the route to add swrast.
> >>
> >> The longer I saw the swrast device the better It appeared to me.
> >> ... from the users point of view.
> >>
> >> >OR
> >> > If there is at least 1 drm device, swrast won't be in the list.
> >> >
> >> > From my perspective, radeonsi is always in the list and I wouldn't
> like
> >> > swrast to be in the list if radeonsi is in the list.
> >>
> >> I see - at least I think. But really the swrast device is good as is. I
> >> see plenty use cases for that.
> >> Having that only present when there is no hardware device makes it less
> >> useful.
> >>
> >
> > How is swrast good? How is swrast useful if you have a supported GPU?
>
> EGL has the possibility of finally displacing OSMesa if we can expose
> swrast devices.  People doing regression testing of higher-level
> software stacks (xserver, servo, etc.) need a consistent rasterizer they
> can use regardless of the hardware a developer might happen to have on
> their desktop.
>

Then I think a separate EGL extension that exposes swrast would be better.
The thing with the device extensions is that swrast is not a device.

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

[Mesa-dev] [Bug 110439] [TRACKER] Mesa 19.1 feature tracker

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110439

Andrés Gómez García  changed:

   What|Removed |Added

 Depends on||110501


Referenced Bugs:

https://bugs.freedesktop.org/show_bug.cgi?id=110501
[Bug 110501] Land VK_KHR_shader_float_controls
-- 
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

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Eric Anholt]

Marek Olšák  writes:

> On Tue, Apr 23, 2019 at 4:39 PM Mathias Fröhlich 
> wrote:
>
>> Hi,
>>
>> On Tuesday, 23 April 2019 22:23:45 CEST Marek Olšák wrote:
>> > On Tue, Apr 23, 2019 at 4:05 PM Mathias Fröhlich <
>> mathias.froehl...@gmx.net>
>> > wrote:
>> >
>> > > Hi Marek,
>> > >
>> > > On Tuesday, 23 April 2019 20:22:15 CEST Marek Olšák wrote:
>> > > > I'd like to remove swrast from devices. It doesn't work
>> (eglInitialize
>> > > > fails) and I don't think I like swrast there. Any objections?
>> > >
>> > > Yes, how do you guarantee that at least one device can be returned
>> > > in any case? Even if no drm device is found?
>> > > The egl device query extension guarantees that there is at least one
>> > > device available.
>> > > Beside that swrast may make sense for itself, that is the reason
>> > > the swrast device is there.
>> > >
>> >
>> > If no device can be returned, the extension won't be exposed.
>> Yes, I was triggering this solution back in the days. There was (is?) no
>> infrastructure to enable
>> or disable an egl extension just past initialization or at runtime.
>> Finally Emil took the route to add swrast.
>>
>> The longer I saw the swrast device the better It appeared to me.
>> ... from the users point of view.
>>
>> >OR
>> > If there is at least 1 drm device, swrast won't be in the list.
>> >
>> > From my perspective, radeonsi is always in the list and I wouldn't like
>> > swrast to be in the list if radeonsi is in the list.
>>
>> I see - at least I think. But really the swrast device is good as is. I
>> see plenty use cases for that.
>> Having that only present when there is no hardware device makes it less
>> useful.
>>
>
> How is swrast good? How is swrast useful if you have a supported GPU?

EGL has the possibility of finally displacing OSMesa if we can expose
swrast devices.  People doing regression testing of higher-level
software stacks (xserver, servo, etc.) need a consistent rasterizer they
can use regardless of the hardware a developer might happen to have on
their desktop.


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

Re: [Mesa-dev] [PATCH v2] st: require compatible driver in autotools

[Alyssa Ross]

> Off the top of my head - none of the VL code should be build when we
> have only a swrast driver.
> Can you try and kill that bug, or shall I?

Isn't that what this patch does?

If there's only swrast, this patch prevents enabling any of the state
trackers, so need_gallium_vl won't be set, and VL won't be built.


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

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Marek Olšák]

On Tue, Apr 23, 2019 at 4:39 PM Mathias Fröhlich 
wrote:

> Hi,
>
> On Tuesday, 23 April 2019 22:23:45 CEST Marek Olšák wrote:
> > On Tue, Apr 23, 2019 at 4:05 PM Mathias Fröhlich <
> mathias.froehl...@gmx.net>
> > wrote:
> >
> > > Hi Marek,
> > >
> > > On Tuesday, 23 April 2019 20:22:15 CEST Marek Olšák wrote:
> > > > I'd like to remove swrast from devices. It doesn't work
> (eglInitialize
> > > > fails) and I don't think I like swrast there. Any objections?
> > >
> > > Yes, how do you guarantee that at least one device can be returned
> > > in any case? Even if no drm device is found?
> > > The egl device query extension guarantees that there is at least one
> > > device available.
> > > Beside that swrast may make sense for itself, that is the reason
> > > the swrast device is there.
> > >
> >
> > If no device can be returned, the extension won't be exposed.
> Yes, I was triggering this solution back in the days. There was (is?) no
> infrastructure to enable
> or disable an egl extension just past initialization or at runtime.
> Finally Emil took the route to add swrast.
>
> The longer I saw the swrast device the better It appeared to me.
> ... from the users point of view.
>
> >OR
> > If there is at least 1 drm device, swrast won't be in the list.
> >
> > From my perspective, radeonsi is always in the list and I wouldn't like
> > swrast to be in the list if radeonsi is in the list.
>
> I see - at least I think. But really the swrast device is good as is. I
> see plenty use cases for that.
> Having that only present when there is no hardware device makes it less
> useful.
>

How is swrast good? How is swrast useful if you have a supported GPU?

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

Re: [Mesa-dev] [PATCH] gallium: document conservative rasterization flags

[Kenneth Graunke]

On Monday, April 22, 2019 10:25:57 AM PDT Marek Olšák wrote:
[snip]
>  /**
>   * Conservative rasterization modes.
>   */
>  enum pipe_conservative_raster_mode
>  {
> PIPE_CONSERVATIVE_RASTER_OFF,
> +
> +   /**
> +* The post-snap mode means the conservative rasterization occurs after
> +* the conversion from floating-point to fixed-point coordinates
> +* on the subpixel grid.
> +*/
> PIPE_CONSERVATIVE_RASTER_POST_SNAP,
> +
> +   /**
> +* The pre-snap mode means the conservative rasterization occurs before
> +* the conversion from floating-point to fixed-point coordinates.
> +*/
> PIPE_CONSERVATIVE_RASTER_PRE_SNAP,
>  };

Thanks Marek, this makes a lot of sense and is helpful when trying to
remember what all of these options mean. :)

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Add an ASSERTED macro to use in place of MAYBE_UNUSED?

[Kenneth Graunke]

On Monday, April 22, 2019 3:21:22 PM PDT Eric Anholt wrote:
> Jason Ekstrand  writes:
> 
> > All,
> >
> > I've seen discussions come up several times lately about whether you should
> > use MAYBE_UNUSED or UNUSED in what scenario and why do we have two of them
> > anyway.  That got me thinking a bit.  Maybe what we actually want instead
> > of MAYBE_UNUSED is something like this:
> >
> > #ifdef NDEBUG
> > #define ASSERTED UNUSED
> > #else
> > #define ASSERTED
> > #endif
> >
> > That way, if you only need a parameter for asserts, you can declare it
> > ASSERTED and it won't warn in release builds will still throw a warning if
> > you do a debug build which doesn't use it.  Of course, there are other
> > times when something is validly MAYBE_UNUSED such as auto-generated code or
> > the genX code we use on Intel.  However, this provides additional meaning
> > and means the compiler warnings are still useful even after you've
> > relegated a value to assert-only.
> >
> > Thoughts?  I'm also open to a better name; that's the best I could do in 5
> > minutes.
> 
> We should delete one or the other of the current ones and not have
> different names to need to know for the same underlying function
> attribute.  I'd prefer deleting MAYBE_UNUSED (UNUSED is less typing and
> the name of the underlying attribute), but would go either way.

I agree, I'd be happy to see MAYBE_UNUSED go.  The actual compiler
attribute is named "unused".  Jason's idea sounds interesting, too,
in that it lets us get warnings about unused code in debug builds,
but still silence release warnings about values we assert on.  I'd
be okay with adding one of those and using that.

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/5] st/dri: flush before throttling in SwapBuffers

[Kenneth Graunke]

On Monday, April 22, 2019 6:29:43 PM PDT Marek Olšák wrote:
> From: Marek Olšák 
> 
> for better CPU-GPU parallelism
> ---
>  src/gallium/state_trackers/dri/dri_drawable.c | 20 +--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/dri/dri_drawable.c 
> b/src/gallium/state_trackers/dri/dri_drawable.c
> index 26bfdbecc53..c1de3bed9dd 100644
> --- a/src/gallium/state_trackers/dri/dri_drawable.c
> +++ b/src/gallium/state_trackers/dri/dri_drawable.c
> @@ -555,33 +555,33 @@ dri_flush(__DRIcontext *cPriv,
> *
> * This pulls a fence off the throttling queue and waits for it if the
> * number of fences on the throttling queue has reached the desired
> * number.
> *
> * Then flushes to insert a fence at the current rendering position, 
> and
> * pushes that fence on the queue. This requires that the 
> st_context_iface
> * flush method returns a fence even if there are no commands to flush.
> */
>struct pipe_screen *screen = drawable->screen->base.screen;
> -  struct pipe_fence_handle *fence;
> +  struct pipe_fence_handle *oldest_fence, *new_fence = NULL;
>  
> -  fence = swap_fences_pop_front(drawable);
> -  if (fence) {
> - (void) screen->fence_finish(screen, NULL, fence, 
> PIPE_TIMEOUT_INFINITE);
> - screen->fence_reference(screen, , NULL);
> -  }
> +  st->flush(st, flush_flags, _fence);
>  
> -  st->flush(st, flush_flags, );
> +  oldest_fence = swap_fences_pop_front(drawable);
> +  if (oldest_fence) {
> + screen->fence_finish(screen, NULL, oldest_fence, 
> PIPE_TIMEOUT_INFINITE);
> + screen->fence_reference(screen, _fence, NULL);
> +  }
>  
> -  if (fence) {
> - swap_fences_push_back(drawable, fence);
> - screen->fence_reference(screen, , NULL);
> +  if (new_fence) {
> + swap_fences_push_back(drawable, new_fence);
> + screen->fence_reference(screen, _fence, NULL);
>}
> }
> else if (flags & (__DRI2_FLUSH_DRAWABLE | __DRI2_FLUSH_CONTEXT)) {
>st->flush(st, flush_flags, NULL);
> }
>  
> if (drawable) {
>drawable->flushing = FALSE;
> }

It seems like this will let us submit one more batch before throttling,
which is a little like increasing the throttle value from 2 to 3...but
not exactly.  I'm not sure I have an opinion on which is better...

The rest of the series is:
Reviewed-by: Kenneth Graunke 

It looks like iris ends up with PIPE_CAP_MAX_FRAMES_IN_FLIGHT == 0 at
the end of the series.  It was 2 before I botched the DRM_CONF config
when adding driconf XML support, so I'd probably like it to be that way
again.  Feel free to just fix that in your patch that introduces the
cap, or I can push a patch after your series lands.

Thanks for cleaning this up, it's nice to have one fewer place to
configure this sort of stuff.


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Mathias Fröhlich]

Hi,

On Tuesday, 23 April 2019 22:23:45 CEST Marek Olšák wrote:
> On Tue, Apr 23, 2019 at 4:05 PM Mathias Fröhlich 
> wrote:
> 
> > Hi Marek,
> >
> > On Tuesday, 23 April 2019 20:22:15 CEST Marek Olšák wrote:
> > > I'd like to remove swrast from devices. It doesn't work (eglInitialize
> > > fails) and I don't think I like swrast there. Any objections?
> >
> > Yes, how do you guarantee that at least one device can be returned
> > in any case? Even if no drm device is found?
> > The egl device query extension guarantees that there is at least one
> > device available.
> > Beside that swrast may make sense for itself, that is the reason
> > the swrast device is there.
> >
> 
> If no device can be returned, the extension won't be exposed.
Yes, I was triggering this solution back in the days. There was (is?) no 
infrastructure to enable
or disable an egl extension just past initialization or at runtime.
Finally Emil took the route to add swrast.

The longer I saw the swrast device the better It appeared to me.
... from the users point of view.

>OR
> If there is at least 1 drm device, swrast won't be in the list.
> 
> From my perspective, radeonsi is always in the list and I wouldn't like
> swrast to be in the list if radeonsi is in the list.

I see - at least I think. But really the swrast device is good as is. I see 
plenty use cases for that.
Having that only present when there is no hardware device makes it less useful.

What I don't like with the current solution is that you get the swrast as the 
first device on the list.
The usual application that I know of just uses something closely derived from 
the nvidia sample code.
And that one just takes the first device. So I personally want to make sure 
that the first
device is a hardware accelerated device. Putting the swrast device at the end 
of the list
would be sufficient I think.

best

Mathias


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

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Marek Olšák]

On Tue, Apr 23, 2019 at 4:05 PM Mathias Fröhlich 
wrote:

> Hi Marek,
>
> On Tuesday, 23 April 2019 20:22:15 CEST Marek Olšák wrote:
> > I'd like to remove swrast from devices. It doesn't work (eglInitialize
> > fails) and I don't think I like swrast there. Any objections?
>
> Yes, how do you guarantee that at least one device can be returned
> in any case? Even if no drm device is found?
> The egl device query extension guarantees that there is at least one
> device available.
> Beside that swrast may make sense for itself, that is the reason
> the swrast device is there.
>

If no device can be returned, the extension won't be exposed.
   OR
If there is at least 1 drm device, swrast won't be in the list.

>From my perspective, radeonsi is always in the list and I wouldn't like
swrast to be in the list if radeonsi is in the list.

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

Re: [Mesa-dev] [PATCH] radeonsi: use CP DMA for the null const buffer clear on CIK

[Marek Olšák]

No, the correct backport is attached.

Marek

On Tue, Apr 23, 2019 at 2:51 PM Dylan Baker  wrote:

> Hi Marek and Samuel,
>
> I've staged this for 19.0, but I had to fix some very minor rebase
> conflicts.
> I'm getting ready to make a release, could one of you take a peak at the
> tip of
> the staging/19.0 branch and let me know if what I did looks okay?
>
> Thanks,
> Dylan
>
> Quoting Samuel Pitoiset (2019-04-16 08:24:01)
> > I don't have much context for that issue, so:
> >
> > Acked-by: Samuel Pitoiset 
> >
> > On 4/12/19 10:15 PM, Marek Ol\u0161 k wrote:
> >
> > Done locally.
> >
> > Marek
> >
> > On Fri, Apr 12, 2019 at 12:20 PM Samuel Pitoiset <
> samuel.pitoi...@gmail.com
> > > wrote:
> >
> > I would suggest to document that workaround somewhere in the
> code.
> >
> > On 4/12/19 5:17 PM, Marek Ol\u0161 k wrote:
> > > From: Marek Ol\u0161 k 
> > >
> > > This is a workaround for a thread deadlock that I have no idea
> > > why it occurs.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108879
> > > Fixes: 9b331e462e5021d994859756d46cd2519d9c9c6e
> > > ---
> > >   src/gallium/drivers/radeonsi/si_clear.c| 6 +++---
> > >   src/gallium/drivers/radeonsi/si_compute_blit.c | 8 +---
> > >   src/gallium/drivers/radeonsi/si_pipe.c | 2 +-
> > >   src/gallium/drivers/radeonsi/si_pipe.h | 3 ++-
> > >   src/gallium/drivers/radeonsi/si_test_dma.c | 2 +-
> > >   5 files changed, 12 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/gallium/drivers/radeonsi/si_clear.c
> b/src/gallium/
> > drivers/radeonsi/si_clear.c
> > > index e1805f2a1c9..ead680b857b 100644
> > > --- a/src/gallium/drivers/radeonsi/si_clear.c
> > > +++ b/src/gallium/drivers/radeonsi/si_clear.c
> > > @@ -256,21 +256,21 @@ void vi_dcc_clear_level(struct si_context
> > *sctx,
> > >* would be more efficient than separate
> per-layer
> > clear operations.
> > >*/
> > >   assert(tex->buffer.b.b.nr_storage_samples <= 2 ||
> > num_layers == 1);
> > >
> > >   dcc_offset += tex->surface.u.legacy.level
> > [level].dcc_offset;
> > >   clear_size = tex->surface.u.legacy.level
> > [level].dcc_fast_clear_size *
> > >num_layers;
> > >   }
> > >
> > >   si_clear_buffer(sctx, dcc_buffer, dcc_offset, clear_size,
> > > - _value, 4, SI_COHERENCY_CB_META);
> > > + _value, 4, SI_COHERENCY_CB_META,
> false);
> > >   }
> > >
> > >   /* Set the same micro tile mode as the destination of the
> last MSAA
> > resolve.
> > >* This allows hitting the MSAA resolve fast path, which
> requires
> > that both
> > >* src and dst micro tile modes match.
> > >*/
> > >   static void si_set_optimal_micro_tile_mode(struct si_screen
> > *sscreen,
> > >  struct si_texture
> *tex)
> > >   {
> > >   if (tex->buffer.b.is_shared ||
> > > @@ -489,21 +489,21 @@ static void si_do_fast_color_clear(struct
> > si_context *sctx,
> > >
> > >   /* DCC fast clear with MSAA should clear
> CMASK
> > to 0xC. */
> > >   if (tex->buffer.b.b.nr_samples >= 2 &&
> tex->
> > cmask_buffer) {
> > >   /* TODO: This doesn't work with
> MSAA. *
> > /
> > >   if (eliminate_needed)
> > >   continue;
> > >
> > >   uint32_t clear_value =
> 0x;
> > >   si_clear_buffer(sctx, >
> > cmask_buffer->b.b,
> > >
>  tex->cmask_offset,
> > tex->surface.cmask_size,
> > > - _value, 4,
> > SI_COHERENCY_CB_META);
> > > + _value, 4,
> > SI_COHERENCY_CB_META, false);
> > >   fmask_decompress_needed = true;
> > >   }
> > >
> > >   vi_dcc_clear_level(sctx, tex, 0,
> reset_value);
> > >   tex->separate_dcc_dirty = true;
> > >   } else {
> > >   if (too_small)
> > >   continue;
> > >
> > >   /* 128-bit formats are unusupported */
> > > @@ -517,21 +517,21 @@ static 

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Mathias Fröhlich]

Hi Marek,

On Tuesday, 23 April 2019 20:22:15 CEST Marek Olšák wrote:
> I'd like to remove swrast from devices. It doesn't work (eglInitialize
> fails) and I don't think I like swrast there. Any objections?

Yes, how do you guarantee that at least one device can be returned
in any case? Even if no drm device is found?
The egl device query extension guarantees that there is at least one device 
available.
Beside that swrast may make sense for itself, that is the reason
the swrast device is there.

Also, I think I found what crashes. Finally a test bug. The change to piglit is 
sent to that list.
I cleaned up the patch series with the egl extension a bit on my gitlab repo.
That with the updated test seems to work here.
Still, it would be good if you could test the part that changes the visuals and 
the pbuffers.

best

Mathias


> 
> Marek
> 
> On Wed, Apr 17, 2019 at 12:38 AM Mathias Fröhlich 
> wrote:
> 
> >
> > Hi,
> >
> > On Tuesday, 16 April 2019 17:50:33 CEST Marek Olšák wrote:
> > > On Wed, Apr 10, 2019 at 5:37 AM Mathias Fröhlich <
> > mathias.froehl...@gmx.net>
> > > wrote:
> > >
> > > > Hi Emil,
> > > >
> > > > On Monday, 8 April 2019 12:28:55 CEST Emil Velikov wrote:
> > > > > > Now that I have been putting together a test case for the the
> > actual
> > > > use
> > > > > > I do see some issues with the pbuffer code path. Well - still
> > > > investigating
> > > > > > if the test is wrong or the result.
> > > > > >
> > > > > Actually I do recall some issues with surfaceless (which is
> > > > > practically identical to this code) and some (with alpha?) formats.
> > > > > My memory is pretty flaky - I would suggest trying various configs:
> > > > > with, w/o alpha, 32/24 etc.
> > > >
> > > > Ok, yesterday evening I spent some time to look into that.
> > > > Finally the egl config mechanism puts double buffer visuals
> > > > to the pbuffer surface which in turn makes mesa think if should look
> > > > at the back buffer which is in turn not set in the pbuffer
> > gl_framebuffer.
> > > > That one makes my drivers just throw away the rendering result so a
> > > > read back does not show anything usable.
> > > >
> > > > With that fixed I get in principle correct results.
> > > >
> > > > My gitlab repo contains a branch that sketches my quick fix that I use
> > to
> > > > make
> > > > at least the new piglit test case work. No further testing done.
> > > > But may be that helps in getting something reasonable out.
> > > >
> > >
> > > So we can push this series now, right?
> >
> > Have you been looking at that branch?
> >
> > Its a sketch at best, and under some circumstance it still crashes
> > somewhere
> > in the back and forth of virtual methods in the loader and driver.
> > I think it's going the riht direction, but until that is fixed I don't
> > want to have my
> > name tag on that unfinished stuff.
> >
> > If you want to help out, you can take a look at the egl config change in
> > that branch. One needs to check if that 'double buffering visual' filter
> > there
> > works as expected. Throw test cases at it. At least punch that code through
> > piglit and whatever test suite you have access.
> >
> > best
> >
> > Mathias
> >
> >
> >
> 




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

Re: [Mesa-dev] [PATCH] radeonsi: use CP DMA for the null const buffer clear on CIK

[Samuel Pitoiset]

Looks good to me.

On 4/23/19 8:51 PM, Dylan Baker wrote:

Hi Marek and Samuel,

I've staged this for 19.0, but I had to fix some very minor rebase conflicts.
I'm getting ready to make a release, could one of you take a peak at the tip of
the staging/19.0 branch and let me know if what I did looks okay?

Thanks,
Dylan

Quoting Samuel Pitoiset (2019-04-16 08:24:01)

I don't have much context for that issue, so:

Acked-by: Samuel Pitoiset 

On 4/12/19 10:15 PM, Marek Ol\u0161 k wrote:

 Done locally.

 Marek

 On Fri, Apr 12, 2019 at 12:20 PM Samuel Pitoiset  wrote:

 I would suggest to document that workaround somewhere in the code.

 On 4/12/19 5:17 PM, Marek Ol\u0161 k wrote:
 > From: Marek Ol\u0161 k 
 >
 > This is a workaround for a thread deadlock that I have no idea
 > why it occurs.
 >
 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108879
 > Fixes: 9b331e462e5021d994859756d46cd2519d9c9c6e
 > ---
 >   src/gallium/drivers/radeonsi/si_clear.c| 6 +++---
 >   src/gallium/drivers/radeonsi/si_compute_blit.c | 8 +---
 >   src/gallium/drivers/radeonsi/si_pipe.c | 2 +-
 >   src/gallium/drivers/radeonsi/si_pipe.h | 3 ++-
 >   src/gallium/drivers/radeonsi/si_test_dma.c | 2 +-
 >   5 files changed, 12 insertions(+), 9 deletions(-)
 >
 > diff --git a/src/gallium/drivers/radeonsi/si_clear.c b/src/gallium/
 drivers/radeonsi/si_clear.c
 > index e1805f2a1c9..ead680b857b 100644
 > --- a/src/gallium/drivers/radeonsi/si_clear.c
 > +++ b/src/gallium/drivers/radeonsi/si_clear.c
 > @@ -256,21 +256,21 @@ void vi_dcc_clear_level(struct si_context
 *sctx,
 >* would be more efficient than separate per-layer
 clear operations.
 >*/
 >   assert(tex->buffer.b.b.nr_storage_samples <= 2 ||
 num_layers == 1);
 >
 >   dcc_offset += tex->surface.u.legacy.level
 [level].dcc_offset;
 >   clear_size = tex->surface.u.legacy.level
 [level].dcc_fast_clear_size *
 >num_layers;
 >   }
 >
 >   si_clear_buffer(sctx, dcc_buffer, dcc_offset, clear_size,
 > - _value, 4, SI_COHERENCY_CB_META);
 > + _value, 4, SI_COHERENCY_CB_META, false);
 >   }
 >
 >   /* Set the same micro tile mode as the destination of the last MSAA
 resolve.
 >* This allows hitting the MSAA resolve fast path, which requires
 that both
 >* src and dst micro tile modes match.
 >*/
 >   static void si_set_optimal_micro_tile_mode(struct si_screen
 *sscreen,
 >  struct si_texture *tex)
 >   {
 >   if (tex->buffer.b.is_shared ||
 > @@ -489,21 +489,21 @@ static void si_do_fast_color_clear(struct
 si_context *sctx,
 >
 >   /* DCC fast clear with MSAA should clear CMASK
 to 0xC. */
 >   if (tex->buffer.b.b.nr_samples >= 2 && tex->
 cmask_buffer) {
 >   /* TODO: This doesn't work with MSAA. *
 /
 >   if (eliminate_needed)
 >   continue;
 >
 >   uint32_t clear_value = 0x;
 >   si_clear_buffer(sctx, >
 cmask_buffer->b.b,
 >   tex->cmask_offset,
 tex->surface.cmask_size,
 > - _value, 4,
 SI_COHERENCY_CB_META);
 > + _value, 4,
 SI_COHERENCY_CB_META, false);
 >   fmask_decompress_needed = true;
 >   }
 >
 >   vi_dcc_clear_level(sctx, tex, 0, reset_value);
 >   tex->separate_dcc_dirty = true;
 >   } else {
 >   if (too_small)
 >   continue;
 >
 >   /* 128-bit formats are unusupported */
 > @@ -517,21 +517,21 @@ static void si_do_fast_color_clear(struct
 si_context *sctx,
 >
 >   /* ensure CMASK is enabled */
 >   si_alloc_separate_cmask(sctx->screen, tex);
 >   if (!tex->cmask_buffer)
 >   continue;
 >
 >   /* Do the 

[Mesa-dev] [Bug 110323] mesa-19.0.1 fails to compile with llvm-8.0.0 and Gallium swr driver

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110323

--- Comment #6 from Jan Zielinski  ---
That's correct, worked in 18.0.0 and 18.1.0. The change breaking 32-bit
compilation was introduced in 18.2.0. 

It should be fixed in one of the upcoming releases. 

As a workaround (again!) to build 32-bit version you can simply remove
SIMDVecHash struct definition from simdlib.hpp.

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

Re: [Mesa-dev] [PATCH] radeonsi: use CP DMA for the null const buffer clear on CIK

[Dylan Baker]

Hi Marek and Samuel,

I've staged this for 19.0, but I had to fix some very minor rebase conflicts.
I'm getting ready to make a release, could one of you take a peak at the tip of
the staging/19.0 branch and let me know if what I did looks okay?

Thanks,
Dylan

Quoting Samuel Pitoiset (2019-04-16 08:24:01)
> I don't have much context for that issue, so:
> 
> Acked-by: Samuel Pitoiset 
> 
> On 4/12/19 10:15 PM, Marek Ol\u0161 k wrote:
> 
> Done locally.
> 
> Marek
> 
> On Fri, Apr 12, 2019 at 12:20 PM Samuel Pitoiset 
>  > wrote:
> 
> I would suggest to document that workaround somewhere in the code.
> 
> On 4/12/19 5:17 PM, Marek Ol\u0161 k wrote:
> > From: Marek Ol\u0161 k 
> >
> > This is a workaround for a thread deadlock that I have no idea
> > why it occurs.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108879
> > Fixes: 9b331e462e5021d994859756d46cd2519d9c9c6e
> > ---
> >   src/gallium/drivers/radeonsi/si_clear.c| 6 +++---
> >   src/gallium/drivers/radeonsi/si_compute_blit.c | 8 +---
> >   src/gallium/drivers/radeonsi/si_pipe.c | 2 +-
> >   src/gallium/drivers/radeonsi/si_pipe.h | 3 ++-
> >   src/gallium/drivers/radeonsi/si_test_dma.c | 2 +-
> >   5 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_clear.c b/src/gallium/
> drivers/radeonsi/si_clear.c
> > index e1805f2a1c9..ead680b857b 100644
> > --- a/src/gallium/drivers/radeonsi/si_clear.c
> > +++ b/src/gallium/drivers/radeonsi/si_clear.c
> > @@ -256,21 +256,21 @@ void vi_dcc_clear_level(struct si_context
> *sctx,
> >* would be more efficient than separate per-layer
> clear operations.
> >*/
> >   assert(tex->buffer.b.b.nr_storage_samples <= 2 ||
> num_layers == 1);
> >   
> >   dcc_offset += tex->surface.u.legacy.level
> [level].dcc_offset;
> >   clear_size = tex->surface.u.legacy.level
> [level].dcc_fast_clear_size *
> >num_layers;
> >   }
> >   
> >   si_clear_buffer(sctx, dcc_buffer, dcc_offset, clear_size,
> > - _value, 4, SI_COHERENCY_CB_META);
> > + _value, 4, SI_COHERENCY_CB_META, false);
> >   }
> >   
> >   /* Set the same micro tile mode as the destination of the last 
> MSAA
> resolve.
> >* This allows hitting the MSAA resolve fast path, which requires
> that both
> >* src and dst micro tile modes match.
> >*/
> >   static void si_set_optimal_micro_tile_mode(struct si_screen
> *sscreen,
> >  struct si_texture *tex)
> >   {
> >   if (tex->buffer.b.is_shared ||
> > @@ -489,21 +489,21 @@ static void si_do_fast_color_clear(struct
> si_context *sctx,
> >   
> >   /* DCC fast clear with MSAA should clear CMASK
> to 0xC. */
> >   if (tex->buffer.b.b.nr_samples >= 2 && tex->
> cmask_buffer) {
> >   /* TODO: This doesn't work with MSAA. 
> *
> /
> >   if (eliminate_needed)
> >   continue;
> >   
> >   uint32_t clear_value = 0x;
> >   si_clear_buffer(sctx, >
> cmask_buffer->b.b,
> >   tex->cmask_offset,
> tex->surface.cmask_size,
> > - _value, 4,
> SI_COHERENCY_CB_META);
> > + _value, 4,
> SI_COHERENCY_CB_META, false);
> >   fmask_decompress_needed = true;
> >   }
> >   
> >   vi_dcc_clear_level(sctx, tex, 0, reset_value);
> >   tex->separate_dcc_dirty = true;
> >   } else {
> >   if (too_small)
> >   continue;
> >   
> >   /* 128-bit formats are unusupported */
> > @@ -517,21 +517,21 @@ static void si_do_fast_color_clear(struct
> si_context *sctx,
> >   
> >   /* ensure CMASK is enabled */
> >   si_alloc_separate_cmask(sctx->screen, tex);
> >   if (!tex->cmask_buffer)
> >  

[Mesa-dev] [Bug 100105] Make Theano OpenCL support work on Clover and RadeonSI

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100105

--- Comment #13 from b...@besd.de  ---
Unfortunately it turns out that even a working opencl (used the closed AMD
legacy driver) isnt getting the application i'm trying to run (see above) to
work. At some point it complains that a data structure requires cuda (somewhere
in libgpuarray).

Which could probably be fixed if theano was still being maintained.
Which it sort of isnt (pymc3 still uses it and they want to maintain what they
use of it)
However pymc4 is using tensorflow which as AFAICT is CUDA only.

So what I really want is probably cuda on opencl
(https://github.com/hughperkins/coriander) but that requires opencl 1.2.

back to square one ;)

-- 
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

[Mesa-dev] [Bug 100105] Make Theano OpenCL support work on Clover and RadeonSI

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100105

--- Comment #12 from b...@besd.de  ---
Seems about right

CLOVER_DEVICE_VERSION_OVERRIDE=1.2 CLOVER_DEVICE_CLC_VERSION_OVERRIDE=1.2
DEVICE="opencl0:0" python3 -c "import pygpu;pygpu.test()"
pygpu is installed in
/usr/local/lib/python3.6/dist-packages/pygpu-0.7.6+20.g9cec614-py3.6-linux-x86_64.egg/pygpu
NumPy version 1.16.3
NumPy relaxed strides checking option: True
NumPy is installed in /home/nano/.local/lib/python3.6/site-packages/numpy
Python version 3.6.7 (default, Oct 22 2018, 11:32:17) [GCC 8.2.0]
nose version 1.3.7
*** Testing for Radeon RX 560 Series (POLARIS11, DRM 3.30.0, 4.15.0-47-generic,
LLVM 8.0.0)
mpi4py found: True

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

[Marek Olšák]

I'd like to remove swrast from devices. It doesn't work (eglInitialize
fails) and I don't think I like swrast there. Any objections?

Marek

On Wed, Apr 17, 2019 at 12:38 AM Mathias Fröhlich 
wrote:

>
> Hi,
>
> On Tuesday, 16 April 2019 17:50:33 CEST Marek Olšák wrote:
> > On Wed, Apr 10, 2019 at 5:37 AM Mathias Fröhlich <
> mathias.froehl...@gmx.net>
> > wrote:
> >
> > > Hi Emil,
> > >
> > > On Monday, 8 April 2019 12:28:55 CEST Emil Velikov wrote:
> > > > > Now that I have been putting together a test case for the the
> actual
> > > use
> > > > > I do see some issues with the pbuffer code path. Well - still
> > > investigating
> > > > > if the test is wrong or the result.
> > > > >
> > > > Actually I do recall some issues with surfaceless (which is
> > > > practically identical to this code) and some (with alpha?) formats.
> > > > My memory is pretty flaky - I would suggest trying various configs:
> > > > with, w/o alpha, 32/24 etc.
> > >
> > > Ok, yesterday evening I spent some time to look into that.
> > > Finally the egl config mechanism puts double buffer visuals
> > > to the pbuffer surface which in turn makes mesa think if should look
> > > at the back buffer which is in turn not set in the pbuffer
> gl_framebuffer.
> > > That one makes my drivers just throw away the rendering result so a
> > > read back does not show anything usable.
> > >
> > > With that fixed I get in principle correct results.
> > >
> > > My gitlab repo contains a branch that sketches my quick fix that I use
> to
> > > make
> > > at least the new piglit test case work. No further testing done.
> > > But may be that helps in getting something reasonable out.
> > >
> >
> > So we can push this series now, right?
>
> Have you been looking at that branch?
>
> Its a sketch at best, and under some circumstance it still crashes
> somewhere
> in the back and forth of virtual methods in the loader and driver.
> I think it's going the riht direction, but until that is fixed I don't
> want to have my
> name tag on that unfinished stuff.
>
> If you want to help out, you can take a look at the egl config change in
> that branch. One needs to check if that 'double buffering visual' filter
> there
> works as expected. Throw test cases at it. At least punch that code through
> piglit and whatever test suite you have access.
>
> best
>
> Mathias
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 100105] Make Theano OpenCL support work on Clover and RadeonSI

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100105

--- Comment #11 from Jan Vesely  ---
It has been some time that I ran theano. Does the error happen even if it's
built without clBLAS support?

clBLAS depends on CL1.2 features which are not implemented, yet. (hence the
dependence on 94273)

-- 
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

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Samuel Pitoiset]

On 4/23/19 5:16 PM, Jason Ekstrand wrote:
On Tue, Apr 23, 2019 at 7:46 AM Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:



On 4/23/19 10:45 AM, Bas Nieuwenhuizen wrote:
> On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
> mailto:samuel.pitoi...@gmail.com>>
wrote:
>> Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
>> ---
>>   src/amd/vulkan/radv_shader.c                 |  2 +-
>>   src/compiler/nir/nir.h                       |  3 ++-
>>   src/compiler/nir/nir_opt_if.c                | 17
++---
>>   src/freedreno/ir3/ir3_nir.c                  |  2 +-
>>   src/gallium/auxiliary/nir/tgsi_to_nir.c      |  2 +-
>>   src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
>>   src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
>>   src/intel/compiler/brw_nir.c                 |  2 +-
>>   src/mesa/state_tracker/st_glsl_to_nir.cpp    |  2 +-
>>   9 files changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_shader.c
b/src/amd/vulkan/radv_shader.c
>> index 13f1f9aa9dc..54a4e732230 100644
>> --- a/src/amd/vulkan/radv_shader.c
>> +++ b/src/amd/vulkan/radv_shader.c
>> @@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader
*shader, bool optimize_conservatively,
>>                          NIR_PASS(progress, shader,
nir_opt_remove_phis);
>>                           NIR_PASS(progress, shader, nir_opt_dce);
>>                   }
>> -                NIR_PASS(progress, shader, nir_opt_if, true);
>> +                NIR_PASS(progress, shader, nir_opt_if, true,
false);
>>                   NIR_PASS(progress, shader, nir_opt_dead_cf);
>>                   NIR_PASS(progress, shader, nir_opt_cse);
>>                   NIR_PASS(progress, shader,
nir_opt_peephole_select, 8, true, true);
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index 7d2062d3691..d7506d6ddd1 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool
value_number);
>>
>>   bool nir_opt_idiv_const(nir_shader *shader, unsigned
min_bit_size);
>>
>> -bool nir_opt_if(nir_shader *shader, bool
aggressive_last_continue);
>> +bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
>> +                bool skip_alu_of_phi);
> Can we have a flag for this instead (e.g. something like
> nir_opt_if_skip_alu_of_phi)? I think have a function with a bunch of
> bools is less than ideal as you can't see at the calling site
what is
> for what arg.
Yes, that seems better to me.


This is the worst kind of hack all around.  We're making NIR more 
complicated and adding a flag to disable a useful and correct piece of 
an optimization, not because it causes a perf regression but because 
the back-end compiler is broken and this is easier than fixing it 
properly. Seriously?  Can't we just fix the LLVM back-end?  Or, if 
this optimization is actually doing something wrong, fix it?  Or maybe 
actually figure out what pattern is causing LLVM to fall over and have 
a hack in your NIR -> LLVM pass?  On the list of "good ways to fix 
this problem", this seems to be pretty far down if it hasn't fallen 
off the bottom.


Best hack of the month? :-)

As discussed over IRC, this is definitely not the best solution, I don't 
like it either as I said.


I will work on a different solution maybe in our NIR->LLVM pass.

The aggressive_last_continue option should also be removed (make it 
default?).




--Jason

>>   bool nir_opt_intrinsics(nir_shader *shader);
>>
>> diff --git a/src/compiler/nir/nir_opt_if.c
b/src/compiler/nir/nir_opt_if.c
>> index f674185f1e2..149b3bd1659 100644
>> --- a/src/compiler/nir/nir_opt_if.c
>> +++ b/src/compiler/nir/nir_opt_if.c
>> @@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct
exec_list *cf_list,
>>    * not do anything to cause the metadata to become invalid.
>>    */
>>   static bool
>> -opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
>> +opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list,
>> +                    bool skip_alu_of_phi)
>>   {
>>      bool progress = false;
>>      foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
>> @@ -1395,16 +1396,17 @@ opt_if_safe_cf_list(nir_builder *b,
struct exec_list *cf_list)
>>
>>         case nir_cf_node_if: {
>>            nir_if *nif = nir_cf_node_as_if(cf_node);
>> -         progress |= opt_if_safe_cf_list(b, >then_list);
>> -         progress |= opt_if_safe_cf_list(b, >else_list);
>> +         progress |= opt_if_safe_cf_list(b, >then_list,
skip_alu_of_phi);
>> +         progress |= opt_if_safe_cf_list(b, >else_list,
skip_alu_of_phi);
>>            

Re: [Mesa-dev] [PATCH] st/mesa/radeonsi: fix race between destruction of types and shader compilation

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Tue, Apr 23, 2019 at 2:04 AM Timothy Arceri 
wrote:

> Commit 624789e3708c moved the destruction of types out of atexit() and
> made use of a ref count instead. This is useful for avoiding a crash
> where drivers such as radeonsi are still compiling in a thread when the app
> exits and has not called MakeCurrent to change from the current context.
>
> While the above scenario is technically an app bug we shouldn't crash.
> However that change caused another race condition between the shader
> compilation tread in radeonsi and context teardown functions.
>
> This patch makes two changes to fix this new problem:
>
> First we explicitly call _mesa_destroy_shader_compiler_types() when
> destroying
> the st context rather than calling it indirectly via
> _mesa_free_context_data().
> We do this as we must call it after st_destroy_context_priv() so that we
> don't
> destory the glsl types before the compilation threads finish.
>
> Next wait for the shader threads to finish in si_destroy_context() this
> also means we need to call context destroy before destroying the queues
> in si_destroy_screen().
>
> Fixes: 624789e3708c ("compiler/glsl: handle case where we have multiple
> users for types")
> ---
>  src/compiler/glsl/glsl_parser_extras.h  | 1 +
>  src/gallium/drivers/radeonsi/si_pipe.c  | 8 ++--
>  src/mesa/drivers/dri/i915/intel_context.c   | 2 +-
>  src/mesa/drivers/dri/i965/brw_context.c | 2 +-
>  src/mesa/drivers/dri/nouveau/nouveau_context.c  | 2 +-
>  src/mesa/drivers/dri/radeon/radeon_common_context.c | 2 +-
>  src/mesa/drivers/osmesa/osmesa.c| 8 
>  src/mesa/drivers/x11/xm_api.c   | 4 ++--
>  src/mesa/main/context.c | 7 ---
>  src/mesa/main/context.h | 2 +-
>  src/mesa/state_tracker/st_context.c | 8 +++-
>  11 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_parser_extras.h
> b/src/compiler/glsl/glsl_parser_extras.h
> index f92d2160aac..edc6fc06c77 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -997,6 +997,7 @@ extern "C" {
>  #endif
>
>  struct glcpp_parser;
> +struct _mesa_glsl_parse_state;
>
>  typedef void (*glcpp_extension_iterator)(
>struct _mesa_glsl_parse_state *state,
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index fa96ce34224..e9e1bd0aa38 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -150,6 +150,9 @@ static void si_destroy_context(struct pipe_context
> *context)
> struct si_context *sctx = (struct si_context *)context;
> int i;
>
> +   util_queue_finish(>screen->shader_compiler_queue);
> +
>  util_queue_finish(>screen->shader_compiler_queue_low_priority);
> +
> /* Unreference the framebuffer normally to disable related logic
>  * properly.
>  */
> @@ -702,6 +705,9 @@ static void si_destroy_screen(struct pipe_screen*
> pscreen)
> if (!sscreen->ws->unref(sscreen->ws))
> return;
>
> +   mtx_destroy(>aux_context_lock);
> +   sscreen->aux_context->destroy(sscreen->aux_context);
> +
> util_queue_destroy(>shader_compiler_queue);
> util_queue_destroy(>shader_compiler_queue_low_priority);
>
> @@ -728,8 +734,6 @@ static void si_destroy_screen(struct pipe_screen*
> pscreen)
> si_gpu_load_kill_thread(sscreen);
>
> mtx_destroy(>gpu_load_mutex);
> -   mtx_destroy(>aux_context_lock);
> -   sscreen->aux_context->destroy(sscreen->aux_context);
>
> slab_destroy_parent(>pool_transfers);
>
> diff --git a/src/mesa/drivers/dri/i915/intel_context.c
> b/src/mesa/drivers/dri/i915/intel_context.c
> index c23e5ffb26e..aa3175816cf 100644
> --- a/src/mesa/drivers/dri/i915/intel_context.c
> +++ b/src/mesa/drivers/dri/i915/intel_context.c
> @@ -599,7 +599,7 @@ intelDestroyContext(__DRIcontext * driContextPriv)
>driDestroyOptionCache(>optionCache);
>
>/* free the Mesa context */
> -  _mesa_free_context_data(>ctx);
> +  _mesa_free_context_data(>ctx, true);
>
>_math_matrix_dtr(>ViewportMatrix);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index ab637ddf721..df12f373f22 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1226,7 +1226,7 @@ intelDestroyContext(__DRIcontext * driContextPriv)
> driDestroyOptionCache(>optionCache);
>
> /* free the Mesa context */
> -   _mesa_free_context_data(>ctx);
> +   _mesa_free_context_data(>ctx, true);
>
> ralloc_free(brw);
> driContextPriv->driverPrivate = NULL;
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_context.c
> 

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Jason Ekstrand]

On Tue, Apr 23, 2019 at 7:46 AM Samuel Pitoiset 
wrote:

>
> On 4/23/19 10:45 AM, Bas Nieuwenhuizen wrote:
> > On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
> >  wrote:
> >> Signed-off-by: Samuel Pitoiset 
> >> ---
> >>   src/amd/vulkan/radv_shader.c |  2 +-
> >>   src/compiler/nir/nir.h   |  3 ++-
> >>   src/compiler/nir/nir_opt_if.c| 17 ++---
> >>   src/freedreno/ir3/ir3_nir.c  |  2 +-
> >>   src/gallium/auxiliary/nir/tgsi_to_nir.c  |  2 +-
> >>   src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
> >>   src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
> >>   src/intel/compiler/brw_nir.c |  2 +-
> >>   src/mesa/state_tracker/st_glsl_to_nir.cpp|  2 +-
> >>   9 files changed, 19 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
> >> index 13f1f9aa9dc..54a4e732230 100644
> >> --- a/src/amd/vulkan/radv_shader.c
> >> +++ b/src/amd/vulkan/radv_shader.c
> >> @@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader *shader, bool
> optimize_conservatively,
> >>  NIR_PASS(progress, shader,
> nir_opt_remove_phis);
> >>   NIR_PASS(progress, shader, nir_opt_dce);
> >>   }
> >> -NIR_PASS(progress, shader, nir_opt_if, true);
> >> +NIR_PASS(progress, shader, nir_opt_if, true, false);
> >>   NIR_PASS(progress, shader, nir_opt_dead_cf);
> >>   NIR_PASS(progress, shader, nir_opt_cse);
> >>   NIR_PASS(progress, shader, nir_opt_peephole_select,
> 8, true, true);
> >> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >> index 7d2062d3691..d7506d6ddd1 100644
> >> --- a/src/compiler/nir/nir.h
> >> +++ b/src/compiler/nir/nir.h
> >> @@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool
> value_number);
> >>
> >>   bool nir_opt_idiv_const(nir_shader *shader, unsigned min_bit_size);
> >>
> >> -bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue);
> >> +bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
> >> +bool skip_alu_of_phi);
> > Can we have a flag for this instead (e.g. something like
> > nir_opt_if_skip_alu_of_phi)? I think have a function with a bunch of
> > bools is less than ideal as you can't see at the calling site what is
> > for what arg.
> Yes, that seems better to me.
>

This is the worst kind of hack all around.  We're making NIR more
complicated and adding a flag to disable a useful and correct piece of an
optimization, not because it causes a perf regression but because the
back-end compiler is broken and this is easier than fixing it properly.
Seriously?  Can't we just fix the LLVM back-end?  Or, if this optimization
is actually doing something wrong, fix it?  Or maybe actually figure out
what pattern is causing LLVM to fall over and have a hack in your NIR ->
LLVM pass?  On the list of "good ways to fix this problem", this seems to
be pretty far down if it hasn't fallen off the bottom.

--Jason


> >>   bool nir_opt_intrinsics(nir_shader *shader);
> >>
> >> diff --git a/src/compiler/nir/nir_opt_if.c
> b/src/compiler/nir/nir_opt_if.c
> >> index f674185f1e2..149b3bd1659 100644
> >> --- a/src/compiler/nir/nir_opt_if.c
> >> +++ b/src/compiler/nir/nir_opt_if.c
> >> @@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct exec_list
> *cf_list,
> >>* not do anything to cause the metadata to become invalid.
> >>*/
> >>   static bool
> >> -opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
> >> +opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list,
> >> +bool skip_alu_of_phi)
> >>   {
> >>  bool progress = false;
> >>  foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
> >> @@ -1395,16 +1396,17 @@ opt_if_safe_cf_list(nir_builder *b, struct
> exec_list *cf_list)
> >>
> >> case nir_cf_node_if: {
> >>nir_if *nif = nir_cf_node_as_if(cf_node);
> >> - progress |= opt_if_safe_cf_list(b, >then_list);
> >> - progress |= opt_if_safe_cf_list(b, >else_list);
> >> + progress |= opt_if_safe_cf_list(b, >then_list,
> skip_alu_of_phi);
> >> + progress |= opt_if_safe_cf_list(b, >else_list,
> skip_alu_of_phi);
> >>progress |= opt_if_evaluate_condition_use(b, nif);
> >>break;
> >> }
> >>
> >> case nir_cf_node_loop: {
> >>nir_loop *loop = nir_cf_node_as_loop(cf_node);
> >> - progress |= opt_if_safe_cf_list(b, >body);
> >> - progress |= opt_split_alu_of_phi(b, loop);
> >> + progress |= opt_if_safe_cf_list(b, >body,
> skip_alu_of_phi);
> >> + if (!skip_alu_of_phi)
> >> +progress |= opt_split_alu_of_phi(b, loop);
> >>break;
> >> }
> >>
> >> @@ -1417,7 +1419,8 @@ opt_if_safe_cf_list(nir_builder *b, struct
> exec_list *cf_list)
> >>  

Re: [Mesa-dev] [PATCH 1/3] pipebuffer: use new pb_usage_flags enum type

[Brian Paul]

It was committed in early March: b286e74df66e25cadd1c82d9ddc4d1fc3887b646

-Brian

On 04/23/2019 07:25 AM, Thomas Hellstrom wrote:

Hi, Brian,

Did this series get reviewed? I don't see any replies?

/Thomas


On Tue, 2019-03-05 at 20:48 -0700, Brian Paul wrote:

Use a new enum type instead of 'unsigned' to make things a bit more
understandable.
---
  src/gallium/auxiliary/pipebuffer/pb_buffer.h   | 34
++
  .../auxiliary/pipebuffer/pb_buffer_fenced.c|  6 ++--
  .../auxiliary/pipebuffer/pb_buffer_malloc.c|  4 +--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c |  4 +--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_debug.c |  6 ++--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_mm.c|  4 +--
  .../auxiliary/pipebuffer/pb_bufmgr_ondemand.c  |  4 +--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_pool.c  |  5 ++--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_slab.c  |  6 ++--
  src/gallium/auxiliary/pipebuffer/pb_validate.c |  2 +-
  src/gallium/auxiliary/pipebuffer/pb_validate.h |  2 +-
  11 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
index 33c2306..11f70ea 100644
--- a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
+++ b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
@@ -59,13 +59,22 @@ struct pb_vtbl;
  struct pb_validate;
  struct pipe_fence_handle;
  
+enum pb_usage_flags {

+   PB_USAGE_CPU_READ = (1 << 0),
+   PB_USAGE_CPU_WRITE = (1 << 1),
+   PB_USAGE_GPU_READ = (1 << 2),
+   PB_USAGE_GPU_WRITE = (1 << 3),
+   PB_USAGE_DONTBLOCK = (1 << 9),
+   PB_USAGE_UNSYNCHRONIZED = (1 << 10),
+};
  
-#define PB_USAGE_CPU_READ  (1 << 0)

-#define PB_USAGE_CPU_WRITE (1 << 1)
-#define PB_USAGE_GPU_READ  (1 << 2)
-#define PB_USAGE_GPU_WRITE (1 << 3)
-#define PB_USAGE_UNSYNCHRONIZED (1 << 10)
-#define PB_USAGE_DONTBLOCK (1 << 9)
+/* For error checking elsewhere */
+#define PB_USAGE_ALL (PB_USAGE_CPU_READ | \
+  PB_USAGE_CPU_WRITE | \
+  PB_USAGE_GPU_READ | \
+  PB_USAGE_GPU_WRITE | \
+  PB_USAGE_DONTBLOCK | \
+  PB_USAGE_UNSYNCHRONIZED)
  
  #define PB_USAGE_CPU_READ_WRITE \

 ( PB_USAGE_CPU_READ | PB_USAGE_CPU_WRITE )
@@ -82,7 +91,7 @@ struct pipe_fence_handle;
  struct pb_desc
  {
 unsigned alignment;
-   unsigned usage;
+   enum pb_usage_flags usage;
  };
  
  
@@ -100,7 +109,7 @@ struct pb_buffer

 struct pipe_reference  reference;
 unsigned   alignment;
 pb_sizesize;
-   unsigned   usage;
+   enum pb_usage_flagsusage;
  
 /**

  * Pointer to the virtual function table.
@@ -126,13 +135,13 @@ struct pb_vtbl
  * flags is bitmask of PB_USAGE_CPU_READ/WRITE.
  */
 void *(*map)( struct pb_buffer *buf,
- unsigned flags, void *flush_ctx );
+ enum pb_usage_flags flags, void *flush_ctx );
 
 void (*unmap)( struct pb_buffer *buf );
  
 enum pipe_error (*validate)( struct pb_buffer *buf,

  struct pb_validate *vl,
-unsigned flags );
+enum pb_usage_flags flags );
  
 void (*fence)( struct pb_buffer *buf,

struct pipe_fence_handle *fence );
@@ -160,7 +169,7 @@ struct pb_vtbl
   */
  static inline void *
  pb_map(struct pb_buffer *buf,
-   unsigned flags, void *flush_ctx)
+   enum pb_usage_flags flags, void *flush_ctx)
  {
 assert(buf);
 if (!buf)
@@ -201,7 +210,8 @@ pb_get_base_buffer( struct pb_buffer *buf,
  
  
  static inline enum pipe_error

-pb_validate(struct pb_buffer *buf, struct pb_validate *vl, unsigned
flags)
+pb_validate(struct pb_buffer *buf, struct pb_validate *vl,
+enum pb_usage_flags flags)
  {
 assert(buf);
 if (!buf)
diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
index 7421741..53b9ce0 100644
--- a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
+++ b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
@@ -139,7 +139,7 @@ struct fenced_buffer
  * A bitmask of PB_USAGE_CPU/GPU_READ/WRITE describing the
current
  * buffer usage.
  */
-   unsigned flags;
+   enum pb_usage_flags flags;
  
 unsigned mapcount;
  
@@ -662,7 +662,7 @@ fenced_buffer_destroy(struct pb_buffer *buf)
  
  static void *

  fenced_buffer_map(struct pb_buffer *buf,
-  unsigned flags, void *flush_ctx)
+  enum pb_usage_flags flags, void *flush_ctx)
  {
 struct fenced_buffer *fenced_buf = fenced_buffer(buf);
 struct fenced_manager *fenced_mgr = fenced_buf->mgr;
@@ -739,7 +739,7 @@ fenced_buffer_unmap(struct pb_buffer *buf)
  static enum pipe_error
  fenced_buffer_validate(struct pb_buffer *buf,
 struct pb_validate *vl,
-   unsigned 

[Mesa-dev] [Bug 109929] tgsi_to_nir.c:2111: undefined reference to `gl_nir_lower_samplers_as_deref'

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109929

--- Comment #16 from Timur Kristóf  ---
Can you guys try this branch?

https://gitlab.freedesktop.org/Venemo/mesa/commits/nir-lower-samplers

It seems to work for me at least on radeonsi, but I haven't been able to test
it thoroughly yet.

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

Re: [Mesa-dev] [PATCH 1/3] pipebuffer: use new pb_usage_flags enum type

[Thomas Hellstrom]

Hi, Brian,

Did this series get reviewed? I don't see any replies?

/Thomas


On Tue, 2019-03-05 at 20:48 -0700, Brian Paul wrote:
> Use a new enum type instead of 'unsigned' to make things a bit more
> understandable.
> ---
>  src/gallium/auxiliary/pipebuffer/pb_buffer.h   | 34
> ++
>  .../auxiliary/pipebuffer/pb_buffer_fenced.c|  6 ++--
>  .../auxiliary/pipebuffer/pb_buffer_malloc.c|  4 +--
>  src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c |  4 +--
>  src/gallium/auxiliary/pipebuffer/pb_bufmgr_debug.c |  6 ++--
>  src/gallium/auxiliary/pipebuffer/pb_bufmgr_mm.c|  4 +--
>  .../auxiliary/pipebuffer/pb_bufmgr_ondemand.c  |  4 +--
>  src/gallium/auxiliary/pipebuffer/pb_bufmgr_pool.c  |  5 ++--
>  src/gallium/auxiliary/pipebuffer/pb_bufmgr_slab.c  |  6 ++--
>  src/gallium/auxiliary/pipebuffer/pb_validate.c |  2 +-
>  src/gallium/auxiliary/pipebuffer/pb_validate.h |  2 +-
>  11 files changed, 45 insertions(+), 32 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
> b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
> index 33c2306..11f70ea 100644
> --- a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
> +++ b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
> @@ -59,13 +59,22 @@ struct pb_vtbl;
>  struct pb_validate;
>  struct pipe_fence_handle;
>  
> +enum pb_usage_flags {
> +   PB_USAGE_CPU_READ = (1 << 0),
> +   PB_USAGE_CPU_WRITE = (1 << 1),
> +   PB_USAGE_GPU_READ = (1 << 2),
> +   PB_USAGE_GPU_WRITE = (1 << 3),
> +   PB_USAGE_DONTBLOCK = (1 << 9),
> +   PB_USAGE_UNSYNCHRONIZED = (1 << 10),
> +};
>  
> -#define PB_USAGE_CPU_READ  (1 << 0)
> -#define PB_USAGE_CPU_WRITE (1 << 1)
> -#define PB_USAGE_GPU_READ  (1 << 2)
> -#define PB_USAGE_GPU_WRITE (1 << 3)
> -#define PB_USAGE_UNSYNCHRONIZED (1 << 10)
> -#define PB_USAGE_DONTBLOCK (1 << 9)
> +/* For error checking elsewhere */
> +#define PB_USAGE_ALL (PB_USAGE_CPU_READ | \
> +  PB_USAGE_CPU_WRITE | \
> +  PB_USAGE_GPU_READ | \
> +  PB_USAGE_GPU_WRITE | \
> +  PB_USAGE_DONTBLOCK | \
> +  PB_USAGE_UNSYNCHRONIZED)
>  
>  #define PB_USAGE_CPU_READ_WRITE \
> ( PB_USAGE_CPU_READ | PB_USAGE_CPU_WRITE )
> @@ -82,7 +91,7 @@ struct pipe_fence_handle;
>  struct pb_desc
>  {
> unsigned alignment;
> -   unsigned usage;
> +   enum pb_usage_flags usage;
>  };
>  
>  
> @@ -100,7 +109,7 @@ struct pb_buffer
> struct pipe_reference  reference;
> unsigned   alignment;
> pb_sizesize;
> -   unsigned   usage;
> +   enum pb_usage_flagsusage;
>  
> /**
>  * Pointer to the virtual function table.
> @@ -126,13 +135,13 @@ struct pb_vtbl
>  * flags is bitmask of PB_USAGE_CPU_READ/WRITE. 
>  */
> void *(*map)( struct pb_buffer *buf, 
> - unsigned flags, void *flush_ctx );
> + enum pb_usage_flags flags, void *flush_ctx );
> 
> void (*unmap)( struct pb_buffer *buf );
>  
> enum pipe_error (*validate)( struct pb_buffer *buf, 
>  struct pb_validate *vl,
> -unsigned flags );
> +enum pb_usage_flags flags );
>  
> void (*fence)( struct pb_buffer *buf, 
>struct pipe_fence_handle *fence );
> @@ -160,7 +169,7 @@ struct pb_vtbl
>   */
>  static inline void *
>  pb_map(struct pb_buffer *buf, 
> -   unsigned flags, void *flush_ctx)
> +   enum pb_usage_flags flags, void *flush_ctx)
>  {
> assert(buf);
> if (!buf)
> @@ -201,7 +210,8 @@ pb_get_base_buffer( struct pb_buffer *buf,
>  
>  
>  static inline enum pipe_error 
> -pb_validate(struct pb_buffer *buf, struct pb_validate *vl, unsigned
> flags)
> +pb_validate(struct pb_buffer *buf, struct pb_validate *vl,
> +enum pb_usage_flags flags)
>  {
> assert(buf);
> if (!buf)
> diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
> b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
> index 7421741..53b9ce0 100644
> --- a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
> +++ b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
> @@ -139,7 +139,7 @@ struct fenced_buffer
>  * A bitmask of PB_USAGE_CPU/GPU_READ/WRITE describing the
> current
>  * buffer usage.
>  */
> -   unsigned flags;
> +   enum pb_usage_flags flags;
>  
> unsigned mapcount;
>  
> @@ -662,7 +662,7 @@ fenced_buffer_destroy(struct pb_buffer *buf)
>  
>  static void *
>  fenced_buffer_map(struct pb_buffer *buf,
> -  unsigned flags, void *flush_ctx)
> +  enum pb_usage_flags flags, void *flush_ctx)
>  {
> struct fenced_buffer *fenced_buf = fenced_buffer(buf);
> struct fenced_manager *fenced_mgr = fenced_buf->mgr;
> @@ -739,7 +739,7 @@ fenced_buffer_unmap(struct pb_buffer *buf)
>  static enum pipe_error
>  fenced_buffer_validate(struct pb_buffer *buf,
>   

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Samuel Pitoiset]

On 4/23/19 10:45 AM, Bas Nieuwenhuizen wrote:

On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
 wrote:

Signed-off-by: Samuel Pitoiset 
---
  src/amd/vulkan/radv_shader.c |  2 +-
  src/compiler/nir/nir.h   |  3 ++-
  src/compiler/nir/nir_opt_if.c| 17 ++---
  src/freedreno/ir3/ir3_nir.c  |  2 +-
  src/gallium/auxiliary/nir/tgsi_to_nir.c  |  2 +-
  src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
  src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
  src/intel/compiler/brw_nir.c |  2 +-
  src/mesa/state_tracker/st_glsl_to_nir.cpp|  2 +-
  9 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 13f1f9aa9dc..54a4e732230 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader *shader, bool 
optimize_conservatively,
 NIR_PASS(progress, shader, nir_opt_remove_phis);
  NIR_PASS(progress, shader, nir_opt_dce);
  }
-NIR_PASS(progress, shader, nir_opt_if, true);
+NIR_PASS(progress, shader, nir_opt_if, true, false);
  NIR_PASS(progress, shader, nir_opt_dead_cf);
  NIR_PASS(progress, shader, nir_opt_cse);
  NIR_PASS(progress, shader, nir_opt_peephole_select, 8, true, 
true);
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 7d2062d3691..d7506d6ddd1 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool value_number);

  bool nir_opt_idiv_const(nir_shader *shader, unsigned min_bit_size);

-bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue);
+bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
+bool skip_alu_of_phi);

Can we have a flag for this instead (e.g. something like
nir_opt_if_skip_alu_of_phi)? I think have a function with a bunch of
bools is less than ideal as you can't see at the calling site what is
for what arg.

Yes, that seems better to me.

  bool nir_opt_intrinsics(nir_shader *shader);

diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index f674185f1e2..149b3bd1659 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list,
   * not do anything to cause the metadata to become invalid.
   */
  static bool
-opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
+opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list,
+bool skip_alu_of_phi)
  {
 bool progress = false;
 foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
@@ -1395,16 +1396,17 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list 
*cf_list)

case nir_cf_node_if: {
   nir_if *nif = nir_cf_node_as_if(cf_node);
- progress |= opt_if_safe_cf_list(b, >then_list);
- progress |= opt_if_safe_cf_list(b, >else_list);
+ progress |= opt_if_safe_cf_list(b, >then_list, skip_alu_of_phi);
+ progress |= opt_if_safe_cf_list(b, >else_list, skip_alu_of_phi);
   progress |= opt_if_evaluate_condition_use(b, nif);
   break;
}

case nir_cf_node_loop: {
   nir_loop *loop = nir_cf_node_as_loop(cf_node);
- progress |= opt_if_safe_cf_list(b, >body);
- progress |= opt_split_alu_of_phi(b, loop);
+ progress |= opt_if_safe_cf_list(b, >body, skip_alu_of_phi);
+ if (!skip_alu_of_phi)
+progress |= opt_split_alu_of_phi(b, loop);
   break;
}

@@ -1417,7 +1419,8 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list 
*cf_list)
  }

  bool
-nir_opt_if(nir_shader *shader, bool aggressive_last_continue)
+nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
+   bool skip_alu_of_phi)
  {
 bool progress = false;

@@ -1430,7 +1433,7 @@ nir_opt_if(nir_shader *shader, bool 
aggressive_last_continue)

nir_metadata_require(function->impl, nir_metadata_block_index |
 nir_metadata_dominance);
-  progress = opt_if_safe_cf_list(, >impl->body);
+  progress = opt_if_safe_cf_list(, >impl->body, 
skip_alu_of_phi);
nir_metadata_preserve(function->impl, nir_metadata_block_index |
  nir_metadata_dominance);

diff --git a/src/freedreno/ir3/ir3_nir.c b/src/freedreno/ir3/ir3_nir.c
index 76230e3be50..1bec3c030a9 100644
--- a/src/freedreno/ir3/ir3_nir.c
+++ b/src/freedreno/ir3/ir3_nir.c
@@ -147,7 +147,7 @@ ir3_optimize_loop(nir_shader *s)
 OPT(s, nir_copy_prop);
 OPT(s, nir_opt_dce);
 }
-   progress |= OPT(s, nir_opt_if, false);
+   progress |= OPT(s, nir_opt_if, 

[Mesa-dev] [PATCH] radv: do not load vertex attributes that are not provided by the pipeline

[Samuel Pitoiset]

Per the Vulkan spec this is definitely invalid but X4 Foundations
does that and it ends up by hanging the GPU.

Found while enabling validation layers with the game. The issue
will be reported to the developers.

Cc: "19.0" mesa-sta...@lists.freedesktop.org
Signed-off-by: Samuel Pitoiset 
---

This is a backport for staging/19.0 *only*!

 src/amd/vulkan/radv_nir_to_llvm.c | 30 ++
 src/amd/vulkan/radv_pipeline.c|  3 +++
 src/amd/vulkan/radv_private.h |  1 +
 src/amd/vulkan/radv_shader.h  |  3 +++
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index bb8ebca0dab..fba08f29134 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2027,10 +2027,32 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
 
t_list = ac_build_load_to_sgpr(>ac, t_list_ptr, t_offset);
 
-   input = ac_build_buffer_load_format(>ac, t_list,
-   buffer_index,
-   ctx->ac.i32_0,
-   num_channels, false, true);
+   if (ctx->options->key.vs.vertex_attribute_provided & (1u << 
attrib_index)) {
+   input = ac_build_buffer_load_format(>ac, t_list,
+   buffer_index,
+   ctx->ac.i32_0,
+   num_channels, 
false, true);
+   } else {
+   /* Per the Vulkan spec, it's invalid to consume vertex
+* attributes that are not provided by the pipeline but
+* some (invalid) apps appear to do that. Fill the
+* input array with (eg. (0, 0, 0, 1)) to workaround
+* the problem and to avoid possible GPU hangs.
+*/
+   LLVMValueRef chan[4];
+
+   /* The input_usage mask might be 0 if input variables
+* are not removed by the compiler.
+*/
+   num_channels = CLAMP(num_channels, 1, 4);
+
+   for (unsigned i = 0; i < num_channels; i++) {
+   chan[i] = i == 3 ? ctx->ac.i32_1 : 
ctx->ac.i32_0;
+   chan[i] = ac_to_float(>ac, chan[i]);
+   }
+
+   input = ac_build_gather_values(>ac, chan, 
num_channels);
+   }
 
input = ac_build_expand_to_vec4(>ac, input, num_channels);
 
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 61bdfc5cd2d..91f468910f1 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -1922,6 +1922,8 @@ radv_generate_graphics_pipeline_key(struct radv_pipeline 
*pipeline,
}
key.vertex_alpha_adjust |= adjust << (2 * location);
}
+
+   key.vertex_attribute_provided |= 1 << location;
}
 
if (pCreateInfo->pTessellationState)
@@ -1950,6 +1952,7 @@ radv_fill_shader_keys(struct radv_shader_variant_key 
*keys,
 {
keys[MESA_SHADER_VERTEX].vs.instance_rate_inputs = 
key->instance_rate_inputs;
keys[MESA_SHADER_VERTEX].vs.alpha_adjust = key->vertex_alpha_adjust;
+   keys[MESA_SHADER_VERTEX].vs.vertex_attribute_provided = 
key->vertex_attribute_provided;
for (unsigned i = 0; i < MAX_VERTEX_ATTRIBS; ++i)
keys[MESA_SHADER_VERTEX].vs.instance_rate_divisors[i] = 
key->instance_rate_divisors[i];
 
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index aaf057ea919..cd025cd5c4c 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -365,6 +365,7 @@ struct radv_pipeline_cache {
 struct radv_pipeline_key {
uint32_t instance_rate_inputs;
uint32_t instance_rate_divisors[MAX_VERTEX_ATTRIBS];
+   uint32_t vertex_attribute_provided;
uint64_t vertex_alpha_adjust;
unsigned tess_input_vertices;
uint32_t col_format;
diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h
index 31839c3a85e..f6f9dd2bbf1 100644
--- a/src/amd/vulkan/radv_shader.h
+++ b/src/amd/vulkan/radv_shader.h
@@ -66,6 +66,9 @@ struct radv_vs_variant_key {
uint32_t instance_rate_inputs;
uint32_t instance_rate_divisors[MAX_VERTEX_ATTRIBS];
 
+   /* Mask of vertex attributes that are provided by the pipeline. */
+   uint32_t vertex_attribute_provided;
+
/* For 2_10_10_10 formats the alpha is handled as unsigned by pre-vega 
HW.
 * so we may need to fix it up. */
uint64_t alpha_adjust;
-- 
2.21.0

___

Re: [Mesa-dev] [PATCH v2] st: require compatible driver in autotools

[Emil Velikov]

On Fri, 19 Apr 2019 at 19:53, Alyssa Ross  wrote:
>
> The meson build system already has these checks. I've just copied them
> to autotools.
>
> Without this, state trackers could be enabled when building with the
> following set of options, which resulted in a compile error due to VL
> being built without DRM.
>
>  --enable-autotools
>  --with-platforms=x11
>  --with-dri-drivers=
>  --with-gallium-drivers=swrast
>  --disable-glx
>  --disable-dri3
>  --disable-gbm
>
> The compile error was:
>
> vl/vl_winsys_dri.c:36:10: fatal error: xf86drm.h: No such file or 
> directory
>  #include 
>   ^~~
> compilation terminated.
>
I was in favour of the the explicit driver/st tracking yet it was
nuked with commit 28e84c93bb33dfc8ac5219480b274c84dbd58762
To address the above use-case, we don't need to reinstate the tracking
- it causes a lot of churn.

Off the top of my head - none of the VL code should be build when we
have only a swrast driver.
Can you try and kill that bug, or shall I?

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

Re: [Mesa-dev] [PATCH] radv: disable DCC for X4 Foundations to workaround a GPU hang

[Samuel Pitoiset]

Please ignore this fix too, the GPU hang is actually unrelated to DCC.

On 3/6/19 7:39 PM, Samuel Pitoiset wrote:

The game apparently hangs inside a copy image operation, but
only when DCC is enabled. I haven't figured out the root cause
yet, but this workaround fixes the problem and allows people
to play that title, at least.

Cc: 18.3 19.0 
Signed-off-by: Samuel Pitoiset 
---
  src/amd/vulkan/radv_device.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index fc04de21025..bb76885d986 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -510,6 +510,12 @@ radv_handle_per_app_options(struct radv_instance *instance,
} else if (!strcmp(name, "DOOM_VFR")) {
/* Work around a Doom VFR game bug */
instance->debug_flags |= RADV_DEBUG_NO_DYNAMIC_BOUNDS;
+   } else if (!strcmp(name, "X4")) {
+   /* FIXME: X4 Foundations hangs the GPU on GFX9. It seems to
+* work on GFX8, but I think it's safer to disable DCC
+* everywhere for now.
+*/
+   instance->debug_flags |= RADV_DEBUG_NO_DCC;
}
  }
  

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

Re: [Mesa-dev] [PATCH] radv: increase aligment when allocating descriptors for meta operations

[Samuel Pitoiset]

Please ignore this fix, it's incorrect and we shouldn't have to increase 
alignment.


On 3/7/19 11:43 AM, Samuel Pitoiset wrote:

We usually align to 0x100 when allocating descriptors and
resources in the upload BO. I assume some data were corrupted
because the alignment was too small. Be consistent (and safe)
and align internal descriptors to 0x100 as well.

This fixes a GPU hang at startup with X4 Foundations.

Cc: 18.3 19.0 
Signed-off-by: Samuel Pitoiset 
---
  src/amd/vulkan/radv_cmd_buffer.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 5b66930d137..24ed6d47a51 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -2848,7 +2848,7 @@ void radv_meta_push_descriptor_set(
push_set->size = layout->set[set].layout->size;
push_set->layout = layout->set[set].layout;
  
-	if (!radv_cmd_buffer_upload_alloc(cmd_buffer, push_set->size, 32,

+   if (!radv_cmd_buffer_upload_alloc(cmd_buffer, push_set->size, 256,
  _offset,
  (void**) _set->mapped_ptr))
return;

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

Re: [Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing

[Andres Gomez]

On Tue, 2019-04-23 at 10:00 +0300, Tapani Pälli wrote:
> Remember to add:
> 
> --- 8< ---
> Fixes the following test:
> KHR-GL44.enhanced_layouts.xfb_output_overlapping
> --- 8< ---

Good point. I developed this patch when this test was yet not
"corrected" and failing for us.

> 
> 
> See below for one addition:
> 
> On 4/22/19 3:00 AM, Andres Gomez wrote:
> >  From page 76 (page 80 of the PDF) of the GLSL 4.60 v.5 spec:
> > 
> >" No aliasing in output buffers is allowed: It is a compile-time or
> >  link-time error to specify variables with overlapping transform
> >  feedback offsets."
> > 
> > Currently, this is expected to fail, but it succeeds:
> > 
> >"
> > 
> >  ...
> > 
> >  layout (xfb_offset = 0) out vec2 a;
> >  layout (xfb_offset = 0) out vec4 b;
> > 
> >  ...
> > 
> >"
> > 
> > v2:
> >- Use a data structure to track the used components instead of a
> >  nested loop (Ilia).
> > 
> > v3:
> >- Take the BITSET_WORD array out from the
> >  gl_transform_feedback_buffer struct and make it local to the
> >  validation process (Timothy).
> >- Do not use a nested scope for the validation (Timothy).
> > 
> > Cc: Timothy Arceri 
> > Cc: Ilia Mirkin 
> > Signed-off-by: Andres Gomez 
> > ---
> >   src/compiler/glsl/link_varyings.cpp | 109 
> >   src/compiler/glsl/link_varyings.h   |   6 +-
> >   2 files changed, 84 insertions(+), 31 deletions(-)
> > 
> > diff --git a/src/compiler/glsl/link_varyings.cpp 
> > b/src/compiler/glsl/link_varyings.cpp
> > index 22ec411837d..89874530980 100644
> > --- a/src/compiler/glsl/link_varyings.cpp
> > +++ b/src/compiler/glsl/link_varyings.cpp
> > @@ -1169,8 +1169,10 @@ bool
> >   tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program 
> > *prog,
> > struct gl_transform_feedback_info *info,
> > unsigned buffer, unsigned buffer_index,
> > -  const unsigned max_outputs, bool *explicit_stride,
> > -  bool has_xfb_qualifiers) const
> > +  const unsigned max_outputs,
> > +  BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS],
> > +  bool *explicit_stride, bool has_xfb_qualifiers,
> > +  const void* mem_ctx) const
> >   {
> >  unsigned xfb_offset = 0;
> >  unsigned size = this->size;
> > @@ -1197,6 +1199,72 @@ tfeedback_decl::store(struct gl_context *ctx, struct 
> > gl_shader_program *prog,
> > unsigned location = this->location;
> > unsigned location_frac = this->location_frac;
> > unsigned num_components = this->num_components();
> > +
> > +  /* From GL_EXT_transform_feedback:
> > +   *
> > +   *   " A program will fail to link if:
> > +   *
> > +   *   * the total number of components to capture is greater than 
> > the
> > +   * constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
> > +   * and the buffer mode is INTERLEAVED_ATTRIBS_EXT."
> > +   *
> > +   * From GL_ARB_enhanced_layouts:
> > +   *
> > +   *   " The resulting stride (implicit or explicit) must be less than 
> > or
> > +   * equal to the implementation-dependent constant
> > +   * gl_MaxTransformFeedbackInterleavedComponents."
> > +   */
> > +  if ((prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS ||
> > +   has_xfb_qualifiers) &&
> > +  xfb_offset + num_components >
> > +  ctx->Const.MaxTransformFeedbackInterleavedComponents) {
> > + linker_error(prog,
> > +  "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
> > +  "limit has been exceeded.");
> > + return false;
> > +  }
> > +
> > +  /* From the OpenGL 4.60.5 spec, section 4.4.2. Output Layout 
> > Qualifiers,
> > +   * Page 76, (Transform Feedback Layout Qualifiers):
> > +   *
> > +   *   " No aliasing in output buffers is allowed: It is a 
> > compile-time or
> > +   * link-time error to specify variables with overlapping 
> > transform
> > +   * feedback offsets."
> > +   */
> > +  const unsigned max_components =
> > + ctx->Const.MaxTransformFeedbackInterleavedComponents;
> > +  const unsigned first_component = xfb_offset;
> > +  const unsigned last_component = xfb_offset + num_components - 1;
> > +  const unsigned start_word = BITSET_BITWORD(first_component);
> > +  const unsigned end_word = BITSET_BITWORD(last_component);
> > +  BITSET_WORD *used;
> > +  assert(last_component < max_components);
> > +
> > +  if (!used_components[buffer]) {
> > + used_components[buffer] =
> > +rzalloc_array(mem_ctx, BITSET_WORD, 
> > BITSET_WORDS(max_components));
> > +  }
> > +  used = used_components[buffer];
> > +
> > +  for (unsigned word = start_word; word <= end_word; 

[Mesa-dev] [Bug 110462] Epic Games Launcher renders nothing with "-opengl" option

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110462

--- Comment #6 from Danylo  ---
> As far as I recall apitrace records the profile it was given not the profile 
> that the application requests.

I have copied the function call itself which I think apitrace doesn't change.
It can show a different info in a current state of the call but that's another
story.

However I rechecked by adding logs and here what is passed to
dri2_convert_glx_attribs which is called by glXCreateContextAttribsARB:

major: 1, minor: 0, compat: 0
major: 1, minor: 0, compat: 0
major: 3, minor: 2, compat: 1
major: 1, minor: 0, compat: 0
major: 4, minor: 4, compat: 0
major: 1, minor: 0, compat: 0
major: 3, minor: 2, compat: 1

So 4.4 created without compat.

-- 
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

[Mesa-dev] [Bug 110472] Graphical Fault (Desktop Freeze) on Specific OpenGL Application

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110472

Michel Dänzer  changed:

   What|Removed |Added

 QA Contact|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org
  Component|Other   |Drivers/Gallium/radeonsi
   Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org

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

Re: [Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Bas Nieuwenhuizen]

On Tue, Apr 23, 2019 at 9:35 AM Samuel Pitoiset
 wrote:
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_shader.c |  2 +-
>  src/compiler/nir/nir.h   |  3 ++-
>  src/compiler/nir/nir_opt_if.c| 17 ++---
>  src/freedreno/ir3/ir3_nir.c  |  2 +-
>  src/gallium/auxiliary/nir/tgsi_to_nir.c  |  2 +-
>  src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
>  src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
>  src/intel/compiler/brw_nir.c |  2 +-
>  src/mesa/state_tracker/st_glsl_to_nir.cpp|  2 +-
>  9 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
> index 13f1f9aa9dc..54a4e732230 100644
> --- a/src/amd/vulkan/radv_shader.c
> +++ b/src/amd/vulkan/radv_shader.c
> @@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader *shader, bool 
> optimize_conservatively,
> NIR_PASS(progress, shader, nir_opt_remove_phis);
>  NIR_PASS(progress, shader, nir_opt_dce);
>  }
> -NIR_PASS(progress, shader, nir_opt_if, true);
> +NIR_PASS(progress, shader, nir_opt_if, true, false);
>  NIR_PASS(progress, shader, nir_opt_dead_cf);
>  NIR_PASS(progress, shader, nir_opt_cse);
>  NIR_PASS(progress, shader, nir_opt_peephole_select, 8, true, 
> true);
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 7d2062d3691..d7506d6ddd1 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool value_number);
>
>  bool nir_opt_idiv_const(nir_shader *shader, unsigned min_bit_size);
>
> -bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue);
> +bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
> +bool skip_alu_of_phi);

Can we have a flag for this instead (e.g. something like
nir_opt_if_skip_alu_of_phi)? I think have a function with a bunch of
bools is less than ideal as you can't see at the calling site what is
for what arg.
>
>  bool nir_opt_intrinsics(nir_shader *shader);
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index f674185f1e2..149b3bd1659 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct exec_list 
> *cf_list,
>   * not do anything to cause the metadata to become invalid.
>   */
>  static bool
> -opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
> +opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list,
> +bool skip_alu_of_phi)
>  {
> bool progress = false;
> foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
> @@ -1395,16 +1396,17 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list 
> *cf_list)
>
>case nir_cf_node_if: {
>   nir_if *nif = nir_cf_node_as_if(cf_node);
> - progress |= opt_if_safe_cf_list(b, >then_list);
> - progress |= opt_if_safe_cf_list(b, >else_list);
> + progress |= opt_if_safe_cf_list(b, >then_list, 
> skip_alu_of_phi);
> + progress |= opt_if_safe_cf_list(b, >else_list, 
> skip_alu_of_phi);
>   progress |= opt_if_evaluate_condition_use(b, nif);
>   break;
>}
>
>case nir_cf_node_loop: {
>   nir_loop *loop = nir_cf_node_as_loop(cf_node);
> - progress |= opt_if_safe_cf_list(b, >body);
> - progress |= opt_split_alu_of_phi(b, loop);
> + progress |= opt_if_safe_cf_list(b, >body, skip_alu_of_phi);
> + if (!skip_alu_of_phi)
> +progress |= opt_split_alu_of_phi(b, loop);
>   break;
>}
>
> @@ -1417,7 +1419,8 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list 
> *cf_list)
>  }
>
>  bool
> -nir_opt_if(nir_shader *shader, bool aggressive_last_continue)
> +nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
> +   bool skip_alu_of_phi)
>  {
> bool progress = false;
>
> @@ -1430,7 +1433,7 @@ nir_opt_if(nir_shader *shader, bool 
> aggressive_last_continue)
>
>nir_metadata_require(function->impl, nir_metadata_block_index |
> nir_metadata_dominance);
> -  progress = opt_if_safe_cf_list(, >impl->body);
> +  progress = opt_if_safe_cf_list(, >impl->body, 
> skip_alu_of_phi);
>nir_metadata_preserve(function->impl, nir_metadata_block_index |
>  nir_metadata_dominance);
>
> diff --git a/src/freedreno/ir3/ir3_nir.c b/src/freedreno/ir3/ir3_nir.c
> index 76230e3be50..1bec3c030a9 100644
> --- a/src/freedreno/ir3/ir3_nir.c
> +++ b/src/freedreno/ir3/ir3_nir.c
> @@ -147,7 +147,7 @@ ir3_optimize_loop(nir_shader *s)
> OPT(s, nir_copy_prop);
> OPT(s, nir_opt_dce);
> }
> - 

[Mesa-dev] [PATCH 2/2] radv: workaround a GPU hang with No Man Sky by skipping split_alu_of_phi

[Samuel Pitoiset]

This pass seems totally correct, as well as the generated NIR and
LLVM IR for the revelant shader that hangs the GPU. This likely
triggers a new bug in LLVM which has to be fixed.

As LLVM 7&8 have already been released we need a workaround.

According to my collection of vkpipeline-db shaders, this is the only
game that is affected by this optimization.

Fixes: 0881e90c099 ("nir: Split ALU instructions in loops that read phis")
Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_shader.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 54a4e732230..fd80e4aac93 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -158,7 +158,15 @@ radv_optimize_nir(struct nir_shader *shader, bool 
optimize_conservatively,
NIR_PASS(progress, shader, nir_opt_remove_phis);
 NIR_PASS(progress, shader, nir_opt_dce);
 }
-NIR_PASS(progress, shader, nir_opt_if, true, false);
+
+   /* Skip the split_alu_of_phi NIR pass because this triggers a
+* new bug in LLVM with No Man Sky.
+* (see https://bugs.freedesktop.org/show_bug.cgi?id=110471).
+* As far I can tell, it's the only game that is affected by
+* this optimization.
+* TODO: Fix LLVM!
+*/
+NIR_PASS(progress, shader, nir_opt_if, true, true);
 NIR_PASS(progress, shader, nir_opt_dead_cf);
 NIR_PASS(progress, shader, nir_opt_cse);
 NIR_PASS(progress, shader, nir_opt_peephole_select, 8, true, 
true);
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH 3/8] etnaviv: remember data offset into BO

[Christian Gmeiner]

Am Fr., 12. Apr. 2019 um 19:38 Uhr schrieb Lucas Stach :
>
> Imported resources might not start at offset 0 into the buffer object.
> Make sure to remember the offset that is provided with the handle on
> import.
>
> Signed-off-by: Lucas Stach 

Reviewed-by: Christian Gmeiner 

> ---
>  src/gallium/drivers/etnaviv/etnaviv_resource.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c 
> b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> index 77d027ac806b..f405b880a6c0 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> @@ -535,6 +535,7 @@ etna_resource_from_handle(struct pipe_screen *pscreen,
>
> level->width = tmpl->width0;
> level->height = tmpl->height0;
> +   level->offset = handle->offset;
>
> /* Determine padding of the imported resource. */
> unsigned paddingX = 0, paddingY = 0;
> --
> 2.20.1
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv



-- 
greets
--
Christian Gmeiner, MSc

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

Re: [Mesa-dev] [PATCH 7/8] etnaviv: improve PIPE_BIND_LINEAR handling

[Christian Gmeiner]

Am Fr., 12. Apr. 2019 um 19:38 Uhr schrieb Lucas Stach :
>
> We weren't handling this flag at all, which broke some assumptions
> made by the users of the resource_create interface. As we can't render
> to a linear surface and the usefulness of yet another layout transition
> to handle this case seems limited, we only respect the flag when the
> resource isn't used for rendering.
>
> Signed-off-by: Lucas Stach 

Reviewed-by: Christian Gmeiner 

> ---
>  src/gallium/drivers/etnaviv/etnaviv_resource.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c 
> b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> index f405b880a6c0..650c8e7eb7f5 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> @@ -369,6 +369,14 @@ etna_resource_create(struct pipe_screen *pscreen,
> if (templat->target == PIPE_TEXTURE_3D)
>layout = ETNA_LAYOUT_LINEAR;
>
> +   /* The render pipe can't handle linear and there is no code to do yet 
> another
> +* layout transformation for this case, so we only respect the linear flag
> +* if the resource isn't meant to be rendered.
> +*/
> +   if ((templat->bind & PIPE_BIND_LINEAR) &&
> +   !(templat->bind & PIPE_BIND_RENDER_TARGET))
> +  layout = ETNA_LAYOUT_LINEAR;
> +
> /* modifier is only used for scanout surfaces, so safe to use LINEAR here 
> */
> return etna_resource_alloc(pscreen, layout, mode, DRM_FORMAT_MOD_LINEAR, 
> templat);
>  }
> --
> 2.20.1
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv



-- 
greets
--
Christian Gmeiner, MSc

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

[Mesa-dev] [PATCH 1/2] nir: add an option for skipping split_alu_of_phi

[Samuel Pitoiset]

Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_shader.c |  2 +-
 src/compiler/nir/nir.h   |  3 ++-
 src/compiler/nir/nir_opt_if.c| 17 ++---
 src/freedreno/ir3/ir3_nir.c  |  2 +-
 src/gallium/auxiliary/nir/tgsi_to_nir.c  |  2 +-
 src/gallium/drivers/freedreno/a2xx/ir2_nir.c |  2 +-
 src/gallium/drivers/radeonsi/si_shader_nir.c |  2 +-
 src/intel/compiler/brw_nir.c |  2 +-
 src/mesa/state_tracker/st_glsl_to_nir.cpp|  2 +-
 9 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 13f1f9aa9dc..54a4e732230 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -158,7 +158,7 @@ radv_optimize_nir(struct nir_shader *shader, bool 
optimize_conservatively,
NIR_PASS(progress, shader, nir_opt_remove_phis);
 NIR_PASS(progress, shader, nir_opt_dce);
 }
-NIR_PASS(progress, shader, nir_opt_if, true);
+NIR_PASS(progress, shader, nir_opt_if, true, false);
 NIR_PASS(progress, shader, nir_opt_dead_cf);
 NIR_PASS(progress, shader, nir_opt_cse);
 NIR_PASS(progress, shader, nir_opt_peephole_select, 8, true, 
true);
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 7d2062d3691..d7506d6ddd1 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -3474,7 +3474,8 @@ bool nir_opt_gcm(nir_shader *shader, bool value_number);
 
 bool nir_opt_idiv_const(nir_shader *shader, unsigned min_bit_size);
 
-bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue);
+bool nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
+bool skip_alu_of_phi);
 
 bool nir_opt_intrinsics(nir_shader *shader);
 
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index f674185f1e2..149b3bd1659 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -1385,7 +1385,8 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list,
  * not do anything to cause the metadata to become invalid.
  */
 static bool
-opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
+opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list,
+bool skip_alu_of_phi)
 {
bool progress = false;
foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
@@ -1395,16 +1396,17 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list 
*cf_list)
 
   case nir_cf_node_if: {
  nir_if *nif = nir_cf_node_as_if(cf_node);
- progress |= opt_if_safe_cf_list(b, >then_list);
- progress |= opt_if_safe_cf_list(b, >else_list);
+ progress |= opt_if_safe_cf_list(b, >then_list, skip_alu_of_phi);
+ progress |= opt_if_safe_cf_list(b, >else_list, skip_alu_of_phi);
  progress |= opt_if_evaluate_condition_use(b, nif);
  break;
   }
 
   case nir_cf_node_loop: {
  nir_loop *loop = nir_cf_node_as_loop(cf_node);
- progress |= opt_if_safe_cf_list(b, >body);
- progress |= opt_split_alu_of_phi(b, loop);
+ progress |= opt_if_safe_cf_list(b, >body, skip_alu_of_phi);
+ if (!skip_alu_of_phi)
+progress |= opt_split_alu_of_phi(b, loop);
  break;
   }
 
@@ -1417,7 +1419,8 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list 
*cf_list)
 }
 
 bool
-nir_opt_if(nir_shader *shader, bool aggressive_last_continue)
+nir_opt_if(nir_shader *shader, bool aggressive_last_continue,
+   bool skip_alu_of_phi)
 {
bool progress = false;
 
@@ -1430,7 +1433,7 @@ nir_opt_if(nir_shader *shader, bool 
aggressive_last_continue)
 
   nir_metadata_require(function->impl, nir_metadata_block_index |
nir_metadata_dominance);
-  progress = opt_if_safe_cf_list(, >impl->body);
+  progress = opt_if_safe_cf_list(, >impl->body, 
skip_alu_of_phi);
   nir_metadata_preserve(function->impl, nir_metadata_block_index |
 nir_metadata_dominance);
 
diff --git a/src/freedreno/ir3/ir3_nir.c b/src/freedreno/ir3/ir3_nir.c
index 76230e3be50..1bec3c030a9 100644
--- a/src/freedreno/ir3/ir3_nir.c
+++ b/src/freedreno/ir3/ir3_nir.c
@@ -147,7 +147,7 @@ ir3_optimize_loop(nir_shader *s)
OPT(s, nir_copy_prop);
OPT(s, nir_opt_dce);
}
-   progress |= OPT(s, nir_opt_if, false);
+   progress |= OPT(s, nir_opt_if, false, false);
progress |= OPT(s, nir_opt_remove_phis);
progress |= OPT(s, nir_opt_undef);
 
diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c 
b/src/gallium/auxiliary/nir/tgsi_to_nir.c
index c55e8b84a41..6b40bff1f73 100644
--- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
+++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
@@ -2066,7 +2066,7 @@ 

Re: [Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing

[Tapani Pälli]

Remember to add:

--- 8< ---
Fixes the following test:
KHR-GL44.enhanced_layouts.xfb_output_overlapping
--- 8< ---


See below for one addition:

On 4/22/19 3:00 AM, Andres Gomez wrote:

 From page 76 (page 80 of the PDF) of the GLSL 4.60 v.5 spec:

   " No aliasing in output buffers is allowed: It is a compile-time or
 link-time error to specify variables with overlapping transform
 feedback offsets."

Currently, this is expected to fail, but it succeeds:

   "

 ...

 layout (xfb_offset = 0) out vec2 a;
 layout (xfb_offset = 0) out vec4 b;

 ...

   "

v2:
   - Use a data structure to track the used components instead of a
 nested loop (Ilia).

v3:
   - Take the BITSET_WORD array out from the
 gl_transform_feedback_buffer struct and make it local to the
 validation process (Timothy).
   - Do not use a nested scope for the validation (Timothy).

Cc: Timothy Arceri 
Cc: Ilia Mirkin 
Signed-off-by: Andres Gomez 
---
  src/compiler/glsl/link_varyings.cpp | 109 
  src/compiler/glsl/link_varyings.h   |   6 +-
  2 files changed, 84 insertions(+), 31 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 22ec411837d..89874530980 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1169,8 +1169,10 @@ bool
  tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program *prog,
struct gl_transform_feedback_info *info,
unsigned buffer, unsigned buffer_index,
-  const unsigned max_outputs, bool *explicit_stride,
-  bool has_xfb_qualifiers) const
+  const unsigned max_outputs,
+  BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS],
+  bool *explicit_stride, bool has_xfb_qualifiers,
+  const void* mem_ctx) const
  {
 unsigned xfb_offset = 0;
 unsigned size = this->size;
@@ -1197,6 +1199,72 @@ tfeedback_decl::store(struct gl_context *ctx, struct 
gl_shader_program *prog,
unsigned location = this->location;
unsigned location_frac = this->location_frac;
unsigned num_components = this->num_components();
+
+  /* From GL_EXT_transform_feedback:
+   *
+   *   " A program will fail to link if:
+   *
+   *   * the total number of components to capture is greater than the
+   * constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
+   * and the buffer mode is INTERLEAVED_ATTRIBS_EXT."
+   *
+   * From GL_ARB_enhanced_layouts:
+   *
+   *   " The resulting stride (implicit or explicit) must be less than or
+   * equal to the implementation-dependent constant
+   * gl_MaxTransformFeedbackInterleavedComponents."
+   */
+  if ((prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS ||
+   has_xfb_qualifiers) &&
+  xfb_offset + num_components >
+  ctx->Const.MaxTransformFeedbackInterleavedComponents) {
+ linker_error(prog,
+  "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
+  "limit has been exceeded.");
+ return false;
+  }
+
+  /* From the OpenGL 4.60.5 spec, section 4.4.2. Output Layout Qualifiers,
+   * Page 76, (Transform Feedback Layout Qualifiers):
+   *
+   *   " No aliasing in output buffers is allowed: It is a compile-time or
+   * link-time error to specify variables with overlapping transform
+   * feedback offsets."
+   */
+  const unsigned max_components =
+ ctx->Const.MaxTransformFeedbackInterleavedComponents;
+  const unsigned first_component = xfb_offset;
+  const unsigned last_component = xfb_offset + num_components - 1;
+  const unsigned start_word = BITSET_BITWORD(first_component);
+  const unsigned end_word = BITSET_BITWORD(last_component);
+  BITSET_WORD *used;
+  assert(last_component < max_components);
+
+  if (!used_components[buffer]) {
+ used_components[buffer] =
+rzalloc_array(mem_ctx, BITSET_WORD, BITSET_WORDS(max_components));
+  }
+  used = used_components[buffer];
+
+  for (unsigned word = start_word; word <= end_word; word++) {
+ unsigned start_range = 0;
+ unsigned end_range = BITSET_WORDBITS - 1;
+
+ if (word == start_word)
+start_range = first_component % BITSET_WORDBITS;
+
+ if (word == end_word)
+end_range = last_component % BITSET_WORDBITS;
+
+ if (used[word] & BITSET_RANGE(start_range, end_range)) {
+linker_error(prog,
+ "variable '%s', xfb_offset (%d) is causing aliasing.",
+ this->orig_name, xfb_offset * 4);
+return false;
+ }
+ used[word] |= BITSET_RANGE(start_range, end_range);
+  }
+
  

[Mesa-dev] [Bug 110476] Overwatch - some objects are rendered incorrectly

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110476

Samuel Pitoiset  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #4 from Samuel Pitoiset  ---
Fixed with LLVM r358890.

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

Re: [Mesa-dev] [PATCH 1/3] ac/nir: only use the new raw/struct image atomic intrinsics with LLVM 9+

[Samuel Pitoiset]

On 4/22/19 7:19 PM, Marek Olšák wrote:

Is "XXX" in the comment really necessary?

No, but I already pushed the series.


Marek

On Thu, Apr 18, 2019 at 3:20 AM Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:


They are buggy with LLVM 8 because they weren't marked as source
of divergence, see r358579.

Fixes: dd0172e865f ("radv: Use structured intrinsics instead of
indexing workaround for GFX9.")"
Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
---
 src/amd/common/ac_nir_to_llvm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c
b/src/amd/common/ac_nir_to_llvm.c
index 3890aebc982..fcd75903088 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2634,7 +2634,10 @@ static LLVMValueRef
visit_image_atomic(struct ac_nir_context *ctx,
                params[param_count++] =
LLVMBuildExtractElement(ctx->ac.builder, get_src(ctx, instr->src[1]),
    ctx->ac.i32_0, ""); /* vindex */
                params[param_count++] = ctx->ac.i32_0; /* voffset */
-               if (HAVE_LLVM >= 0x800) {
+               if (HAVE_LLVM >= 0x900) {
+                       /* XXX: The new raw/struct atomic
intrinsics are buggy
+                        * with LLVM 8, see r358579.
+                        */
                        params[param_count++] = ctx->ac.i32_0; /*
soffset */
                        params[param_count++] = ctx->ac.i32_0;  /*
slc */

-- 
2.21.0


___
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] [Bug 110440] [REGRESSION] [BISECTED] [OpenGL CTS] dEQP-GLES2.functional.uniform_api.random.3

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=110440

Tapani Pälli  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #5 from Tapani Pälli  ---
Verified that this got fixed with 47303b466c1 and 6981069fc80 did not regress
it, passes fine on master.

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

[Mesa-dev] [PATCH] st/mesa/radeonsi: fix race between destruction of types and shader compilation

[Timothy Arceri]

Commit 624789e3708c moved the destruction of types out of atexit() and
made use of a ref count instead. This is useful for avoiding a crash
where drivers such as radeonsi are still compiling in a thread when the app
exits and has not called MakeCurrent to change from the current context.

While the above scenario is technically an app bug we shouldn't crash.
However that change caused another race condition between the shader
compilation tread in radeonsi and context teardown functions.

This patch makes two changes to fix this new problem:

First we explicitly call _mesa_destroy_shader_compiler_types() when destroying
the st context rather than calling it indirectly via _mesa_free_context_data().
We do this as we must call it after st_destroy_context_priv() so that we don't
destory the glsl types before the compilation threads finish.

Next wait for the shader threads to finish in si_destroy_context() this
also means we need to call context destroy before destroying the queues
in si_destroy_screen().

Fixes: 624789e3708c ("compiler/glsl: handle case where we have multiple users 
for types")
---
 src/compiler/glsl/glsl_parser_extras.h  | 1 +
 src/gallium/drivers/radeonsi/si_pipe.c  | 8 ++--
 src/mesa/drivers/dri/i915/intel_context.c   | 2 +-
 src/mesa/drivers/dri/i965/brw_context.c | 2 +-
 src/mesa/drivers/dri/nouveau/nouveau_context.c  | 2 +-
 src/mesa/drivers/dri/radeon/radeon_common_context.c | 2 +-
 src/mesa/drivers/osmesa/osmesa.c| 8 
 src/mesa/drivers/x11/xm_api.c   | 4 ++--
 src/mesa/main/context.c | 7 ---
 src/mesa/main/context.h | 2 +-
 src/mesa/state_tracker/st_context.c | 8 +++-
 11 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index f92d2160aac..edc6fc06c77 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -997,6 +997,7 @@ extern "C" {
 #endif
 
 struct glcpp_parser;
+struct _mesa_glsl_parse_state;
 
 typedef void (*glcpp_extension_iterator)(
   struct _mesa_glsl_parse_state *state,
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index fa96ce34224..e9e1bd0aa38 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -150,6 +150,9 @@ static void si_destroy_context(struct pipe_context *context)
struct si_context *sctx = (struct si_context *)context;
int i;
 
+   util_queue_finish(>screen->shader_compiler_queue);
+   util_queue_finish(>screen->shader_compiler_queue_low_priority);
+
/* Unreference the framebuffer normally to disable related logic
 * properly.
 */
@@ -702,6 +705,9 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
if (!sscreen->ws->unref(sscreen->ws))
return;
 
+   mtx_destroy(>aux_context_lock);
+   sscreen->aux_context->destroy(sscreen->aux_context);
+
util_queue_destroy(>shader_compiler_queue);
util_queue_destroy(>shader_compiler_queue_low_priority);
 
@@ -728,8 +734,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
si_gpu_load_kill_thread(sscreen);
 
mtx_destroy(>gpu_load_mutex);
-   mtx_destroy(>aux_context_lock);
-   sscreen->aux_context->destroy(sscreen->aux_context);
 
slab_destroy_parent(>pool_transfers);
 
diff --git a/src/mesa/drivers/dri/i915/intel_context.c 
b/src/mesa/drivers/dri/i915/intel_context.c
index c23e5ffb26e..aa3175816cf 100644
--- a/src/mesa/drivers/dri/i915/intel_context.c
+++ b/src/mesa/drivers/dri/i915/intel_context.c
@@ -599,7 +599,7 @@ intelDestroyContext(__DRIcontext * driContextPriv)
   driDestroyOptionCache(>optionCache);
 
   /* free the Mesa context */
-  _mesa_free_context_data(>ctx);
+  _mesa_free_context_data(>ctx, true);
 
   _math_matrix_dtr(>ViewportMatrix);
 
diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index ab637ddf721..df12f373f22 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -1226,7 +1226,7 @@ intelDestroyContext(__DRIcontext * driContextPriv)
driDestroyOptionCache(>optionCache);
 
/* free the Mesa context */
-   _mesa_free_context_data(>ctx);
+   _mesa_free_context_data(>ctx, true);
 
ralloc_free(brw);
driContextPriv->driverPrivate = NULL;
diff --git a/src/mesa/drivers/dri/nouveau/nouveau_context.c 
b/src/mesa/drivers/dri/nouveau/nouveau_context.c
index 93f6ce473a1..8fec35237c0 100644
--- a/src/mesa/drivers/dri/nouveau/nouveau_context.c
+++ b/src/mesa/drivers/dri/nouveau/nouveau_context.c
@@ -217,7 +217,7 @@ nouveau_context_deinit(struct gl_context *ctx)
nouveau_object_del(>hw.chan);