subject:"\[Mesa\-dev\] \[PATCH 8\/8\] st\/glsl_to_nir\: disable io lowering and forced indirect array splitting in fs"

Re: [Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

2018-01-29 Thread Timothy Arceri


On 30/01/18 09:30, Connor Abbott wrote:

When I was talking about handling I/O for radeonsi NIR with Nicolai, I
think the conclusion was that the best way forward is to make the
driver call nir_lower_io and friends, at least for inputs and outputs.
This is a perfect example of the kind of hacks that we have to do --
precisely how to handle inputs is a driver-specific thing, and piling
up special cases like this in the state tracker is just unsustainable.

As an aside, we probably should be using the load_barycentric_*
intrinsics, which would make the code for interpolateAt* much cleaner
for radeonsi, since we don't have to deal with variable derefs. The
goal should be to eliminatem manual deref walking and offset
calculation from radeonsi nir entirely, it shouldn't be necessary at
all.


That is probably a nice goal longer term but currently the shared nir to 
llvm code expects nir_lower_io not to be used. For now I'm planning on 
adding a CAP here rather than using glsl version, as suggested by others.


I'd first like to get nir for radeonsi passing piglit before tackling 
any large refactors to the shared nir to llvm code, besides the forced 
lowering and array splitting is the real hack here and this patch is 
avoiding that.




Connor


On Sun, Jan 14, 2018 at 10:46 PM, Timothy Arceri  wrote:

We need this to be able to support the interpolateAt builtins in a
sane way. It also leads to the generation of more optimal code.

The lowering and splitting is made conditional on glsl 400 because
vc4 and freedreno both expect these passes to be enabled and niether
support glsl 400 so don't need to deal with the interpolateAt builtins.

We leave the other stages for now as to avoid regressions.
---
  src/mesa/state_tracker/st_glsl_to_nir.cpp | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 6e3a1548f4..bc55c5b7db 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -461,7 +461,9 @@ st_nir_get_mesa_program(struct gl_context *ctx,
  struct gl_linked_shader *shader)
  {
 struct st_context *st = st_context(ctx);
+   struct pipe_screen *screen = st->pipe->screen;
 struct gl_program *prog;
+   unsigned glsl_version = screen->get_param(screen, 
PIPE_CAP_GLSL_FEATURE_LEVEL);

 validate_ir_tree(shader->ir);

@@ -491,11 +493,14 @@ st_nir_get_mesa_program(struct gl_context *ctx,
 prog->nir = nir;

 if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
-   nir->info.stage != MESA_SHADER_TESS_EVAL) {
+   nir->info.stage != MESA_SHADER_TESS_EVAL &&
+   (nir->info.stage != MESA_SHADER_FRAGMENT ||
+(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {
NIR_PASS_V(nir, nir_lower_io_to_temporaries,
   nir_shader_get_entrypoint(nir),
   true, true);
 }
+
 NIR_PASS_V(nir, nir_lower_global_vars_to_local);
 NIR_PASS_V(nir, nir_split_var_copies);
 NIR_PASS_V(nir, nir_lower_var_copies);
@@ -665,12 +670,16 @@ st_finalize_nir(struct st_context *st, struct gl_program 
*prog,
  struct gl_shader_program *shader_program, nir_shader *nir)
  {
 struct pipe_screen *screen = st->pipe->screen;
+   unsigned glsl_version = screen->get_param(screen, 
PIPE_CAP_GLSL_FEATURE_LEVEL);

 NIR_PASS_V(nir, nir_split_var_copies);
 NIR_PASS_V(nir, nir_lower_var_copies);
 if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
-   nir->info.stage != MESA_SHADER_TESS_EVAL)
+   nir->info.stage != MESA_SHADER_TESS_EVAL &&
+   (nir->info.stage != MESA_SHADER_FRAGMENT ||
+(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {
NIR_PASS_V(nir, nir_lower_io_arrays_to_elements_no_indirects);
+   }

 if (nir->info.stage == MESA_SHADER_VERTEX) {
/* Needs special handling so drvloc matches the vbo state: */
--
2.14.3

___
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 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

2018-01-29 Thread Connor Abbott

When I was talking about handling I/O for radeonsi NIR with Nicolai, I
think the conclusion was that the best way forward is to make the
driver call nir_lower_io and friends, at least for inputs and outputs.
This is a perfect example of the kind of hacks that we have to do --
precisely how to handle inputs is a driver-specific thing, and piling
up special cases like this in the state tracker is just unsustainable.

As an aside, we probably should be using the load_barycentric_*
intrinsics, which would make the code for interpolateAt* much cleaner
for radeonsi, since we don't have to deal with variable derefs. The
goal should be to eliminatem manual deref walking and offset
calculation from radeonsi nir entirely, it shouldn't be necessary at
all.

Connor


On Sun, Jan 14, 2018 at 10:46 PM, Timothy Arceri  wrote:
> We need this to be able to support the interpolateAt builtins in a
> sane way. It also leads to the generation of more optimal code.
>
> The lowering and splitting is made conditional on glsl 400 because
> vc4 and freedreno both expect these passes to be enabled and niether
> support glsl 400 so don't need to deal with the interpolateAt builtins.
>
> We leave the other stages for now as to avoid regressions.
> ---
>  src/mesa/state_tracker/st_glsl_to_nir.cpp | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
> b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index 6e3a1548f4..bc55c5b7db 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -461,7 +461,9 @@ st_nir_get_mesa_program(struct gl_context *ctx,
>  struct gl_linked_shader *shader)
>  {
> struct st_context *st = st_context(ctx);
> +   struct pipe_screen *screen = st->pipe->screen;
> struct gl_program *prog;
> +   unsigned glsl_version = screen->get_param(screen, 
> PIPE_CAP_GLSL_FEATURE_LEVEL);
>
> validate_ir_tree(shader->ir);
>
> @@ -491,11 +493,14 @@ st_nir_get_mesa_program(struct gl_context *ctx,
> prog->nir = nir;
>
> if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
> -   nir->info.stage != MESA_SHADER_TESS_EVAL) {
> +   nir->info.stage != MESA_SHADER_TESS_EVAL &&
> +   (nir->info.stage != MESA_SHADER_FRAGMENT ||
> +(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {
>NIR_PASS_V(nir, nir_lower_io_to_temporaries,
>   nir_shader_get_entrypoint(nir),
>   true, true);
> }
> +
> NIR_PASS_V(nir, nir_lower_global_vars_to_local);
> NIR_PASS_V(nir, nir_split_var_copies);
> NIR_PASS_V(nir, nir_lower_var_copies);
> @@ -665,12 +670,16 @@ st_finalize_nir(struct st_context *st, struct 
> gl_program *prog,
>  struct gl_shader_program *shader_program, nir_shader *nir)
>  {
> struct pipe_screen *screen = st->pipe->screen;
> +   unsigned glsl_version = screen->get_param(screen, 
> PIPE_CAP_GLSL_FEATURE_LEVEL);
>
> NIR_PASS_V(nir, nir_split_var_copies);
> NIR_PASS_V(nir, nir_lower_var_copies);
> if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
> -   nir->info.stage != MESA_SHADER_TESS_EVAL)
> +   nir->info.stage != MESA_SHADER_TESS_EVAL &&
> +   (nir->info.stage != MESA_SHADER_FRAGMENT ||
> +(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {
>NIR_PASS_V(nir, nir_lower_io_arrays_to_elements_no_indirects);
> +   }
>
> if (nir->info.stage == MESA_SHADER_VERTEX) {
>/* Needs special handling so drvloc matches the vbo state: */
> --
> 2.14.3
>
> ___
> 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 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

2018-01-29 Thread Marek Olšák

On Mon, Jan 15, 2018 at 4:46 AM, Timothy Arceri  wrote:
> We need this to be able to support the interpolateAt builtins in a
> sane way. It also leads to the generation of more optimal code.
>
> The lowering and splitting is made conditional on glsl 400 because
> vc4 and freedreno both expect these passes to be enabled and niether
> support glsl 400 so don't need to deal with the interpolateAt builtins.
>
> We leave the other stages for now as to avoid regressions.
> ---
>  src/mesa/state_tracker/st_glsl_to_nir.cpp | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
> b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index 6e3a1548f4..bc55c5b7db 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -461,7 +461,9 @@ st_nir_get_mesa_program(struct gl_context *ctx,
>  struct gl_linked_shader *shader)
>  {
> struct st_context *st = st_context(ctx);
> +   struct pipe_screen *screen = st->pipe->screen;
> struct gl_program *prog;
> +   unsigned glsl_version = screen->get_param(screen, 
> PIPE_CAP_GLSL_FEATURE_LEVEL);
>
> validate_ir_tree(shader->ir);
>
> @@ -491,11 +493,14 @@ st_nir_get_mesa_program(struct gl_context *ctx,
> prog->nir = nir;
>
> if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
> -   nir->info.stage != MESA_SHADER_TESS_EVAL) {
> +   nir->info.stage != MESA_SHADER_TESS_EVAL &&
> +   (nir->info.stage != MESA_SHADER_FRAGMENT ||
> +(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {

SInce it's clear we don't want that pass in a lot of cases, you could
just replace the whole conditional with:
if (glsl_version < 400) {

>NIR_PASS_V(nir, nir_lower_io_to_temporaries,
>   nir_shader_get_entrypoint(nir),
>   true, true);
> }
> +
> NIR_PASS_V(nir, nir_lower_global_vars_to_local);
> NIR_PASS_V(nir, nir_split_var_copies);
> NIR_PASS_V(nir, nir_lower_var_copies);
> @@ -665,12 +670,16 @@ st_finalize_nir(struct st_context *st, struct 
> gl_program *prog,
>  struct gl_shader_program *shader_program, nir_shader *nir)
>  {
> struct pipe_screen *screen = st->pipe->screen;
> +   unsigned glsl_version = screen->get_param(screen, 
> PIPE_CAP_GLSL_FEATURE_LEVEL);
>
> NIR_PASS_V(nir, nir_split_var_copies);
> NIR_PASS_V(nir, nir_lower_var_copies);
> if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
> -   nir->info.stage != MESA_SHADER_TESS_EVAL)
> +   nir->info.stage != MESA_SHADER_TESS_EVAL &&
> +   (nir->info.stage != MESA_SHADER_FRAGMENT ||
> +(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {

Same here.

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

Re: [Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

2018-01-18 Thread Dieter Nützel


For the series:

Tested-by: Dieter Nützel 

on RX580 with UH, UV, glmark2 and Blender 2.79

UH and UV crash for 'wireframe' (as expected - not implemented, yet) ;-)

Dieter

Am 15.01.2018 04:46, schrieb Timothy Arceri:

We need this to be able to support the interpolateAt builtins in a
sane way. It also leads to the generation of more optimal code.

The lowering and splitting is made conditional on glsl 400 because
vc4 and freedreno both expect these passes to be enabled and niether
support glsl 400 so don't need to deal with the interpolateAt builtins.

We leave the other stages for now as to avoid regressions.
---
 src/mesa/state_tracker/st_glsl_to_nir.cpp | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 6e3a1548f4..bc55c5b7db 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -461,7 +461,9 @@ st_nir_get_mesa_program(struct gl_context *ctx,
 struct gl_linked_shader *shader)
 {
struct st_context *st = st_context(ctx);
+   struct pipe_screen *screen = st->pipe->screen;
struct gl_program *prog;
+   unsigned glsl_version = screen->get_param(screen,
PIPE_CAP_GLSL_FEATURE_LEVEL);

validate_ir_tree(shader->ir);

@@ -491,11 +493,14 @@ st_nir_get_mesa_program(struct gl_context *ctx,
prog->nir = nir;

if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
-   nir->info.stage != MESA_SHADER_TESS_EVAL) {
+   nir->info.stage != MESA_SHADER_TESS_EVAL &&
+   (nir->info.stage != MESA_SHADER_FRAGMENT ||
+(glsl_version < 400 && nir->info.stage == 
MESA_SHADER_FRAGMENT))) {

   NIR_PASS_V(nir, nir_lower_io_to_temporaries,
  nir_shader_get_entrypoint(nir),
  true, true);
}
+
NIR_PASS_V(nir, nir_lower_global_vars_to_local);
NIR_PASS_V(nir, nir_split_var_copies);
NIR_PASS_V(nir, nir_lower_var_copies);
@@ -665,12 +670,16 @@ st_finalize_nir(struct st_context *st, struct
gl_program *prog,
 struct gl_shader_program *shader_program, nir_shader 
*nir)

 {
struct pipe_screen *screen = st->pipe->screen;
+   unsigned glsl_version = screen->get_param(screen,
PIPE_CAP_GLSL_FEATURE_LEVEL);

NIR_PASS_V(nir, nir_split_var_copies);
NIR_PASS_V(nir, nir_lower_var_copies);
if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
-   nir->info.stage != MESA_SHADER_TESS_EVAL)
+   nir->info.stage != MESA_SHADER_TESS_EVAL &&
+   (nir->info.stage != MESA_SHADER_FRAGMENT ||
+(glsl_version < 400 && nir->info.stage == 
MESA_SHADER_FRAGMENT))) {

   NIR_PASS_V(nir, nir_lower_io_arrays_to_elements_no_indirects);
+   }

if (nir->info.stage == MESA_SHADER_VERTEX) {
   /* Needs special handling so drvloc matches the vbo state: */

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

[Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

2018-01-14 Thread Timothy Arceri

We need this to be able to support the interpolateAt builtins in a
sane way. It also leads to the generation of more optimal code.

The lowering and splitting is made conditional on glsl 400 because
vc4 and freedreno both expect these passes to be enabled and niether
support glsl 400 so don't need to deal with the interpolateAt builtins.

We leave the other stages for now as to avoid regressions.
---
 src/mesa/state_tracker/st_glsl_to_nir.cpp | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 6e3a1548f4..bc55c5b7db 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -461,7 +461,9 @@ st_nir_get_mesa_program(struct gl_context *ctx,
 struct gl_linked_shader *shader)
 {
struct st_context *st = st_context(ctx);
+   struct pipe_screen *screen = st->pipe->screen;
struct gl_program *prog;
+   unsigned glsl_version = screen->get_param(screen, 
PIPE_CAP_GLSL_FEATURE_LEVEL);
 
validate_ir_tree(shader->ir);
 
@@ -491,11 +493,14 @@ st_nir_get_mesa_program(struct gl_context *ctx,
prog->nir = nir;
 
if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
-   nir->info.stage != MESA_SHADER_TESS_EVAL) {
+   nir->info.stage != MESA_SHADER_TESS_EVAL &&
+   (nir->info.stage != MESA_SHADER_FRAGMENT ||
+(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {
   NIR_PASS_V(nir, nir_lower_io_to_temporaries,
  nir_shader_get_entrypoint(nir),
  true, true);
}
+
NIR_PASS_V(nir, nir_lower_global_vars_to_local);
NIR_PASS_V(nir, nir_split_var_copies);
NIR_PASS_V(nir, nir_lower_var_copies);
@@ -665,12 +670,16 @@ st_finalize_nir(struct st_context *st, struct gl_program 
*prog,
 struct gl_shader_program *shader_program, nir_shader *nir)
 {
struct pipe_screen *screen = st->pipe->screen;
+   unsigned glsl_version = screen->get_param(screen, 
PIPE_CAP_GLSL_FEATURE_LEVEL);
 
NIR_PASS_V(nir, nir_split_var_copies);
NIR_PASS_V(nir, nir_lower_var_copies);
if (nir->info.stage != MESA_SHADER_TESS_CTRL &&
-   nir->info.stage != MESA_SHADER_TESS_EVAL)
+   nir->info.stage != MESA_SHADER_TESS_EVAL &&
+   (nir->info.stage != MESA_SHADER_FRAGMENT ||
+(glsl_version < 400 && nir->info.stage == MESA_SHADER_FRAGMENT))) {
   NIR_PASS_V(nir, nir_lower_io_arrays_to_elements_no_indirects);
+   }
 
if (nir->info.stage == MESA_SHADER_VERTEX) {
   /* Needs special handling so drvloc matches the vbo state: */
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

Re: [Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

Re: [Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

Re: [Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

[Mesa-dev] [PATCH 8/8] st/glsl_to_nir: disable io lowering and forced indirect array splitting in fs

5 matches

Site Navigation

Mail list logo

Footer information