Hello Nicolai,
many thanks for the review.
Am Dienstag, den 01.05.2018, 12:41 +0200 schrieb Nicolai Hähnle:
> Again, I don't think having separate classes for tracking
> temporaries and tracking arrays is a good idea.
>
> They do require some semantic differences, but the only one I can
> really think of can be summarized as "there are no unconditional
> writes into arrays". If you think about it that way, it should be
> quite feasible to refactor the existing temporary tracking code so
> that it also tracks arrays, simply by skipping the unconditional
> write optimizations.
Hmm .. the existing tracking code has a class to track individual
components, and class that holds four of these components to join their
live-ranges later, keep track of the access mask, and I track read and
write operations separately, and access in if/else, and loops.
With the array the implementation of the tracking I kept way more
simple: Because of the indirect access it is essentially impossible to
establish a definite per array element read/write pattern. Hence, I
only keep track of the access mask, first and last access, and whether
there was a conditional access within a loop (there is no distinction
between read and write, but it came to me now that any first write in a
loop should always extend the live-range to the whole outer loop, so I
should change this).
In any case, I don't see how I can map this properly to the temporary
tracking code without adding more complexity to the already rather
difficult temp_comp_access::get_required_live_range() method.
We had quite some discussion about how to make this code
understandable, and for that reason I think that keeping the array live
range tracking in a separate class is not such a bad idea.
Anyway, I'll think about it, but right now I'm not convinced that
harnessing the temporary tracking for also tracking the arrays is
something that will make the code easier to understand.
Best,
Gert
>
> Cheers,
> Nicolai
>
> On 28.04.2018 21:30, Gert Wollny wrote:
> > In preparation of the array live range tracking the evaluation of
> > the read
> > mask is moved out the register live range tracking to the enclosing
> > call
> > of the reneralized read access tracking.
> >
> > Signed-off-by: Gert Wollny <gw.foss...@gmail.com>
> > ---
> > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp | 15
> > ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> > b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> > index 6e799ae731..b6e87e9a02 100644
> > --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> > @@ -494,13 +494,8 @@ void temp_access::record_write(int line,
> > prog_scope *scope, int writemask)
> > comp[3].record_write(line, scope);
> > }
> >
> > -void temp_access::record_read(int line, prog_scope *scope, int
> > swizzle)
> > +void temp_access::record_read(int line, prog_scope *scope, int
> > readmask)
> > {
> > - int readmask = 0;
> > - for (int idx = 0; idx < 4; ++idx) {
> > - int swz = GET_SWZ(swizzle, idx);
> > - readmask |= (1 << swz) & 0xF;
> > - }
> > update_access_mask(readmask);
> >
> > if (readmask & WRITEMASK_X)
> > @@ -940,8 +935,14 @@ access_recorder::~access_recorder()
> > void access_recorder::record_read(const st_src_reg& src, int
> > line,
> > prog_scope *scope)
> > {
> > + int readmask = 0;
> > + for (int idx = 0; idx < 4; ++idx) {
> > + int swz = GET_SWZ(src.swizzle, idx);
> > + readmask |= (1 << swz) & 0xF;
> > + }
> > +
> > if (src.file == PROGRAM_TEMPORARY)
> > - temp_acc[src.index].record_read(line, scope, src.swizzle);
> > + temp_acc[src.index].record_read(line, scope, readmask);
> >
> > if (src.reladdr)
> > record_read(*src.reladdr, line, scope);
> >
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
