[Mesa-dev] [PATCH 04/20] i965/vec4: Preserve CFG in spill_reg().

Pohjolainen, Topi topi.pohjolainen at intel.com
Sat Sep 6 00:35:46 PDT 2014


On Tue, Sep 02, 2014 at 09:34:15PM -0700, Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp             |  6 ++-
>  src/mesa/drivers/dri/i965/brw_vec4.h               | 10 ++--
>  .../drivers/dri/i965/brw_vec4_reg_allocate.cpp     | 11 ++--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     | 62 +++++++++++++---------
>  4 files changed, 54 insertions(+), 35 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index f1c5210..6ccd6a1 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -804,10 +804,12 @@ vec4_visitor::move_push_constants_to_pull_constants()
>        }
>     }
>  
> +   calculate_cfg();
> +
>     /* Now actually rewrite usage of the things we've moved to pull
>      * constants.
>      */
> -   foreach_in_list_safe(vec4_instruction, inst, &instructions) {
> +   foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
>        for (int i = 0 ; i < 3; i++) {
>  	 if (inst->src[i].file != UNIFORM ||
>  	     pull_constant_loc[inst->src[i].reg] == -1)
> @@ -817,7 +819,7 @@ vec4_visitor::move_push_constants_to_pull_constants()
>  
>  	 dst_reg temp = dst_reg(this, glsl_type::vec4_type);
>  
> -	 emit_pull_constant_load(inst, temp, inst->src[i],
> +	 emit_pull_constant_load(block, inst, temp, inst->src[i],
>  				 pull_constant_loc[uniform]);
>  
>  	 inst->src[i].file = temp.file;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 186667c..e752415 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -549,17 +549,17 @@ public:
>     void emit_untyped_surface_read(unsigned surf_index, dst_reg dst,
>                                    src_reg offset);
>  
> -   src_reg get_scratch_offset(vec4_instruction *inst,
> +   src_reg get_scratch_offset(bblock_t *block, vec4_instruction *inst,
>  			      src_reg *reladdr, int reg_offset);
> -   src_reg get_pull_constant_offset(vec4_instruction *inst,
> +   src_reg get_pull_constant_offset(bblock_t *block, vec4_instruction *inst,
>  				    src_reg *reladdr, int reg_offset);
> -   void emit_scratch_read(vec4_instruction *inst,
> +   void emit_scratch_read(bblock_t *block, vec4_instruction *inst,
>  			  dst_reg dst,
>  			  src_reg orig_src,
>  			  int base_offset);
> -   void emit_scratch_write(vec4_instruction *inst,
> +   void emit_scratch_write(bblock_t *block, vec4_instruction *inst,
>  			   int base_offset);
> -   void emit_pull_constant_load(vec4_instruction *inst,
> +   void emit_pull_constant_load(bblock_t *block, vec4_instruction *inst,
>  				dst_reg dst,
>  				src_reg orig_src,
>  				int base_offset);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
> index ddab342..72c72e6 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
> @@ -28,6 +28,7 @@ extern "C" {
>  
>  #include "brw_vec4.h"
>  #include "brw_vs.h"
> +#include "brw_cfg.h"
>  
>  using namespace brw;
>  
> @@ -326,8 +327,10 @@ vec4_visitor::spill_reg(int spill_reg_nr)
>     assert(virtual_grf_sizes[spill_reg_nr] == 1);
>     unsigned int spill_offset = c->last_scratch++;
>  
> +   calculate_cfg();
> +
>     /* Generate spill/unspill instructions for the objects being spilled. */
> -   foreach_in_list(vec4_instruction, inst, &instructions) {
> +   foreach_block_and_inst(block, vec4_instruction, inst, cfg) {
>        for (unsigned int i = 0; i < 3; i++) {
>           if (inst->src[i].file == GRF && inst->src[i].reg == spill_reg_nr) {
>              src_reg spill_reg = inst->src[i];
> @@ -342,16 +345,16 @@ vec4_visitor::spill_reg(int spill_reg_nr)
>                 temp.writemask |= (1 << BRW_GET_SWZ(inst->src[i].swizzle, c));
>              assert(temp.writemask != 0);
>  
> -            emit_scratch_read(inst, temp, spill_reg, spill_offset);
> +            emit_scratch_read(block, inst, temp, spill_reg, spill_offset);
>           }
>        }
>  
>        if (inst->dst.file == GRF && inst->dst.reg == spill_reg_nr) {
> -         emit_scratch_write(inst, spill_offset);
> +         emit_scratch_write(block, inst, spill_offset);
>        }
>     }
>  
> -   invalidate_live_intervals();
> +   invalidate_live_intervals(false);
>  }
>  
>  } /* namespace brw */
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index ce94db8..300da35 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "brw_vec4.h"
> +#include "brw_cfg.h"
>  #include "glsl/ir_uniform.h"
>  extern "C" {
>  #include "program/sampler.h"
> @@ -3147,7 +3148,7 @@ vec4_visitor::emit_vertex()
>  
>  
>  src_reg
> -vec4_visitor::get_scratch_offset(vec4_instruction *inst,
> +vec4_visitor::get_scratch_offset(bblock_t *block, vec4_instruction *inst,
>  				 src_reg *reladdr, int reg_offset)
>  {
>     /* Because we store the values to scratch interleaved like our
> @@ -3164,9 +3165,10 @@ vec4_visitor::get_scratch_offset(vec4_instruction *inst,
>     if (reladdr) {
>        src_reg index = src_reg(this, glsl_type::int_type);
>  
> -      emit_before(inst, ADD(dst_reg(index), *reladdr, src_reg(reg_offset)));
> -      emit_before(inst, MUL(dst_reg(index),
> -			    index, src_reg(message_header_scale)));
> +      inst->insert_before(block, ADD(dst_reg(index), *reladdr,
> +                                     src_reg(reg_offset)));
> +      inst->insert_before(block, MUL(dst_reg(index), index,
> +                                     src_reg(message_header_scale)));
>  
>        return index;
>     } else {
> @@ -3175,26 +3177,27 @@ vec4_visitor::get_scratch_offset(vec4_instruction *inst,
>  }
>  
>  src_reg
> -vec4_visitor::get_pull_constant_offset(vec4_instruction *inst,
> +vec4_visitor::get_pull_constant_offset(bblock_t * block, vec4_instruction *inst,
>  				       src_reg *reladdr, int reg_offset)
>  {
>     if (reladdr) {
>        src_reg index = src_reg(this, glsl_type::int_type);
>  
> -      emit_before(inst, ADD(dst_reg(index), *reladdr, src_reg(reg_offset)));
> +      inst->insert_before(block, ADD(dst_reg(index), *reladdr,
> +                                     src_reg(reg_offset)));
>  
>        /* Pre-gen6, the message header uses byte offsets instead of vec4
>         * (16-byte) offset units.
>         */
>        if (brw->gen < 6) {
> -	 emit_before(inst, MUL(dst_reg(index), index, src_reg(16)));
> +         inst->insert_before(block, MUL(dst_reg(index), index, src_reg(16)));
>        }
>  
>        return index;
>     } else if (brw->gen >= 8) {
>        /* Store the offset in a GRF so we can send-from-GRF. */
>        src_reg offset = src_reg(this, glsl_type::int_type);
> -      emit_before(inst, MOV(dst_reg(offset), src_reg(reg_offset)));
> +      inst->insert_before(block, MOV(dst_reg(offset), src_reg(reg_offset)));
>        return offset;
>     } else {
>        int message_header_scale = brw->gen < 6 ? 16 : 1;
> @@ -3209,14 +3212,18 @@ vec4_visitor::get_pull_constant_offset(vec4_instruction *inst,
>   * @base_offset is measured in 32-byte units (the size of a register).
>   */
>  void
> -vec4_visitor::emit_scratch_read(vec4_instruction *inst,
> +vec4_visitor::emit_scratch_read(bblock_t *block, vec4_instruction *inst,
>  				dst_reg temp, src_reg orig_src,
>  				int base_offset)
>  {
>     int reg_offset = base_offset + orig_src.reg_offset;
> -   src_reg index = get_scratch_offset(inst, orig_src.reladdr, reg_offset);
> +   src_reg index = get_scratch_offset(block, inst, orig_src.reladdr,
> +                                      reg_offset);
>  
> -   emit_before(inst, SCRATCH_READ(temp, index));
> +   vec4_instruction *read = SCRATCH_READ(temp, index);
> +   read->ir = inst->ir;
> +   read->annotation = inst->annotation;

There are quite a few other changes from emit_before() to insert_before() in
this patch. But this is the only one that in addition preserves the old
behavior where the newly introduced instruction "inherits" the reference to
the intermediate instruction as well as the annotation. Why does it need to
be done here - or equivalently why isn't this needed in the other occurrences?

Other than this everything seems to add up. I checked the existing code and
I didn't see any insertion or removal missed by this patch - every occurence
within the context of this patch got switched to block aware counterparts.

> +   inst->insert_before(block, read);
>  }
>  
>  /**
> @@ -3226,10 +3233,12 @@ vec4_visitor::emit_scratch_read(vec4_instruction *inst,
>   * @base_offset is measured in 32-byte units (the size of a register).
>   */
>  void
> -vec4_visitor::emit_scratch_write(vec4_instruction *inst, int base_offset)
> +vec4_visitor::emit_scratch_write(bblock_t *block, vec4_instruction *inst,
> +                                 int base_offset)
>  {
>     int reg_offset = base_offset + inst->dst.reg_offset;
> -   src_reg index = get_scratch_offset(inst, inst->dst.reladdr, reg_offset);
> +   src_reg index = get_scratch_offset(block, inst, inst->dst.reladdr,
> +                                      reg_offset);
>  
>     /* Create a temporary register to store *inst's result in.
>      *
> @@ -3256,7 +3265,7 @@ vec4_visitor::emit_scratch_write(vec4_instruction *inst, int base_offset)
>     write->predicate = inst->predicate;
>     write->ir = inst->ir;
>     write->annotation = inst->annotation;
> -   inst->insert_after(write);
> +   inst->insert_after(block, write);
>  
>     inst->dst.file = temp.file;
>     inst->dst.reg = temp.reg;
> @@ -3279,11 +3288,13 @@ vec4_visitor::move_grf_array_access_to_scratch()
>        scratch_loc[i] = -1;
>     }
>  
> +   calculate_cfg();
> +
>     /* First, calculate the set of virtual GRFs that need to be punted
>      * to scratch due to having any array access on them, and where in
>      * scratch.
>      */
> -   foreach_in_list(vec4_instruction, inst, &instructions) {
> +   foreach_block_and_inst(block, vec4_instruction, inst, cfg) {
>        if (inst->dst.file == GRF && inst->dst.reladdr &&
>  	  scratch_loc[inst->dst.reg] == -1) {
>  	 scratch_loc[inst->dst.reg] = c->last_scratch;
> @@ -3306,13 +3317,13 @@ vec4_visitor::move_grf_array_access_to_scratch()
>      * we may generate a new scratch_write instruction after the one
>      * we're processing.
>      */
> -   foreach_in_list_safe(vec4_instruction, inst, &instructions) {
> +   foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
>        /* Set up the annotation tracking for new generated instructions. */
>        base_ir = inst->ir;
>        current_annotation = inst->annotation;
>  
>        if (inst->dst.file == GRF && scratch_loc[inst->dst.reg] != -1) {
> -	 emit_scratch_write(inst, scratch_loc[inst->dst.reg]);
> +	 emit_scratch_write(block, inst, scratch_loc[inst->dst.reg]);
>        }
>  
>        for (int i = 0 ; i < 3; i++) {
> @@ -3321,7 +3332,7 @@ vec4_visitor::move_grf_array_access_to_scratch()
>  
>  	 dst_reg temp = dst_reg(this, glsl_type::vec4_type);
>  
> -	 emit_scratch_read(inst, temp, inst->src[i],
> +	 emit_scratch_read(block, inst, temp, inst->src[i],
>  			   scratch_loc[inst->src[i].reg]);
>  
>  	 inst->src[i].file = temp.file;
> @@ -3337,19 +3348,20 @@ vec4_visitor::move_grf_array_access_to_scratch()
>   * from the pull constant buffer (surface) at @base_offset to @temp.
>   */
>  void
> -vec4_visitor::emit_pull_constant_load(vec4_instruction *inst,
> +vec4_visitor::emit_pull_constant_load(bblock_t *block, vec4_instruction *inst,
>  				      dst_reg temp, src_reg orig_src,
>  				      int base_offset)
>  {
>     int reg_offset = base_offset + orig_src.reg_offset;
>     src_reg index = src_reg(prog_data->base.binding_table.pull_constants_start);
> -   src_reg offset = get_pull_constant_offset(inst, orig_src.reladdr, reg_offset);
> +   src_reg offset = get_pull_constant_offset(block, inst, orig_src.reladdr,
> +                                             reg_offset);
>     vec4_instruction *load;
>  
>     if (brw->gen >= 7) {
>        dst_reg grf_offset = dst_reg(this, glsl_type::int_type);
>        grf_offset.type = offset.type;
> -      emit_before(inst, MOV(grf_offset, offset));
> +      inst->insert_before(block, MOV(grf_offset, offset));
>  
>        load = new(mem_ctx) vec4_instruction(this,
>                                             VS_OPCODE_PULL_CONSTANT_LOAD_GEN7,
> @@ -3360,7 +3372,7 @@ vec4_visitor::emit_pull_constant_load(vec4_instruction *inst,
>        load->base_mrf = 14;
>        load->mlen = 1;
>     }
> -   emit_before(inst, load);
> +   inst->insert_before(block, load);
>  }
>  
>  /**
> @@ -3384,13 +3396,15 @@ vec4_visitor::move_uniform_array_access_to_pull_constants()
>        pull_constant_loc[i] = -1;
>     }
>  
> +   calculate_cfg();
> +
>     /* Walk through and find array access of uniforms.  Put a copy of that
>      * uniform in the pull constant buffer.
>      *
>      * Note that we don't move constant-indexed accesses to arrays.  No
>      * testing has been done of the performance impact of this choice.
>      */
> -   foreach_in_list_safe(vec4_instruction, inst, &instructions) {
> +   foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
>        for (int i = 0 ; i < 3; i++) {
>  	 if (inst->src[i].file != UNIFORM || !inst->src[i].reladdr)
>  	    continue;
> @@ -3419,7 +3433,7 @@ vec4_visitor::move_uniform_array_access_to_pull_constants()
>  
>  	 dst_reg temp = dst_reg(this, glsl_type::vec4_type);
>  
> -	 emit_pull_constant_load(inst, temp, inst->src[i],
> +	 emit_pull_constant_load(block, inst, temp, inst->src[i],
>  				 pull_constant_loc[uniform]);
>  
>  	 inst->src[i].file = temp.file;
> -- 
> 1.8.5.5
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list