[Mesa-dev] [PATCH 2/6] i965/fs: Use a helper function for checking for flow control instructions.

[Kenneth Graunke]

On 02/06/2013 05:29 PM, Eric Anholt wrote:
> In 2 of our checks, we were missing BREAK and CONTINUE.
>
> NOTE: Candidate for the stable branches.
> ---
>   src/mesa/drivers/dri/i965/brw_fs.cpp               |   33 +++++++++++---------
>   src/mesa/drivers/dri/i965/brw_fs.h                 |    1 +
>   .../dri/i965/brw_fs_schedule_instructions.cpp      |    9 +-----
>   3 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 8e57eb0..fdccd75 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -328,6 +328,23 @@ fs_inst::is_math()
>   }
>
>   bool
> +fs_inst::is_flow_control()
> +{
> +   switch (opcode) {
> +   case BRW_OPCODE_DO:
> +   case BRW_OPCODE_WHILE:
> +   case BRW_OPCODE_IF:
> +   case BRW_OPCODE_ELSE:
> +   case BRW_OPCODE_ENDIF:
> +   case BRW_OPCODE_BREAK:
> +   case BRW_OPCODE_CONTINUE:
> +      return true;
> +   default:
> +      return false;
> +   }
> +}

Shouldn't BRW_OPCODE_HALT be counted as a flow control instruction too?

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

> +bool
>   fs_inst::is_send_from_grf()
>   {
>      return (opcode == FS_OPCODE_VARYING_PULL_CONSTANT_LOAD_GEN7 ||
> @@ -2074,12 +2091,8 @@ fs_visitor::compute_to_mrf()
>   	  * values that end up in MRFs are shortly before the MRF
>   	  * write anyway.
>   	  */
> -	 if (scan_inst->opcode == BRW_OPCODE_DO ||
> -	     scan_inst->opcode == BRW_OPCODE_WHILE ||
> -	     scan_inst->opcode == BRW_OPCODE_ELSE ||
> -	     scan_inst->opcode == BRW_OPCODE_ENDIF) {
> +	 if (scan_inst->is_flow_control() && scan_inst->opcode != BRW_OPCODE_IF)
>   	    break;
> -	 }
>
>   	 /* You can't read from an MRF, so if someone else reads our
>   	  * MRF's source GRF that we wanted to rewrite, that stops us.
> @@ -2163,16 +2176,8 @@ fs_visitor::remove_duplicate_mrf_writes()
>      foreach_list_safe(node, &this->instructions) {
>         fs_inst *inst = (fs_inst *)node;
>
> -      switch (inst->opcode) {
> -      case BRW_OPCODE_DO:
> -      case BRW_OPCODE_WHILE:
> -      case BRW_OPCODE_IF:
> -      case BRW_OPCODE_ELSE:
> -      case BRW_OPCODE_ENDIF:
> +      if (inst->is_flow_control()) {
>   	 memset(last_mrf_move, 0, sizeof(last_mrf_move));
> -	 continue;
> -      default:
> -	 break;
>         }
>
>         if (inst->opcode == BRW_OPCODE_MOV &&
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index d332502..dbf9e05 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -178,6 +178,7 @@ public:
>      bool overwrites_reg(const fs_reg &reg);
>      bool is_tex();
>      bool is_math();
> +   bool is_flow_control();
>      bool is_send_from_grf();
>
>      fs_reg dst;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_schedule_instructions.cpp b/src/mesa/drivers/dri/i965/brw_fs_schedule_instructions.cpp
> index 3fbca6c..a268058 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_schedule_instructions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_schedule_instructions.cpp
> @@ -816,15 +816,8 @@ fs_visitor::schedule_instructions(bool post_reg_alloc)
>   	 next_block_header = (fs_inst *)next_block_header->next;
>
>   	 sched.add_inst(inst);
> -	 if (inst->opcode == BRW_OPCODE_IF ||
> -	     inst->opcode == BRW_OPCODE_ELSE ||
> -	     inst->opcode == BRW_OPCODE_ENDIF ||
> -	     inst->opcode == BRW_OPCODE_DO ||
> -	     inst->opcode == BRW_OPCODE_WHILE ||
> -	     inst->opcode == BRW_OPCODE_BREAK ||
> -	     inst->opcode == BRW_OPCODE_CONTINUE) {
> +         if (inst->is_flow_control())
>   	    break;
> -	 }
>         }
>         sched.calculate_deps();
>         sched.schedule_instructions(next_block_header);
>