On Fri, 17 Sep 2021 16:26:50 +0200
Thomas Schwinge <tho...@codesourcery.com> wrote:
> > @@ -1449,8 +1634,120 @@ oacc_do_neutering (void)
>
> > + addr_range ar
> > + = first_fit_range (conflicts, size, align,
> > &worker_shm_bounds); +
> > + splay_tree_delete (conflicts);
> > +
> > + if (ar.invalid ())
> > + {
> > + unsigned HOST_WIDE_INT base;
> > + base = bounds_lo + random () % 512;
> > + base = (base + align - 1) & ~(align - 1);
> > + if (base + size > bounds_hi)
> > + error_at (UNKNOWN_LOCATION, "shared-memory region
> > overflow");
>
> My dice doesn't have 512 faces -- what am I to read into the
> expression 'random () % 512' here? (Surely this must offend the
> folks of <https://reproducible-builds.org/>.) ;-)
For this one -- the randomness shouldn't be necessary for the
correctness of the patch (i.e. it could just be "base = bounds_lo", or
indeed folded into the line after).
The "ar.invalid ()" case happens when we fail to allocate a block of
memory in LDS space for broadcasting a particular set of variables,
and trigger a fall-back path in the broadcasting code that adds extra
barriers around the broadcast in question. I imagine I was thinking
that adding randomness could mean we can "get lucky" sometimes and
avoid needing those barriers in some cases, but in fact I don't think
that was implemented, so the randomness is useless. (Or it could just
have been leftover debug code... oops).
Thanks,
Julian
