[Mesa-dev] [PATCH 02/20] i965/fs: Preserve the CFG in a few more places.

Pohjolainen, Topi topi.pohjolainen at intel.com
Fri Sep 5 01:02:19 PDT 2014


On Tue, Sep 02, 2014 at 09:34:13PM -0700, Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 30 +++++++++++++++++-------------
>  src/mesa/drivers/dri/i965/brw_fs.h   |  6 ++++--
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 268f528..5c4ecb4 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2126,7 +2126,7 @@ fs_visitor::opt_register_renaming()
>     }
>  
>     if (progress) {
> -      invalidate_live_intervals();
> +      invalidate_live_intervals(false);

Checked that here we don't remove or add instructions, and hence this
should be safe.

>  
>        for (unsigned i = 0; i < ARRAY_SIZE(delta_x); i++) {
>           if (delta_x[i].file == GRF && remap[delta_x[i].reg] != -1) {
> @@ -2465,7 +2465,7 @@ fs_visitor::remove_duplicate_mrf_writes()
>     }
>  
>     if (progress)
> -      invalidate_live_intervals();
> +      invalidate_live_intervals(false);

Here in turn the logic is using block-aware insertions and removals.

>  
>     return progress;
>  }
> @@ -2516,7 +2516,8 @@ clear_deps_for_inst_src(fs_inst *inst, int dispatch_width, bool *deps,
>   *      same time that both consider ???r3??? as the target of their final writes.
>   */
>  void
> -fs_visitor::insert_gen4_pre_send_dependency_workarounds(fs_inst *inst)
> +fs_visitor::insert_gen4_pre_send_dependency_workarounds(bblock_t *block,
> +                                                        fs_inst *inst)
>  {
>     int reg_size = dispatch_width / 8;
>     int write_len = inst->regs_written * reg_size;
> @@ -2545,7 +2546,7 @@ fs_visitor::insert_gen4_pre_send_dependency_workarounds(fs_inst *inst)
>        if (scan_inst->is_control_flow()) {
>           for (int i = 0; i < write_len; i++) {
>              if (needs_dep[i]) {
> -               inst->insert_before(DEP_RESOLVE_MOV(first_write_grf + i));
> +               inst->insert_before(block, DEP_RESOLVE_MOV(first_write_grf + i));
>              }
>           }
>           return;
> @@ -2566,7 +2567,7 @@ fs_visitor::insert_gen4_pre_send_dependency_workarounds(fs_inst *inst)
>              if (reg >= first_write_grf &&
>                  reg < first_write_grf + write_len &&
>                  needs_dep[reg - first_write_grf]) {
> -               inst->insert_before(DEP_RESOLVE_MOV(reg));
> +               inst->insert_before(block, DEP_RESOLVE_MOV(reg));
>                 needs_dep[reg - first_write_grf] = false;
>                 if (scan_inst_simd16)
>                    needs_dep[reg - first_write_grf + 1] = false;
> @@ -2597,7 +2598,7 @@ fs_visitor::insert_gen4_pre_send_dependency_workarounds(fs_inst *inst)
>   *      instruction with a different destination register.
>   */
>  void
> -fs_visitor::insert_gen4_post_send_dependency_workarounds(fs_inst *inst)
> +fs_visitor::insert_gen4_post_send_dependency_workarounds(bblock_t *block, fs_inst *inst)
>  {
>     int write_len = inst->regs_written * dispatch_width / 8;
>     int first_write_grf = inst->dst.reg;
> @@ -2616,7 +2617,8 @@ fs_visitor::insert_gen4_post_send_dependency_workarounds(fs_inst *inst)
>        if (scan_inst->is_control_flow()) {
>           for (int i = 0; i < write_len; i++) {
>              if (needs_dep[i])
> -               scan_inst->insert_before(DEP_RESOLVE_MOV(first_write_grf + i));
> +               scan_inst->insert_before(block,
> +                                        DEP_RESOLVE_MOV(first_write_grf + i));
>           }
>           return;
>        }
> @@ -2632,7 +2634,7 @@ fs_visitor::insert_gen4_post_send_dependency_workarounds(fs_inst *inst)
>            scan_inst->dst.reg >= first_write_grf &&
>            scan_inst->dst.reg < first_write_grf + write_len &&
>            needs_dep[scan_inst->dst.reg - first_write_grf]) {
> -         scan_inst->insert_before(DEP_RESOLVE_MOV(scan_inst->dst.reg));
> +         scan_inst->insert_before(block, DEP_RESOLVE_MOV(scan_inst->dst.reg));
>           needs_dep[scan_inst->dst.reg - first_write_grf] = false;
>        }
>  
> @@ -2653,7 +2655,7 @@ fs_visitor::insert_gen4_post_send_dependency_workarounds(fs_inst *inst)
>     assert(last_inst->eot);
>     for (int i = 0; i < write_len; i++) {
>        if (needs_dep[i])
> -         last_inst->insert_before(DEP_RESOLVE_MOV(first_write_grf + i));
> +         last_inst->insert_before(block, DEP_RESOLVE_MOV(first_write_grf + i));
>     }
>  }
>  
> @@ -2669,16 +2671,18 @@ fs_visitor::insert_gen4_send_dependency_workarounds()
>      * have a .reg_offset of 0.
>      */
>  
> -   foreach_in_list_safe(fs_inst, inst, &instructions) {
> +   calculate_cfg();
> +
> +   foreach_block_and_inst(block, fs_inst, inst, cfg) {
>        if (inst->mlen != 0 && inst->dst.file == GRF) {
> -         insert_gen4_pre_send_dependency_workarounds(inst);
> -         insert_gen4_post_send_dependency_workarounds(inst);
> +         insert_gen4_pre_send_dependency_workarounds(block, inst);
> +         insert_gen4_post_send_dependency_workarounds(block, inst);
>           progress = true;
>        }
>     }
>  
>     if (progress)
> -      invalidate_live_intervals();
> +      invalidate_live_intervals(false);

And now that insertions are block-aware, this should be safe as well:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

>  }
>  
>  /**
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 02435b7..d7d7c3b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -347,8 +347,10 @@ public:
>     bool virtual_grf_interferes(int a, int b);
>     void schedule_instructions(instruction_scheduler_mode mode);
>     void insert_gen4_send_dependency_workarounds();
> -   void insert_gen4_pre_send_dependency_workarounds(fs_inst *inst);
> -   void insert_gen4_post_send_dependency_workarounds(fs_inst *inst);
> +   void insert_gen4_pre_send_dependency_workarounds(bblock_t *block,
> +                                                    fs_inst *inst);
> +   void insert_gen4_post_send_dependency_workarounds(bblock_t *block,
> +                                                     fs_inst *inst);
>     void vfail(const char *msg, va_list args);
>     void fail(const char *msg, ...);
>     void no16(const char *msg, ...);
> -- 
> 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