Re: [Mesa-dev] [PATCH v2 13/20] i965/fs: Support compute programs in fs_visitor

2015-04-27 Thread Kenneth Graunke
On Friday, April 24, 2015 04:33:05 PM Jordan Justen wrote:
> v2:
>  * Clean out some unneeded code copied from run_fs (krh)
>  * Always use NIR
>  * Split shader time out into a separate commit
> 
> Signed-off-by: Jordan Justen 
> ---
>  src/mesa/drivers/dri/i965/brw_context.c  |  2 +
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 61 
> ++--
>  src/mesa/drivers/dri/i965/brw_fs.h   | 10 +
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 23 +++
>  4 files changed, 93 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 9d90360..25b50c7 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -594,6 +594,8 @@ brw_initialize_context_constants(struct brw_context *brw)
> if (brw_env_var_as_boolean("INTEL_USE_NIR", true))
>ctx->Const.ShaderCompilerOptions[MESA_SHADER_FRAGMENT].NirOptions = 
> &nir_options;
>  
> +   ctx->Const.ShaderCompilerOptions[MESA_SHADER_COMPUTE].NirOptions = 
> &nir_options;
> +
> /* ARB_viewport_array */
> if (brw->gen >= 7 && ctx->API == API_OPENGL_CORE) {
>ctx->Const.MaxViewports = GEN7_NUM_VIEWPORTS;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 61ee056..24d3fa9 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1719,9 +1719,15 @@ fs_visitor::assign_curb_setup()
> if (dispatch_width == 8) {
>prog_data->dispatch_grf_start_reg = payload.num_regs;
> } else {
> -  assert(stage == MESA_SHADER_FRAGMENT);
> -  brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
> -  prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
> +  if (stage == MESA_SHADER_FRAGMENT) {
> + brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
> + prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
> +  } else if (stage == MESA_SHADER_COMPUTE) {
> + brw_cs_prog_data *prog_data = (brw_cs_prog_data*) this->prog_data;
> + prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
> +  } else {
> + unreachable("Unsupported shader type!");
> +  }
> }

We should be able to eliminate brw_cs_prog_data::dispatch_grf_start_reg_16
and just use brw_stage_prog_data::dispatch_grf_start_reg (since compute
shaders only use one SIMD mode).

I think you want to simplify this hunk to:

if (stage != MESA_SHADER_FRAGMENT || dispatch_width == 8) {
   prog_data->dispatch_grf_start_reg = payload.num_regs;
} else {
   brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
   prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
}

>  
> prog_data->curb_read_length = ALIGN(stage_prog_data->nr_params, 8) / 8;
> @@ -3726,6 +3732,14 @@ fs_visitor::setup_vs_payload()
>  }
>  
>  void
> +fs_visitor::setup_cs_payload()
> +{
> +   assert(brw->gen >= 7);
> +
> +   payload.num_regs = 1;
> +}

Are you planning to add more code here?  Not sure if it's worth a
helper function.  It's probably fine though :)

> +
> +void
>  fs_visitor::assign_binding_table_offsets()
>  {
> assert(stage == MESA_SHADER_FRAGMENT);
> @@ -4065,6 +4079,47 @@ fs_visitor::run_fs()
> return !failed;
>  }
>  
> +bool
> +fs_visitor::run_cs()
> +{
> +   assert(stage == MESA_SHADER_COMPUTE);
> +   assert (shader);

   ^^^ bonus space

Patches 1-13 are:
Reviewed-by: Kenneth Graunke 


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


[Mesa-dev] [PATCH v2 13/20] i965/fs: Support compute programs in fs_visitor

2015-04-24 Thread Jordan Justen
v2:
 * Clean out some unneeded code copied from run_fs (krh)
 * Always use NIR
 * Split shader time out into a separate commit

Signed-off-by: Jordan Justen 
---
 src/mesa/drivers/dri/i965/brw_context.c  |  2 +
 src/mesa/drivers/dri/i965/brw_fs.cpp | 61 ++--
 src/mesa/drivers/dri/i965/brw_fs.h   | 10 +
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 23 +++
 4 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index 9d90360..25b50c7 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -594,6 +594,8 @@ brw_initialize_context_constants(struct brw_context *brw)
if (brw_env_var_as_boolean("INTEL_USE_NIR", true))
   ctx->Const.ShaderCompilerOptions[MESA_SHADER_FRAGMENT].NirOptions = 
&nir_options;
 
+   ctx->Const.ShaderCompilerOptions[MESA_SHADER_COMPUTE].NirOptions = 
&nir_options;
+
/* ARB_viewport_array */
if (brw->gen >= 7 && ctx->API == API_OPENGL_CORE) {
   ctx->Const.MaxViewports = GEN7_NUM_VIEWPORTS;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 61ee056..24d3fa9 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -1719,9 +1719,15 @@ fs_visitor::assign_curb_setup()
if (dispatch_width == 8) {
   prog_data->dispatch_grf_start_reg = payload.num_regs;
} else {
-  assert(stage == MESA_SHADER_FRAGMENT);
-  brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
-  prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
+  if (stage == MESA_SHADER_FRAGMENT) {
+ brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
+ prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
+  } else if (stage == MESA_SHADER_COMPUTE) {
+ brw_cs_prog_data *prog_data = (brw_cs_prog_data*) this->prog_data;
+ prog_data->dispatch_grf_start_reg_16 = payload.num_regs;
+  } else {
+ unreachable("Unsupported shader type!");
+  }
}
 
prog_data->curb_read_length = ALIGN(stage_prog_data->nr_params, 8) / 8;
@@ -3726,6 +3732,14 @@ fs_visitor::setup_vs_payload()
 }
 
 void
+fs_visitor::setup_cs_payload()
+{
+   assert(brw->gen >= 7);
+
+   payload.num_regs = 1;
+}
+
+void
 fs_visitor::assign_binding_table_offsets()
 {
assert(stage == MESA_SHADER_FRAGMENT);
@@ -4065,6 +4079,47 @@ fs_visitor::run_fs()
return !failed;
 }
 
+bool
+fs_visitor::run_cs()
+{
+   assert(stage == MESA_SHADER_COMPUTE);
+   assert (shader);
+
+   sanity_param_count = prog->Parameters->NumParameters;
+
+   assign_common_binding_table_offsets(0);
+
+   setup_cs_payload();
+
+   emit_nir_code();
+
+   if (failed)
+  return false;
+
+   emit_cs_terminate();
+
+   calculate_cfg();
+
+   optimize();
+
+   assign_curb_setup();
+
+   fixup_3src_null_dest();
+   allocate_registers();
+
+   if (failed)
+  return false;
+
+   /* If any state parameters were appended, then ParameterValues could have
+* been realloced, in which case the driver uniform storage set up by
+* _mesa_associate_uniform_storage() would point to freed memory.  Make
+* sure that didn't happen.
+*/
+   assert(sanity_param_count == prog->Parameters->NumParameters);
+
+   return !failed;
+}
+
 const unsigned *
 brw_wm_fs_emit(struct brw_context *brw,
void *mem_ctx,
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index 8a71ac7..d233260 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -89,6 +89,14 @@ public:
   struct gl_vertex_program *cp,
   unsigned dispatch_width);
 
+   fs_visitor(struct brw_context *brw,
+  void *mem_ctx,
+  const struct brw_cs_prog_key *key,
+  struct brw_cs_prog_data *prog_data,
+  struct gl_shader_program *shader_prog,
+  struct gl_compute_program *cp,
+  unsigned dispatch_width);
+
~fs_visitor();
void init();
 
@@ -189,12 +197,14 @@ public:
 
bool run_fs();
bool run_vs();
+   bool run_cs();
void optimize();
void allocate_registers();
void assign_binding_table_offsets();
void setup_payload_gen4();
void setup_payload_gen6();
void setup_vs_payload();
+   void setup_cs_payload();
void fixup_3src_null_dest();
void assign_curb_setup();
void calculate_urb_setup();
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index ba8b811..74f2e52 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -39,6 +39,7 @@
 #include "brw_context.h"
 #include "brw_eu.h"
 #include "brw_wm.h"
+#include "brw_cs.h"
 #include "brw_vec4.h"
 #include "brw_fs.h"
 #include "main/uniforms.h"
@@ -4232,6 +4233,25 @@ fs_visitor::