I came across some inefficient code generation by Clang while answering a different question (How do i parallelize this code using openmp with reduction)
Let's consider this simple code:
void scale(float* inout, ptrdiff_t n, ptrdiff_t m, ptrdiff_t stride, float value)
{
const float inverse = 1.f / value;
# pragma omp parallel for
for(ptrdiff_t i = 0; i < n; ++i) {
# pragma omp simd
for(ptrdiff_t j = 0; j < m; ++j)
inout[i * stride + j] *= inverse;
}
}
Where do you put the computation of the inverse and does it matter? Options that I've explored:
- Outside the loop, where it is in the example
- In the parallel section but before the loop
- In the outer loop
- in the inner loop
For GCC-11, option 1 generates the best code: One division, then a single memory load and broadcast per thread. Option 2-4 all generate basically the same code, doing the division once per thread.
Clang assembly
However, with Clang-13 the code is vastly different.
Option 1: Does a redundant memory load and broadcast in the inner loop. And it doesn't load via stack pointer but wastes a general purpose register as a pointer to the constant. If you change the code to require multiple constants, Clang will waste multiple GP registers.
Option 2: Same code pattern as GCC
Option 3: Repeats the division once per iteration of the outer loop
Option 4: Repeats the division in the inner loop
Summary
It seems as if Clang's code generation has some issues with pulling redundant computations out of OpenMP loops. Interestingly, it doesn't seem to affect the array index computation. That gets pulled out of the inner loop just fine.
If I want code that works well on both GCC and Clang, I have to write something like this:
void scale(float* inout, ptrdiff_t n, ptrdiff_t m, ptrdiff_t stride, float value)
{
# pragma omp parallel
{
const float inverse = 1.f / value;
# pragma omp for nowait
for(ptrdiff_t i = 0; i < n; ++i) {
# pragma omp simd
for(ptrdiff_t j = 0; j < m; ++j)
inout[i * stride + j] *= inverse;
}
}
}
But that is awfully verbose. This whole thing is a minor nuisance in this code example but if you check out the code in the other answer above, it gets so bad (especially with the GP register waste) that it seriously impacts performance.
So in conclusion, am I missing something? Should I write loops differently to ensure good code in both Clang and GCC?
Supplementary information
Here is a version of the code that allows easy testing and here is a Godbolt link
#include <cstddef>
// using std::ptrdiff_t
#define CONST_LOCATION 1
void scale(float* inout, std::ptrdiff_t n, std::ptrdiff_t m, std::ptrdiff_t stride,
float value)
{
# if CONST_LOCATION == 1
/*
* Clang-13.0.1: Redundant broadcast from memory in inner loop.
* Wastes GP register for pointer to constant
* GCC-11.2: Optimal
*/
const float inv = 1.f / value;
#endif
# pragma omp parallel
{
# if CONST_LOCATION == 2
/*
* Clang: Redundant computation in outer loop setup. Otherwise optimal
* GCC: Same as Clang
*/
const float inv = 1.f / value;
# endif
# pragma omp for nowait
for(std::ptrdiff_t i = 0; i < n; ++i) {
# if CONST_LOCATION == 3
/*
* Clang: Redundant computation in inner loop setup!
* GCC: Same as 2
*/
const float inv = 1.f / value;
# endif
# pragma omp simd
for(std::ptrdiff_t j = 0; j < m; ++j) {
# if CONST_LOCATION == 4
/*
* Clang: Redundant computation in inner loop!
* GCC: Same as 2
*/
const float inv = 1.f / value;
# endif
inout[i*stride + j] *= inv;
}
}
}
}
Tested with -O3 -mavx2 -mfma -fopenmp for a reasonably generic, modern compilation.