Re: [Mesa-dev] [PATCH] radv: keep a stage mask per pipeline. (v2)

[Samuel Pitoiset]

On 11/06/2017 06:37 AM, Dave Airlie wrote:

From: Dave Airlie 

This should reduce some pointless loops.

v2: fix missing check which causes crashes with compute shaders

Signed-off-by: Dave Airlie 
---
  src/amd/vulkan/radv_cmd_buffer.c | 53 +++-
  src/amd/vulkan/radv_pipeline.c   |  2 ++
  src/amd/vulkan/radv_private.h|  1 +
  3 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 833c3eb3f0d..7357eadae39 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -682,17 +682,10 @@ static void
  radv_emit_shaders_prefetch(struct radv_cmd_buffer *cmd_buffer,
   struct radv_pipeline *pipeline)
  {
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_VERTEX]);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_TESS_CTRL]);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_TESS_EVAL]);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_GEOMETRY]);
+   radv_foreach_stage(stage, pipeline->stage_mask)
+   radv_emit_shader_prefetch(cmd_buffer,
+ pipeline->shaders[stage]);
radv_emit_shader_prefetch(cmd_buffer, pipeline->gs_copy_shader);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_FRAGMENT]);
  }
  
  static void

@@ -1603,12 +1596,11 @@ radv_emit_descriptor_set_userdata(struct 
radv_cmd_buffer *cmd_buffer,
  struct radv_descriptor_set *set,
  unsigned idx)
  {
-   if (cmd_buffer->state.pipeline) {
+   if (cmd_buffer->state.pipeline && (stages & 
cmd_buffer->state.pipeline->stage_mask)) {
radv_foreach_stage(stage, stages) {


Why don't you use 'cmd_buffer->state.pipeline->stage_mask' instead of 
'stages' here?



-   if (cmd_buffer->state.pipeline->shaders[stage])
-   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
-  idx, set->va,
-  stage);
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  stage);
}
}
  
@@ -1658,25 +1650,10 @@ radv_flush_indirect_descriptor_sets(struct radv_cmd_buffer *cmd_buffer)

va += offset;
  
  	if (cmd_buffer->state.pipeline) {

-   if (cmd_buffer->state.pipeline->shaders[MESA_SHADER_VERTEX])
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_VERTEX,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (cmd_buffer->state.pipeline->shaders[MESA_SHADER_FRAGMENT])
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_FRAGMENT,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (radv_pipeline_has_gs(cmd_buffer->state.pipeline))
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_GEOMETRY,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (radv_pipeline_has_tess(cmd_buffer->state.pipeline))
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_TESS_CTRL,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (radv_pipeline_has_tess(cmd_buffer->state.pipeline))
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_TESS_EVAL,
+   radv_foreach_stage(stage, 
cmd_buffer->state.pipeline->stage_mask) {
+   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, stage,
   
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
+   }
}
  
  	if (cmd_buffer->state.compute_pipeline)

@@ -1751,10 +1728,8 @@ radv_flush_constants(struct radv_cmd_buffer *cmd_buffer,
   cmd_buffer->cs, 
MESA_SHADER_STAGES * 4);
  
  	radv_foreach_stage(stage, stages) {

-   if (pipeline->shaders[stage]) {
-   radv_emit_userdata_address(cmd_buffer, pipeline, stage,
-   

[Mesa-dev] [PATCH] radv: keep a stage mask per pipeline. (v2)

[Dave Airlie]

From: Dave Airlie 

This should reduce some pointless loops.

v2: fix missing check which causes crashes with compute shaders

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_cmd_buffer.c | 53 +++-
 src/amd/vulkan/radv_pipeline.c   |  2 ++
 src/amd/vulkan/radv_private.h|  1 +
 3 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 833c3eb3f0d..7357eadae39 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -682,17 +682,10 @@ static void
 radv_emit_shaders_prefetch(struct radv_cmd_buffer *cmd_buffer,
   struct radv_pipeline *pipeline)
 {
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_VERTEX]);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_TESS_CTRL]);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_TESS_EVAL]);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_GEOMETRY]);
+   radv_foreach_stage(stage, pipeline->stage_mask)
+   radv_emit_shader_prefetch(cmd_buffer,
+ pipeline->shaders[stage]);
radv_emit_shader_prefetch(cmd_buffer, pipeline->gs_copy_shader);
-   radv_emit_shader_prefetch(cmd_buffer,
- pipeline->shaders[MESA_SHADER_FRAGMENT]);
 }
 
 static void
@@ -1603,12 +1596,11 @@ radv_emit_descriptor_set_userdata(struct 
radv_cmd_buffer *cmd_buffer,
  struct radv_descriptor_set *set,
  unsigned idx)
 {
-   if (cmd_buffer->state.pipeline) {
+   if (cmd_buffer->state.pipeline && (stages & 
cmd_buffer->state.pipeline->stage_mask)) {
radv_foreach_stage(stage, stages) {
-   if (cmd_buffer->state.pipeline->shaders[stage])
-   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
-  idx, set->va,
-  stage);
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  stage);
}
}
 
@@ -1658,25 +1650,10 @@ radv_flush_indirect_descriptor_sets(struct 
radv_cmd_buffer *cmd_buffer)
va += offset;
 
if (cmd_buffer->state.pipeline) {
-   if (cmd_buffer->state.pipeline->shaders[MESA_SHADER_VERTEX])
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_VERTEX,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (cmd_buffer->state.pipeline->shaders[MESA_SHADER_FRAGMENT])
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_FRAGMENT,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (radv_pipeline_has_gs(cmd_buffer->state.pipeline))
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_GEOMETRY,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (radv_pipeline_has_tess(cmd_buffer->state.pipeline))
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_TESS_CTRL,
-  
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
-
-   if (radv_pipeline_has_tess(cmd_buffer->state.pipeline))
-   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, MESA_SHADER_TESS_EVAL,
+   radv_foreach_stage(stage, 
cmd_buffer->state.pipeline->stage_mask) {
+   radv_emit_userdata_address(cmd_buffer, 
cmd_buffer->state.pipeline, stage,
   
AC_UD_INDIRECT_DESCRIPTOR_SETS, va);
+   }
}
 
if (cmd_buffer->state.compute_pipeline)
@@ -1751,10 +1728,8 @@ radv_flush_constants(struct radv_cmd_buffer *cmd_buffer,
   cmd_buffer->cs, 
MESA_SHADER_STAGES * 4);
 
radv_foreach_stage(stage, stages) {
-   if (pipeline->shaders[stage]) {
-   radv_emit_userdata_address(cmd_buffer, pipeline, stage,
-  AC_UD_PUSH_CONSTANTS, va);
-   }
+   radv_emit_userdata_address(cmd_buffer, pipeline,