Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Compilation failure with icpx 2024.0.1 and OpenMP backend #2205

Closed
bernhardmgruber opened this issue Dec 16, 2023 · 6 comments · Fixed by #2213 · May be fixed by #2209
Closed

Compilation failure with icpx 2024.0.1 and OpenMP backend #2205

bernhardmgruber opened this issue Dec 16, 2023 · 6 comments · Fixed by #2213 · May be fixed by #2209
Labels
Type:Bug

Comments

@bernhardmgruber
Copy link
Member

alpaka 1.0 and also the develop branch fail to compile with icpx 2024.0.1 when the OpenMP backend is turned on:

CXX=/opt/intel/oneapi/compiler/2024.0/bin/icpx cmake -Dalpaka_ACC_CPU_B_SEQ_T_OMP2_ENABLE=ON -DBUILD_TESTING=ON ..
make accTest

Error:

/home/bgruber/dev/alpaka/include/alpaka/atomic/AtomicOmpBuiltIn.hpp:191:25: error: the statement for 'atomic compare capture' must be '{v=x; cond-update-stmt}', '{cond-update-stmt v = x;}', 'if(x==e) {x=d;} else {v=x;}', '{r=x==e; if(r) {x=d;}}' or '{r=x==e; if(r) {x=d;} else {v=x;}}' where x, r, and v are lvalue expressions with scalar type
  191 |                         ref = value;
      |                         ^~~
/home/bgruber/dev/alpaka/include/alpaka/atomic/AtomicOmpBuiltIn.hpp:191:25: note: expected compound statement containing one assignment
  191 |                         ref = value;
      |                         ^~~
/home/bgruber/dev/alpaka/include/alpaka/atomic/AtomicOmpBuiltIn.hpp:210:25: error: the statement for 'atomic compare capture' must be '{v=x; cond-update-stmt}', '{cond-update-stmt v = x;}', 'if(x==e) {x=d;} else {v=x;}', '{r=x==e; if(r) {x=d;}}' or '{r=x==e; if(r) {x=d;} else {v=x;}}' where x, r, and v are lvalue expressions with scalar type
  210 |                         ref = value;
      |                         ^~~
/home/bgruber/dev/alpaka/include/alpaka/atomic/AtomicOmpBuiltIn.hpp:210:25: note: expected compound statement containing one assignment
  210 |                         ref = value;
      |                         ^~~
/home/bgruber/dev/alpaka/include/alpaka/atomic/AtomicOmpBuiltIn.hpp:263:21: error: the statement for 'atomic compare capture' must be '{v=x; cond-update-stmt}', '{cond-update-stmt v = x;}', 'if(x==e) {x=d;} else {v=x;}', '{r=x==e; if(r) {x=d;}}' or '{r=x==e; if(r) {x=d;} else {v=x;}}' where x, r, and v are lvalue expressions with scalar type
  263 |                     old = ref;
      |                     ^~~
/home/bgruber/dev/alpaka/include/alpaka/atomic/AtomicOmpBuiltIn.hpp:263:21: note: expected if statement
  263 |                     old = ref;
      |                     ^~~
3 errors generated.
@psychocoderHPC psychocoderHPC mentioned this issue Dec 20, 2023
Draft
@psychocoderHPC
Copy link
Member

@bernhardmgruber Do you changed something else in alpaka to get ipcx start to compile OpenMP code?

My short test to reproduce the issue in the CI in #2209 provides the following error

# error If ALPAKA_ACC_CPU_B_OMP2_T_SEQ_ENABLED is set, the compiler has to support OpenMP 2.0 or higher!

@bernhardmgruber
Copy link
Member Author

@psychocoderHPC, nothing else should be needed. I just retried it locally and it shows the same error, starting with a clean build directory and using the command I provided.

Your CI shows the following however:

-- Found OpenMP_CXX: -fiopenmp  
-- Found OpenMP: TRUE  found components: CXX 
-- oneDPL: ONEDPL_PAR_BACKEND=tbb, disable OpenMP backend

Maybe this is related.

@psychocoderHPC
Copy link
Member

psychocoderHPC commented Dec 21, 2023 •
edited

I tried to change it back to the implementation we used in the past which is compatible with old OpenMP 3.1+ Standard

#        pragma omp atomic capture compare
                {
                    old = ref;
                    ref = (value < ref) ? value : ref;
                }

But even then the compiler is complaining.
I checked the standard https://www.openmp.org/spec-html/5.1/openmpsu105.html#x138-1480002.19.7 and can not find whats wrong with our implementation.

There is also a clang fix available which introduced this error message: https://reviews.llvm.org/D102449

@bernhardmgruber
Copy link
Member Author

I also think our implementation is correct. Clang 17 compiles it correctly.

Here is a reproducer using the alpaka single-header: https://godbolt.org/z/vdnfaE48E

@psychocoderHPC
Copy link
Member

for min/max this works

		#        pragma omp atomic capture compare
		                {
		                    old = ref;
		                    if(value > ref)
		                        {ref = value;}
		                }

@psychocoderHPC
Copy link
Member

looks like icpx requires { and if those are missing then it fails. #2209 is currently running the validation.

psychocoderHPC added a commit to psychocoderHPC/alpaka that referenced this issue Dec 22, 2023
@psychocoderHPC
fix icpx atomic OpenMP methods
354bb2b 
fix alpaka-group#2205

The compiler requires that the atomics are written in a very spezific
syntax else the code is not compiling.
psychocoderHPC added a commit to psychocoderHPC/alpaka that referenced this issue Dec 22, 2023
@psychocoderHPC
fix icpx atomic OpenMP methods
74ac3d7 
fix alpaka-group#2205

The compiler requires that the atomics are written in a very specific
syntax else the code is not compiling.
@psychocoderHPC psychocoderHPC mentioned this issue Dec 22, 2023
Merged
bernhardmgruber pushed a commit that referenced this issue Dec 27, 2023
@psychocoderHPC @bernhardmgruber
fix icpx atomic OpenMP methods
70285ba 
fix #2205

The compiler requires that the atomics are written in a very specific
syntax else the code is not compiling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type:Bug
Projects
None yet
Milestone
No milestone
2 participants
@bernhardmgruber @psychocoderHPC