After doing about 14 corrections, I reran the Inspection and still had 25 data races. I also knew that adding the pragma was not the best solution - but I was interested in getting something correct, but not necessarily optimal. Having lost the will to live, I ditched this attempt and started to look at other options.

Strategies for fixing  legacy problem

Fundamental problem of the code are two fold. Firstly  there are a lot of globals, and secondly some of the local variables in main() are being shared across the threads.  Sharing of data, whether global or local across threads will produce data races, unless there is a suitable synchronisation mechanism in place.  In the previous section I tried to enforce some protection by adding #pragma omp critical statements, which has the effect of serialising access to the block of code below the pragma. As you already know, I didn't pursue this strategy for very long.

The style of  the code reflects the programming standards of the day. It's not uncommon when parallelising legacy code that there needs to be some refactoring of the code.     

Strategies

Strategies to correct the data-races could include

• Use synchronisation to protect the objects
• Move the globals so they are local variable on the stack
• Use OpenMP data sharing constructs.
• Place globals \ locals  within the parallel region
• Use OpenMP private\etc  to localise variables

Goals

My overall goal is to

    1. make minimum changes to the code
    2. make sure code can still run in serial mode
    3. make code so it is scalable
    4. make the code run on all cores.

Any changes I make to the code should not break these goals.

Code Changes in a nutshell

A copy of the modified files are listed at the end of this blog.

Globals

To work around the globals issue, I

1. Created a structure to hold the globals:

    typedef struct globs
    {
        Rec_Pointer Ptr,
        Next_Ptr;
        int Int;
        Boolean Bool;
        char Ch_1,
        Ch_2;
        int Arr_1 [50];
        int Arr_2 [50] [50];
     } GLOBALS;

    extern GLOBALS *pGlob_Loc;
 

2. Malloced  memory for structures based on max threads

    pGlob_Loc = (GLOBALS *)malloc(sizeof(GLOBALS) * omp_get_max_threads());

3. Added a define that uses the ThreadID
 

    #define Ptr_Glob pGlob_Loc[ThreadID].Ptr
    #define Next_Ptr_Glob pGlob_Loc[ThreadID].Next_Ptr
    #define Int_Glob pGlob_Loc[ThreadID].Int
    #define Bool_Glob pGlob_Loc[ThreadID].Bool
    #define Ch_1_Glob pGlob_Loc[ThreadID].Ch_1
    #define Ch_2_Glob pGlob_Loc[ThreadID].Ch_2
    #define Arr_1_Glob pGlob_Loc[ThreadID].Arr_1
    #define Arr_2_Glob pGlob_Loc[ThreadID].Arr_2

5. Passed ThreadID into sub functions

Locals

To fix the shared local symbols I split the #pragma omp parallel for, so the parallel region started before the locals were declared:

    main (int argc, char * argv[])
    /*****/
    /* main program, corresponds to procedures */
    /* Main and Proc_0 in the Ada version */
    {
        pGlob_Loc = (GLOBALS *)malloc(sizeof(GLOBALS) * omp_get_max_threads());
        #pragma omp parallel
        {
             One_Fifty Int_1_Loc;
            REG One_Fifty Int_2_Loc;
            One_Fifty Int_3_Loc;

            ...

        #pragma omp for
        for (Run_Index = 1; Run_Index <= Number_Of_Runs; ++Run_Index)

Having made the changes to the code, yippee I only end up with 3 data races. All of these are associated with the calculation of time.

Time Measurements

What I'd like to do is make sure time just measured on one thread. To do this I enclosed all the time-related code into a pragma omp master. 

e.g.

#pragma omp master
{
    #ifdef TIMES
    times (&time_info);
    Begin_Time = (long) time_info.tms_utime;
    #endif
    #ifdef TIME
   Begin_Time = time ( (long *) 0);
    #endif
}

I did the same with two other blocks of code that are concerned with time,

Once I did this, the Problems P1 to P3 disappeared.

Final Check with Parallel Lint

I re-ran Parallel Lint and got 7 warnings about possible problems with access to pGlob_Loc, and 67 errors about an I/O problem.

I'm not unduly worried by the pGlob_Loc. However, the I/O issue is due to the fact that printf may be  non-re-entrant. I'll continue my explorations in the next blog

Conclusions (so far)

I hope I've shown you that doing both static and dynamic analysis of a parallel application is worthwhile. Using tools such as Intel Parallel Inspector and Intel Parallel Lint can help boost the confidence you have in your parallel code.

There's still more to do on this code - and I have purposely ignored some errors that will need correcting. , At least now I have an application that is going in the right direction. 

Modified Files

A copy of the modified files can be found here: 
    dhry_correctness.h
    dhry_1_correctness.c
    dhry_2_correctness.c