On Wed, Jan 22, 2025 at 04:19:37PM +0530, Tejas Belagod wrote:
> On 1/21/25 10:16 PM, Jakub Jelinek wrote:
> > On Fri, Oct 18, 2024 at 11:52:22AM +0530, Tejas Belagod wrote:
> > > Currently poly-int type structures are passed by value to OpenMP runtime
> > > functions for shared clauses etc. This patch improves on this by passing
> > > around poly-int structures by address to avoid copy-overhead.
> > >
> > > gcc/ChangeLog
> > > * omp-low.c (use_pointer_for_field): Use pointer if the OMP data
> > > structure's field type is a poly-int.
> >
> > I think I've acked this one earlier already.
> > It is still ok.
> >
>
> Thanks Jakub for the reviews. Just to clarify - this series is all now for
> Stage 1, I'm guessing?
Not necessarily, but likely.
That said, I think the use_pointer_for_field patch can be committed
separately now, doesn't have to wait for the rest.
The general idea behind the tests would be to test something that users
could be naturally using, so for worksharing constructs test something
parallelizable, with multiple threads doing some work (of course, for the
testcase purposes it doesn't need to be some really meaningful work, just
something simple) and when testing the various data sharing or mapping of
the variable length types, it should check that they are actually handled
correctly (so for something shared see if multiple threads can read the
shared variable (and otherwise perhaps do some slightly different work
rather than the same in all threads), then perhaps in a parallel after
a #pragma omp barrier write it in one of the threads, then after another
#pragma omp barrier try to read it again in all the threads and verify each
thread sees the one written earlier; for the privatization clauses verify
that they are really private, let each thread write a different value to
them concurrently and say after a barrier verify they read what they
actually wrote, plus test the various extra properties of the privatization
clauses, say for firstprivate test reading from the value before the
parallelization that all threads read the expected value, for lastprivate
that the value from the last iteration or section has been propagated back
to the original item, etc.). In any cases the tests shouldn't have data
races. Some tests e.g. for tasks would be nice too.
Perhaps as the work for the different threads or different iterations of
say omp for or omp loop or omp distribute you could be using just helper
functions that take some SVE vectors as arguments (and perhaps the iteration
number or thread number as another so that the work is different in each)
and/or return them and e.g. for the privatization also check passing of SVE
var address to a helper function and reading the value in there.
Now, obviously somewhere in the gomp/libgomp testsuites we have tests that
test the corner cases like behavior of a parallel with a single thread or
worksharing constructs outside of an explicit parallel, but I think doing
the same for SVE isn't really needed, or at least it should be tested in
addition of tests actually testing something parallelized.
Jakub
