[Mesa-dev] [PATCH 1/5] i965: Add writes_accumulator flag

Matt Turner mattst88 at gmail.com
Wed Apr 9 13:58:45 PDT 2014


On Fri, Apr 4, 2014 at 6:51 AM, Juha-Pekka Heikkila
<juhapekka.heikkila at gmail.com> wrote:
> diff --git a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> index a951459..92f82fd 100644
> --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> @@ -758,6 +758,7 @@ fs_instruction_scheduler::calculate_deps()
>     schedule_node *last_fixed_grf_write = NULL;
>     int reg_width = v->dispatch_width / 8;
>
> +   schedule_node *last_accumulator_write = NULL;
>     /* The last instruction always needs to still be the last
>      * instruction.  Either it's flow control (IF, ELSE, ENDIF, DO,
>      * WHILE) and scheduling other things after it would disturb the
> @@ -822,6 +823,10 @@ fs_instruction_scheduler::calculate_deps()

The line before this was
  if (inst->reads_flag()) {
>          add_dep(last_conditional_mod[inst->flag_subreg], n);
>        }
>
> +      if (inst->writes_accumulator || inst->dst.is_accumulator()) {
> +         add_dep(last_accumulator_write, n);
> +      }

But we're checking if we're writing the accumulator here, instead of reading it.

We're also not giving the scheduler any benefits from it's new
knowledge of accumulator dependencies, because we're still calling
add_barrier_deps() above when we don't recognize the destination. I
hope you don't mind, but I split the is_accumulator() additions into a
separate patch, fixed up the scheduler hunks and sent the revised
patch. Let me know if it looks right to you.


More information about the mesa-dev mailing list