Skip to content

Conversation

@JAicewizard
Copy link

@JAicewizard JAicewizard commented Dec 11, 2025

This makes emcc recognize -fopenmp, and links it as needed.

When updating openmp, it neesd to be compiled. This is not strictly neccesary, but we need to invoke cmake in order to generate the headers. When compiling openemp, we need to make use of cmake as well, as the build system is quite long and complicated. I am sure it can be done another way using ninja, and I encourage others to implement this, but I do not have the capacity right now to make this happen.

Fixes: #13892
Fixes: #17637

@JAicewizard
Copy link
Author

To clarify, I have tested this by compiling + linking a program requiring openmp

@sbc100 sbc100 changed the title Initial OMP support Initial OpenMP support Dec 11, 2025
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the contents of system/lib/libomp is coming from an upstream repo?

Can you using git submodule for this and put it in third_party/ maybe?

@JAicewizard
Copy link
Author

I assume the contents of system/lib/libomp is coming from an upstream repo?

Can you using git submodule for this and put it in third_party/ maybe?

Yess, it is from llvm. Since all the other llvm libraries are in this location, it made sense to put it here. Do you want the entire LLVM repository as a submodule? It seems a bit overkill, at that point the other libraries can also be pulled from there

@sbc100
Copy link
Collaborator

sbc100 commented Dec 11, 2025

I assume the contents of system/lib/libomp is coming from an upstream repo?
Can you using git submodule for this and put it in third_party/ maybe?

Yess, it is from llvm. Since all the other llvm libraries are in this location, it made sense to put it here. Do you want the entire LLVM repository as a submodule? It seems a bit overkill, at that point the other libraries can also be pulled from there

Oh i see, I didn't realize this was from llvm. I that case the import approach your took does seem reasonable.

@JAicewizard JAicewizard marked this pull request as draft December 11, 2025 17:25
@JAicewizard JAicewizard marked this pull request as ready for review December 16, 2025 01:13
@JAicewizard
Copy link
Author

Failing tests:

  • other.test_shell_cmd_with_quotes: No idea, the failing command works for me, but it fails in the test. No idea how to debug
  • wasm2js1.test_openmp_max_threads: Seems to fail because of unimplemented in binaryen: Assertion !curr->isAtomic && "atomic store not implemented"' failed.`
  • wasm64_4gb.test_openmp_max_threads: I fixed that libomp is no longer compiled for 32bit on 64 bit target, however libomp is not supported for 64bit wasm.

For the last two I would like to disable the tests, but I don't know how.
For the first, I really don't know how to find out what the issue is, let alone how to fix it.

Ruff issues can be easily fixed ofc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenMP support (omp.h file not found) Enabling openmp

2 participants