On Sun, Mar 13, 2016 at 8:47 PM, Francisco Jerez <curroje...@riseup.net> wrote: > This could be improved somewhat with additional validation of the > calculated live in/out sets and by checking that the calculated live > intervals are minimal (which isn't strictly necessary to guarantee the > correctness of the program). This should be good enough though to > catch accidental use of stale liveness results due to missing or > incorrect analysis invalidation. > --- > .../drivers/dri/i965/brw_fs_live_variables.cpp | 41 > ++++++++++++++++++++++ > src/mesa/drivers/dri/i965/brw_fs_live_variables.h | 3 ++ > 2 files changed, 44 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp > b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp > index 4b0943f..215349a 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp > @@ -304,6 +304,47 @@ fs_live_variables::~fs_live_variables() > ralloc_free(mem_ctx); > } > > +static bool > +check_register_live_range(const fs_live_variables *live, int ip, > + const fs_reg ®, unsigned n) > +{ > + const unsigned var = live->var_from_reg(reg); > + > + if (var + n > unsigned(live->num_vars) || > + live->vgrf_start[reg.nr] > ip || live->vgrf_end[reg.nr] < ip) > + return false; > + > + for (unsigned j = 0; j < n; j++) { > + if (live->start[var + j] > ip || live->end[var + j] < ip) > + return false; > + } > + > + return true; > +} > + > +bool > +fs_live_variables::validate(const backend_shader *s) const > +{ > + int ip = 0; > + > + foreach_block_and_inst(block, fs_inst, inst, s->cfg) { > + for (unsigned i = 0; i < inst->sources; i++) { > + if (inst->src[i].file == VGRF && > + !check_register_live_range(this, ip, > + inst->src[i], inst->regs_read(i))) > + return false; > + } > + > + if (inst->dst.file == VGRF && > + !check_register_live_range(this, ip, inst->dst, inst->regs_written))
Looks like the indentation is slightly off on this line. > + return false; > + > + ip++; > + } > + > + return true; > +} > + > void > fs_visitor::invalidate_live_intervals() > { > diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.h > b/src/mesa/drivers/dri/i965/brw_fs_live_variables.h > index e1cd12c..c2a3c63 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.h > +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.h > @@ -69,6 +69,9 @@ public: > fs_live_variables(const backend_shader *s); > ~fs_live_variables(); > > + bool > + validate(const backend_shader *s) const; Return type on its own line -- intentional? Others, seen below, are on the same line. I'd have put it on the same line. > + > bool vars_interfere(int a, int b) const; > bool vgrfs_interfere(int a, int b) const; > int var_from_reg(const fs_reg ®) const > -- _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev