Hi,
> > +{
> > + bool progress = false;
> > +
> > + /* Find writes that are unused and can be removed. */
> > + util_dynarray_foreach_reverse(unused_writes, struct write_entry,
> > entry) {
> > + nir_deref_compare_result comp = nir_compare_derefs(dst, entry->dst);
> > + if (comp & nir_derefs_a_contains_b_bit) {
> >
>
> Mind throwing an assert in here:
>
> assert((comp & nir_derefs_equal_bit) || mask == ~(nir_component_mask_t)0);
We can assert that. We can have an entry for a copy between arrays a
and b, and see a store a[1].x that will invalidate the 'x' component
of the copy.
(...)
> > + case nir_intrinsic_copy_deref: {
> > + nir_deref_instr *src = nir_src_as_deref(intrin->src[1]);
> > + nir_deref_instr *dst = nir_src_as_deref(intrin->src[0]);
> > +
> > + /* Self-copy is removed. */
> > + if (nir_compare_derefs(src, dst) & nir_derefs_equal_bit) {
> > + nir_instr_remove(instr);
> > + progress = true;
> > + break;
> > + }
> > +
> > + uintptr_t mask = ~(1 << NIR_MAX_VEC_COMPONENTS);
> >
>
> I don't think this does quite what you want. Perhaps
>
> nir_component_mask_t mask = ~(nir_component_mask_t)0;
I'm going with
nir_component_mask_t mask = (1 << glsl_get_vector_elements(dst->type)) - 1;
The idea is that we only fill bits that are valid, so we can detect
the condition that no bits are set and remove the entry. Sounds good?
>
> All of the comments were fairly trivial and nit-picky. Assuming you're ok
> with the changes,
>
> Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
Caio
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
