[Mesa-dev] [PATCH] i965/fs: Take into account lower frequency of conditional blocks in spilling cost heuristic.

[Kenneth Graunke]

On Monday, April 10, 2017 5:23:20 PM PDT Francisco Jerez wrote:
> The individual branches of an if/else/endif construct will be executed
> some unknown number of times between 0 and 1 relative to the parent
> block.  Use some factor in between as weight while approximating the
> cost of spill/fill instructions within a conditional if-else branch.
> This favors spilling registers used within conditional branches which
> are likely to be executed less frequently than registers used at the
> top level.
> 
> Improves the framerate of the SynMark2 OglCSDof benchmark by ~1.9x on
> my SKL GT4e.  Should have a comparable effect on other platforms.  No
> significant regressions.
> ---
>  src/intel/compiler/brw_fs_reg_allocate.cpp | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp b/src/intel/compiler/brw_fs_reg_allocate.cpp
> index 5c6f3d4..c981d72 100644
> --- a/src/intel/compiler/brw_fs_reg_allocate.cpp
> +++ b/src/intel/compiler/brw_fs_reg_allocate.cpp
> @@ -806,7 +806,7 @@ emit_spill(const fs_builder &bld, fs_reg src,
>  int
>  fs_visitor::choose_spill_reg(struct ra_graph *g)
>  {
> -   float loop_scale = 1.0;
> +   float block_scale = 1.0;
>     float spill_costs[this->alloc.count];
>     bool no_spill[this->alloc.count];
>  
> @@ -822,23 +822,32 @@ fs_visitor::choose_spill_reg(struct ra_graph *g)
>     foreach_block_and_inst(block, fs_inst, inst, cfg) {
>        for (unsigned int i = 0; i < inst->sources; i++) {
>  	 if (inst->src[i].file == VGRF)
> -            spill_costs[inst->src[i].nr] += loop_scale;
> +            spill_costs[inst->src[i].nr] += block_scale;
>        }
>  
>        if (inst->dst.file == VGRF)
>           spill_costs[inst->dst.nr] += DIV_ROUND_UP(inst->size_written, REG_SIZE)
> -                                      * loop_scale;
> +                                      * block_scale;
>  
>        switch (inst->opcode) {
>  
>        case BRW_OPCODE_DO:
> -	 loop_scale *= 10;
> +	 block_scale *= 10;
>  	 break;
>  
>        case BRW_OPCODE_WHILE:
> -	 loop_scale /= 10;
> +	 block_scale /= 10;
>  	 break;
>  
> +      case BRW_OPCODE_IF:
> +      case BRW_OPCODE_IFF:
> +         block_scale *= 0.5;
> +         break;
> +
> +      case BRW_OPCODE_ENDIF:
> +         block_scale /= 0.5;
> +         break;
> +
>        case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
>  	 if (inst->src[0].file == VGRF)
>              no_spill[inst->src[0].nr] = true;
> 

Makes sense, nice simple improvement!

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170410/a8093375/attachment.sig>