[PATCH] D13802: [OPENMP] Make -fopenmp to turn on OpenMP support by default.

Alexey Bataev via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 28 11:38:01 PDT 2015


ABataev added a comment.

In http://reviews.llvm.org/D13802#276755, @hfinkel wrote:

> In http://reviews.llvm.org/D13802#275471, @chandlerc wrote:
>
> > In http://reviews.llvm.org/D13802#274847, @ABataev wrote:
> >
> > > Hi Chandler, thanks for the review.
> > >
> > > In http://reviews.llvm.org/D13802#272053, @chandlerc wrote:
> > >
> > > > I've also had one test fail, and then start passing for me on Linux (after fixing the above). I haven't had it fail again, but I don't have a good way of running tests repeatedly (see below, llvm-lit doesn't yet work). It might be good to give the test suite a good 10 or 50 runs and see if anything starts failing. I'll do that overnight and report back if so.
> > >
> > >
> > > Actually, these tests are written so, that they repeats about 1000 times to be sure that they are correct. But some of them might be sensible to system load.
> >
> >
> > Hmm, so I think we need to find some way to make some of these tests more reliable.
> >
> > With the fix from http://reviews.llvm.org/D14055 patched in (minus the flag change in it) I am seeing tests fail pretty regularly:
> >
> >   libomp :: worksharing/for/omp_for_schedule_auto.c
>
>
> Credit goes to a colleague of mine, Ray Loy, for spotting this. This test has a race condition. The updates to the global sum1 are not protected by any kind of synchronization.
>
>   sum1 = sum0;
>   
>
> the for pragma is missing a private(sum1) clause?
>
> > This test seems to fail pretty frequently for me. Maybe as much as half the time.
>
> > 
>
> >   libomp :: worksharing/sections/omp_sections_nowait.c
>
>
> In omp_testsuite.h, we have this:
>
>   /* following times are in seconds */
>   #define SLEEPTIME 0.1
>   
>
> and this test, and also omp_flush.c, are sensitive to this. If the machine is heavily loaded, or otherwise configured, so the threads involved don't all get scheduled within 0.1 seconds, this will certainly fail.
>
> Also, does this test have a race condition? Specifically, should there be a
>
>   #pragma omp flush(count)
>   
>
> before
>
>   if (count == 0)
>   
>
> I think there should be.
>
> > This test fails less frequently, but still have seen it a few times.
>
> > 
>
> >   libomp :: flush/omp_flush.c
>
> >    
>
> > 
>
> > I've seen this test fail just once...
>
>
> This is trying to test the memory flush pragma by having one thread write and flush, and a second thread wait for a fixed time, flush and read. As mentioned above, this is sensitive to SLEEPTIME.


Hal, thanks for pointing this. I'll take a look and try to fix these tests.


http://reviews.llvm.org/D13802





More information about the llvm-commits mailing list