[Mesa-dev] [PATCH] i965/sched: Simplify work done by add_barrier_deps().

[Kenneth Graunke]

On Thursday, August 18, 2016 5:17:43 PM PDT Matt Turner wrote:
> Scheduling barriers are implemented by placing a dependence on every
> node before and after the barrier. This is unnecessary as we can limit
> the number of nodes we place dependencies on to those between us and the
> next barrier in each direction.
> 
> Runtime of dEQP-GLES31.functional.ssbo.layout.random.all_shared_buffer.23
> is reduced from ~25 minutes to a little more than three.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94681
> ---
> Would people rather I make is_scheduling_barrier() a virtual member of
> backend_instruction and move the implementations into vec4_instruction
> and fs_inst?
> 
>  src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> index 8afdc25..2e53f9d 100644
> --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> @@ -86,6 +86,8 @@ public:
>      * its children, or just the issue_time if it's a leaf node.
>      */
>     int delay;
> +
> +   bool is_barrier;
>  };
>  
>  void
> @@ -772,6 +774,7 @@ schedule_node::schedule_node(backend_instruction *inst,
>     this->unblocked_time = 0;
>     this->cand_generation = 0;
>     this->delay = 0;
> +   this->is_barrier = false;
>  
>     /* We can't measure Gen6 timings directly but expect them to be much
>      * closer to Gen7 than Gen4.
> @@ -872,9 +875,13 @@ instruction_scheduler::add_barrier_deps(schedule_node *n)
>     schedule_node *prev = (schedule_node *)n->prev;
>     schedule_node *next = (schedule_node *)n->next;
>  
> +   n->is_barrier = true;
> +
>     if (prev) {
>        while (!prev->is_head_sentinel()) {
>           add_dep(prev, n, 0);
> +         if (prev->is_barrier)
> +            break;
>           prev = (schedule_node *)prev->prev;
>        }
>     }
> @@ -882,6 +889,8 @@ instruction_scheduler::add_barrier_deps(schedule_node *n)
>     if (next) {
>        while (!next->is_tail_sentinel()) {
>           add_dep(n, next, 0);
> +         if (next->is_barrier)
> +            break;
>           next = (schedule_node *)next->next;
>        }
>     }
> 

I think adding the bool is reasonable.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks for fixing this!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160819/6fe5ef32/attachment.sig>