<p dir="ltr"><br>
On May 16, 2016 6:44 PM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br>
><br>
> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
><br>
> > This allows us to disable spilling for blorp shaders since blorp state<br>
> > setup doesn't handle spilling.  Without this, blorp faisl hard if you run<br>
> > with INTEL_DEBUG=spill.<br>
> ><br>
> > Cc: Curro <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>><br>
><br>
> Reviewed-by: Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>></p>
<p dir="ltr">Did you test it?  One of is probably should. :-p</p>
<p dir="ltr">> > ---<br>
> >  src/intel/vulkan/anv_pipeline.c                   |  3 ++-<br>
> >  src/mesa/drivers/dri/i965/brw_blorp.c             |  2 +-<br>
> >  src/mesa/drivers/dri/i965/brw_compiler.h          |  1 +<br>
> >  src/mesa/drivers/dri/i965/brw_fs.cpp              | 29 +++++++++++++----------<br>
> >  src/mesa/drivers/dri/i965/brw_fs.h                |  6 ++---<br>
> >  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp |  4 ++--<br>
> >  src/mesa/drivers/dri/i965/brw_wm.c                |  3 ++-<br>
> >  7 files changed, 28 insertions(+), 20 deletions(-)<br>
> ><br>
> > diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c<br>
> > index a8e31b1..7d265d8 100644<br>
> > --- a/src/intel/vulkan/anv_pipeline.c<br>
> > +++ b/src/intel/vulkan/anv_pipeline.c<br>
> > @@ -675,7 +675,8 @@ anv_pipeline_compile_fs(struct anv_pipeline *pipeline,<br>
> >        unsigned code_size;<br>
> >        const unsigned *shader_code =<br>
> >           brw_compile_fs(compiler, NULL, mem_ctx, &key, &prog_data, nir,<br>
> > -                        NULL, -1, -1, pipeline->use_repclear, &code_size, NULL);<br>
> > +                        NULL, -1, -1, true, pipeline->use_repclear,<br>
> > +                        &code_size, NULL);<br>
> >        if (shader_code == NULL) {<br>
> >           ralloc_free(mem_ctx);<br>
> >           return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);<br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c<br>
> > index 09a0fd1..9590968 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c<br>
> > @@ -223,7 +223,7 @@ brw_blorp_compile_nir_shader(struct brw_context *brw, struct nir_shader *nir,<br>
> ><br>
> >     const unsigned *program =<br>
> >        brw_compile_fs(compiler, brw, mem_ctx, wm_key, &wm_prog_data, nir,<br>
> > -                     NULL, -1, -1, use_repclear, program_size, NULL);<br>
> > +                     NULL, -1, -1, false, use_repclear, program_size, NULL);<br>
> ><br>
> >     /* Copy the relavent bits of wm_prog_data over into the blorp prog data */<br>
> >     prog_data->dispatch_8 = wm_prog_data.dispatch_8;<br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h b/src/mesa/drivers/dri/i965/brw_compiler.h<br>
> > index a2148ae..15f32be 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_compiler.h<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_compiler.h<br>
> > @@ -789,6 +789,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,<br>
> >                 struct gl_program *prog,<br>
> >                 int shader_time_index8,<br>
> >                 int shader_time_index16,<br>
> > +               bool allow_spilling,<br>
> >                 bool use_rep_send,<br>
> >                 unsigned *final_assembly_size,<br>
> >                 char **error_str);<br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
> > index 2de5533..55d10fc 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
> > @@ -5462,7 +5462,7 @@ fs_visitor::fixup_3src_null_dest()<br>
> >  }<br>
> ><br>
> >  void<br>
> > -fs_visitor::allocate_registers()<br>
> > +fs_visitor::allocate_registers(bool allow_spilling)<br>
> >  {<br>
> >     bool allocated_without_spills;<br>
> ><br>
> > @@ -5472,6 +5472,8 @@ fs_visitor::allocate_registers()<br>
> >        SCHEDULE_PRE_LIFO,<br>
> >     };<br>
> ><br>
> > +   bool spill_all = allow_spilling && (INTEL_DEBUG & DEBUG_SPILL_FS);<br>
> > +<br>
> >     /* Try each scheduling heuristic to see if it can successfully register<br>
> >      * allocate without spilling.  They should be ordered by decreasing<br>
> >      * performance but increasing likelihood of allocating.<br>
> > @@ -5483,7 +5485,7 @@ fs_visitor::allocate_registers()<br>
> >           assign_regs_trivial();<br>
> >           allocated_without_spills = true;<br>
> >        } else {<br>
> > -         allocated_without_spills = assign_regs(false);<br>
> > +         allocated_without_spills = assign_regs(false, spill_all);<br>
> >        }<br>
> >        if (allocated_without_spills)<br>
> >           break;<br>
> > @@ -5508,12 +5510,14 @@ fs_visitor::allocate_registers()<br>
> >        /* Since we're out of heuristics, just go spill registers until we<br>
> >         * get an allocation.<br>
> >         */<br>
> > -      while (!assign_regs(true)) {<br>
> > +      while (!assign_regs(true, spill_all)) {<br>
> >           if (failed)<br>
> >              break;<br>
> >        }<br>
> >     }<br>
> ><br>
> > +   assert(last_scratch == 0 || allow_spilling);<br>
> > +<br>
> >     /* This must come after all optimization and register allocation, since<br>
> >      * it inserts dead code that happens to have side effects, and it does<br>
> >      * so based on the actual physical registers in use.<br>
> > @@ -5559,7 +5563,7 @@ fs_visitor::run_vs(gl_clip_plane *clip_planes)<br>
> >     assign_vs_urb_setup();<br>
> ><br>
> >     fixup_3src_null_dest();<br>
> > -   allocate_registers();<br>
> > +   allocate_registers(true);<br>
> ><br>
> >     return !failed;<br>
> >  }<br>
> > @@ -5641,7 +5645,7 @@ fs_visitor::run_tcs_single_patch()<br>
> >     assign_tcs_single_patch_urb_setup();<br>
> ><br>
> >     fixup_3src_null_dest();<br>
> > -   allocate_registers();<br>
> > +   allocate_registers(true);<br>
> ><br>
> >     return !failed;<br>
> >  }<br>
> > @@ -5675,7 +5679,7 @@ fs_visitor::run_tes()<br>
> >     assign_tes_urb_setup();<br>
> ><br>
> >     fixup_3src_null_dest();<br>
> > -   allocate_registers();<br>
> > +   allocate_registers(true);<br>
> ><br>
> >     return !failed;<br>
> >  }<br>
> > @@ -5724,13 +5728,13 @@ fs_visitor::run_gs()<br>
> >     assign_gs_urb_setup();<br>
> ><br>
> >     fixup_3src_null_dest();<br>
> > -   allocate_registers();<br>
> > +   allocate_registers(true);<br>
> ><br>
> >     return !failed;<br>
> >  }<br>
> ><br>
> >  bool<br>
> > -fs_visitor::run_fs(bool do_rep_send)<br>
> > +fs_visitor::run_fs(bool allow_spilling, bool do_rep_send)<br>
> >  {<br>
> >     brw_wm_prog_data *wm_prog_data = (brw_wm_prog_data *) this->prog_data;<br>
> >     brw_wm_prog_key *wm_key = (brw_wm_prog_key *) this->key;<br>
> > @@ -5794,7 +5798,7 @@ fs_visitor::run_fs(bool do_rep_send)<br>
> >        assign_urb_setup();<br>
> ><br>
> >        fixup_3src_null_dest();<br>
> > -      allocate_registers();<br>
> > +      allocate_registers(allow_spilling);<br>
> ><br>
> >        if (failed)<br>
> >           return false;<br>
> > @@ -5837,7 +5841,7 @@ fs_visitor::run_cs()<br>
> >     assign_curb_setup();<br>
> ><br>
> >     fixup_3src_null_dest();<br>
> > -   allocate_registers();<br>
> > +   allocate_registers(true);<br>
> ><br>
> >     if (failed)<br>
> >        return false;<br>
> > @@ -5962,6 +5966,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,<br>
> >                 const nir_shader *src_shader,<br>
> >                 struct gl_program *prog,<br>
> >                 int shader_time_index8, int shader_time_index16,<br>
> > +               bool allow_spilling,<br>
> >                 bool use_rep_send,<br>
> >                 unsigned *final_assembly_size,<br>
> >                 char **error_str)<br>
> > @@ -6005,7 +6010,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,<br>
> >     fs_visitor v8(compiler, log_data, mem_ctx, key,<br>
> >                   &prog_data->base, prog, shader, 8,<br>
> >                   shader_time_index8);<br>
> > -   if (!v8.run_fs(false /* do_rep_send */)) {<br>
> > +   if (!v8.run_fs(allow_spilling, false /* do_rep_send */)) {<br>
> >        if (error_str)<br>
> >           *error_str = ralloc_strdup(mem_ctx, v8.fail_msg);<br>
> ><br>
> > @@ -6023,7 +6028,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,<br>
> >                       &prog_data->base, prog, shader, 16,<br>
> >                       shader_time_index16);<br>
> >        v16.import_uniforms(&v8);<br>
> > -      if (!v16.run_fs(use_rep_send)) {<br>
> > +      if (!v16.run_fs(allow_spilling, use_rep_send)) {<br>
> >           compiler->shader_perf_log(log_data,<br>
> >                                     "SIMD16 shader failed to compile: %s",<br>
> >                                     v16.fail_msg);<br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h<br>
> > index d4eb8fb..b6fe564 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_fs.h<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.h<br>
> > @@ -105,14 +105,14 @@ public:<br>
> >                                     uint32_t const_offset);<br>
> >     void DEP_RESOLVE_MOV(const brw::fs_builder &bld, int grf);<br>
> ><br>
> > -   bool run_fs(bool do_rep_send);<br>
> > +   bool run_fs(bool allow_spilling, bool do_rep_send);<br>
> >     bool run_vs(gl_clip_plane *clip_planes);<br>
> >     bool run_tcs_single_patch();<br>
> >     bool run_tes();<br>
> >     bool run_gs();<br>
> >     bool run_cs();<br>
> >     void optimize();<br>
> > -   void allocate_registers();<br>
> > +   void allocate_registers(bool allow_spilling);<br>
> >     void setup_fs_payload_gen4();<br>
> >     void setup_fs_payload_gen6();<br>
> >     void setup_vs_payload();<br>
> > @@ -127,7 +127,7 @@ public:<br>
> >     void assign_tcs_single_patch_urb_setup();<br>
> >     void assign_tes_urb_setup();<br>
> >     void assign_gs_urb_setup();<br>
> > -   bool assign_regs(bool allow_spilling);<br>
> > +   bool assign_regs(bool allow_spilling, bool spill_all);<br>
> >     void assign_regs_trivial();<br>
> >     void calculate_payload_ranges(int payload_node_count,<br>
> >                                   int *payload_last_use_ip);<br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp<br>
> > index 2347cd5..e65f73b 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp<br>
> > @@ -542,7 +542,7 @@ setup_mrf_hack_interference(fs_visitor *v, struct ra_graph *g,<br>
> >  }<br>
> ><br>
> >  bool<br>
> > -fs_visitor::assign_regs(bool allow_spilling)<br>
> > +fs_visitor::assign_regs(bool allow_spilling, bool spill_all)<br>
> >  {<br>
> >     /* Most of this allocation was written for a reg_width of 1<br>
> >      * (dispatch_width == 8).  In extending to SIMD16, the code was<br>
> > @@ -668,7 +668,7 @@ fs_visitor::assign_regs(bool allow_spilling)<br>
> >     }<br>
> ><br>
> >     /* Debug of register spilling: Go spill everything. */<br>
> > -   if (unlikely(INTEL_DEBUG & DEBUG_SPILL_FS)) {<br>
> > +   if (unlikely(spill_all)) {<br>
> >        int reg = choose_spill_reg(g);<br>
> ><br>
> >        if (reg != -1) {<br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c<br>
> > index 395b0b8..8af1ff7 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_wm.c<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_wm.c<br>
> > @@ -137,7 +137,8 @@ brw_codegen_wm_prog(struct brw_context *brw,<br>
> >     program = brw_compile_fs(brw->intelScreen->compiler, brw, mem_ctx,<br>
> >                              key, &prog_data, fp->program.Base.nir,<br>
> >                              &fp->program.Base, st_index8, st_index16,<br>
> > -                            brw->use_rep_send, &program_size, &error_str);<br>
> > +                            true, brw->use_rep_send,<br>
> > +                            &program_size, &error_str);<br>
> >     if (program == NULL) {<br>
> >        if (prog) {<br>
> >           prog->LinkStatus = false;<br>
> > --<br>
> > 2.5.0.400.gff86faf<br>
</p>