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

hfinkel@anl.gov via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 27 15:33:29 PDT 2015


hfinkel added a comment.

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.


http://reviews.llvm.org/D13802





More information about the cfe-commits mailing list