OMP | OMP is an open-source music player being developed for Linux | Audio Utils library

The non-OpenMP vectorizer is defeating your benchmark with loop inversion.
Make your function __attribute__((noinline, noclone)) to stop GCC from inlining it into the repeat loop. For cases like this with large enough functions that call/ret overhead is minor, and constant propagation isn't important, this is a pretty good way to make sure that the compiler doesn't hoist work out of the loop.

And in future, check the asm, and/or make sure the benchmark time scales linearly with the iteration count. e.g. increasing 500 up to 1000 should give the same average time in a benchmark that's working properly, but it won't with -O3. (Although it's surprisingly close here, so that smell test doesn't definitively detect the problem!)

After adding the missing #pragma omp simd to the code, yeah I can reproduce this. On i7-6700k Skylake (3.9GHz with DDR4-2666) with GCC 10.2 -O3 (without -march=native or -fopenmp), I get 18266, but with -O3 -fopenmp I get avg time 39772.

With the OpenMP vectorized version, if I look at top while it runs, memory usage (RSS) is steady at 771 MiB. (As expected: init code faults in the two inputs, and the first iteration of the timed region writes to result, triggering page-faults for it, too.)

But with the "normal" vectorizer (not OpenMP), I see the memory usage climb from ~500 MiB until it exits just as it reaches the max 770MiB.

So it looks like gcc -O3 performed some kind of loop inversion after inlining and defeated the memory-bandwidth-intensive aspect of your benchmark loop, only touching each array element once.

The asm shows the evidence: GCC 9.3 -O3 on Godbolt doesn't vectorize, and it leaves an empty inner loop instead of repeating the work.

Yes, it must be at every occurrence. Quoting from the OpenMP 5.0 standard

If a threadprivate directive that specifies a common block name appears in one program unit, then such a directive must also appear in every other program unit that contains a COMMON statement that specifies the same name. It must appear after the last such COMMON statement in the program unit.

As a comment putting OpenMP into a program full of global variables is likely to lead to a life of pain. I would at least give some thought to "do I want to start from here" before I begin such an endeavour - modernisation of the code before you add OpenMP might turn out to be an easier and cheaper option, especially in the long run.

so I don't know what to do

You have to measure.

I made only a simple for-loop to fill one array and this takes half of the time. I made two global arrays with 10 Mio floats.

For comparison:

You should not repeat parallel, you are already inside a parallel block, so you only need pragma omp for for the loop, and each thread executing the parallel block will automatically take a chunk of the loop if you specify pragma omp for. If you want to specify the number of threads you can do pragma omp parallel num_threads(4) and then pragma omp for. In any case for such a simple piece of code you can just drop the entire outer block which seems unneeded.

Here's the correct version:

As pointed out in the comments by 1201ProgramAlarm, you can get rid of the error by eliminating the if branch that exists between the two loops:

The problem with your code is that multiple threads will try to modify array2 at the same time (race condition). This can easily be avoided by reordering the loops. If array2.size doesn't provide enough parallelism, you may apply the collapse clause, as the loops are now in canonical form.

Why the 16 Threads case differs so much from the others? I'm running the algorithm on a Google VM machine with 24 Threads and 96 GB of ram.

As you have mentioned on the comments:

It's a Intel Xeon CPU @2.30 GHz, 12 physical core

That is the reason that when you moved to 16 thread you stop (almost) linearly scaling, because you are no longer just using physical cores but also logic cores (i.e., hyper-threading).

I expected that static would be the best since the iterations takes approximately the same time, while the dynamic would introduce too much overhead.

Most of the overhead of the dynamic distribution comes from the locking step performed by the threads to acquire the new iteration to work with. It just looks to me that there is not much thread locking contention going on, and even if it is, it is being compensated by better loading balancing achieved with the dynamic scheduler. I have seen this exact pattern before there is not wrong with it.

Aside note you can transform your code into:

Using std::is_constant_evaluated, you can get exactly what you want:

A naive compiler/runtime would do at each iteration:

• take a lock
• compute `4.0 / (1.0 + x*x)
• perform area += ...
• release the lock

An alternative would be not to use locks, but perform area += ... with an atomic instruction.

In both cases, this is way less efficient that using a reduction clause, in which each thread runs without any synchronization, and the reduction (possibly tree-based) only happens at the end of the OpenMP region.