Matt Turner <[email protected]> writes: > On Sun, Mar 13, 2016 at 8:47 PM, Francisco Jerez <[email protected]> > 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.
Nope, it wasn't intentional, just got confused by having to put the return type on a separate line but only half of the time. Fixed locally. > >> + >> bool vars_interfere(int a, int b) const; >> bool vgrfs_interfere(int a, int b) const; >> int var_from_reg(const fs_reg ®) const >> --
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
