-1

I am trying to parallelize the following decoding function (from binary code to int):

int decodePrimeFactorization(int code){
  int prod = 1;
  #pragma omp parallel for
  for (int j=0; j<PF_NUMBER ; j++){
      #pragma omp critical
      {
          if ((code & 1) == 1){
              prod = prod * prime_factors[j];
          }
          code = code / 2;
      }
   } 
   return(prod);
}

Although the loop section is critical, the result is still wrong.

Any help would be appreciated.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
AbdelKh
  • 499
  • 7
  • 19
  • Instead of downvoting the question, please explain how can I improve it. – AbdelKh Oct 05 '17 at 06:37
  • 1
    I didn't down-vote, but I think putting a critical pragma that cover all lines of parallel loop is nonsense. so what should be paralleled? – Abolfazl Oct 05 '17 at 06:41
  • I agree. I started by using two atomic directives on the two instructions but that didn't work. So I tried this one (although it brings no advantage) just to check if it would give a correct result, it still didn't work! – AbdelKh Oct 05 '17 at 06:52

1 Answers1

3

The loop as written can not be made parallel as the value of your code variable is dependent on which iteration of the loop you are in. When making a loop parallel each iteration of the loop must be independent. For a value like code you need to rewrite to leave the loop external value constant and use a loop internal value that is dependent on the loop iterator j. You also should make the critical section as small as possible; here the only critical action is updating prod.

int decodePrimeFactorization(int code){
int prod = 1;
#pragma omp parallel for
  for (int j=0; j<PF_NUMBER ; j++){
    int code_tmp = (code >> j); // replaces code = code / 2
    if ((code_tmp & 1) == 1){
      #pragma omp critical
      {
        prod = prod * prime_factors[j];
      }
    }
  } 
  return(prod);
}

I use a loop internal value code_tmp to make it clearer what is required but you could also just replace the if statement with if (((code >> j) & 1) == 1)....

This is typical of many parallel loops where you have to do potentially more computation in each loop to replace values that are carried between each loop iteration in a serial version.

Justin Finnerty
  • 329
  • 1
  • 7