0

The following code will behave differently, depending on the optimization applied by gcc and on the target architecture:

#include <omp.h>
#include <stdlib.h>
#include <stdio.h>
#include <time.h>

static void malloc_testvals(int **testvals, int num_tests, int num_threads) {
  for (int i = 0; i < num_tests; i++) {
    testvals[i] = malloc(num_threads * sizeof(int));
  }
}

static void free_testvals(int **testvals, int num_tests) {
  for (int i = 0; i < num_tests; i++) {
    free(testvals[i]);
  }
}

static void make_random_testvals(int **testvals, int *sums, int num_tests, int num_threads) {
  srand(time(NULL));
  for (int i = 0; i < num_tests; i++) {
    sums[i] = 0;
    for (int j = 0; j < num_threads; j++) {
      testvals[i][j] = -100 + rand() % 201;
      sums[i] += testvals[i][j];
    }
  }
}

typedef struct ThreadCommunicator_s ThreadCommunicator;

typedef struct {
  long num_threads;
  ThreadCommunicator **threads;
} Communicator;


typedef struct ThreadCommunicator_s {
  Communicator *parent;
  long omp_longval;
} ThreadCommunicator;


static void ThreadCommunicator_init(ThreadCommunicator* self, Communicator* parent) {
  *self = (ThreadCommunicator) {
    .parent = parent,
    .omp_longval = 0
  };
}

static void Communicator_init(Communicator* self) {
  self->num_threads = omp_get_max_threads();
  self->threads = malloc(sizeof(ThreadCommunicator *) * self->num_threads);
  
  for (int rank = 0; rank < self->num_threads; rank++) {
    self->threads[rank] = malloc(sizeof(ThreadCommunicator));
    ThreadCommunicator_init(self->threads[rank], self);
  }
}

static void Communicator_deinit(Communicator* self) {
  for (int rank = 0; rank < self->num_threads; rank++) {
    free(self->threads[rank]);
  }
  free(self->threads);
}

//Sums over all thread-inherent numbers
static long ThreadCommunicator_allreduce_sum_l(ThreadCommunicator* self, long myval) {
  //share my result with others
  self->omp_longval = myval;
  #pragma omp barrier

  #pragma omp single
  {
    printf("self->parent->num_threads = %ld\n", self->parent->num_threads);
    printf("omp_get_num_threads()     = %d\n", omp_get_num_threads());
  }
  
  //------------------------------------------------------------------------------------------------------------------
  //Error will be miraculously gone if self->parent->num_threads is replaced by omp_get_num_threads().
  //------------------------------------------------------------------------------------------------------------------
  long sum = 0;
  for (int rank = 0; rank < self->parent->num_threads; rank++) {
    sum += self->parent->threads[rank]->omp_longval;
  }
  #pragma omp barrier

  return sum;
}

#define NUM_TESTS 1

int main() {
  Communicator communicator;
  Communicator_init(&communicator);

  int *testvals[NUM_TESTS];
  //solutions
  int sums[NUM_TESTS];
  malloc_testvals(testvals, NUM_TESTS, communicator.num_threads);

  make_random_testvals(testvals, sums, NUM_TESTS, communicator.num_threads);

  unsigned long error = 0;
  #pragma omp parallel
  {
    if (communicator.num_threads != omp_get_num_threads()) {
      printf("This is not supported in this test.\n");
      //due to make_random_testvals works with communicator.num_threads
      //but ThreadCommunicator_allreduce_sum_l does set only the first omp_get_num_threads() values of it
      abort();
    }

    ThreadCommunicator *thread_comm = communicator.threads[omp_get_thread_num()];

    for (int i = 0; i < NUM_TESTS; i++) {
      long thread_sum = ThreadCommunicator_allreduce_sum_l(thread_comm, testvals[i][omp_get_thread_num()]);
      #pragma omp atomic
        error += (unsigned long) labs(thread_sum - sums[i]);
    }
  }

  if (error != 0) {
    printf("Error occurred (error = %lu)!\n", error);
  }

  free_testvals(testvals, NUM_TESTS);

  Communicator_deinit(&communicator);
}

Compiling by

gcc -Wall -std=c99 -fopenmp -O3 -march=skylake-avx512

or

gcc -Wall -std=c99 -fopenmp -O3 -march=native

on

Intel(R) Xeon(R) Gold 6230 CPU and with gcc (GCC) 8.3.1 20191121 (Red Hat 8.3.1-5)

will exemplarily produce this output:

self->parent->num_threads = 16
omp_get_num_threads()     = 16
Error occurred (error = 8070309797393041808)!

Interestingly, this error goes away if one of the following changes are applied:

  • Replace -O3 by -O2
  • Remove -march=... from the list of options
  • Replace self->parent->num_threads by omp_get_num_threads() as indicated in the code.
  • Compile on a different machine with -march=native (although I have of course no complete overview which systems are affected and which not)

I am asking whether this is a compiler bug or whether my code is not compliant with either C or the OpenMP specification, e.g. due to a data race. Any help is highly appreciated!


EDIT: Updated code according to comment (@Laci).

questioner
  • 158
  • 1
  • 9
  • You have 3 parallel regions, but you check only once if you get all the available threads. You should also check in `Communicator_init` and `Communicator_deinit`. This strange behavior suggests uninitialized variables. – Laci Dec 01 '21 at 16:09
  • Note also that `omp_get_max_threads()` can return different value inside and outside of a parallel region. From OpenMP specification: "The `omp_get_max_threads` routine returns an upper bound on the number of threads that could be used to form a new team if a parallel construct without a `num_threads` clause were encountered after execution returns from this routine. " – Laci Dec 01 '21 at 16:18
  • Thanks for that comment. I totally agree, that the code above is not 100% rigorous in that regard. I did another check to confirm if this caused the issue, but even if I remove the two additional parallel regions (for the init and deinit), and write it instead with sequential loops, the problem remains the same. And I also replaced the omp_get_max_thread() by the communicator.num_threads value in order to keep it consistent allthrough. Do you want the code to be updated in the question accordingly? – questioner Dec 01 '21 at 19:30
  • Possibly I am a victim of https://stackoverflow.com/questions/58026153/why-does-march-native-corrupt-my-program. To be checked... – questioner Dec 01 '21 at 20:35
  • Yes, probably it is worth to update your code. Is the problem exists if number of threads is set to 1? The problem may be independent of OpenMP... – Laci Dec 01 '21 at 20:48
  • I made the code update. It seemed to be an OpenMP issue, because the problem only exists with OMP_NUM_THREADS >= 8, maybe I should've mentioned this from the start. It becomes more and more clear, that this is likely an automatic vectorization issue (as suggested by the previously cited question), as 8 longs typically fit into an AVX512 register. Therefore, I suppose in the case when OMP_NUM_THREADS < 8 the vectorized part of the loop will not be executed and hence the issue is not showing up. – questioner Dec 01 '21 at 21:00

1 Answers1

0

As it turned out, the problem went away after updating binutils. Hence the issue is considered a compiler bug.

questioner
  • 158
  • 1
  • 9