Initial OpenMP support #25937
Initial OpenMP support #25937
Conversation
|
To clarify, I have tested this by compiling + linking a program requiring openmp |
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
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?
| @@ -563,6 +563,12 @@ def consume_arg_file(): | |||
| else: | |||
| exit_with_error(f'invalid value for --output-eol: `{style}`') | |||
| # Record PTHREADS setting because it controls whether --shared-memory is passed to lld | |||
| elif arg == '-fopenmp' or arg == '-fopenmp=libomp': | |||
There was a problem hiding this comment.
arg in ('-fopenmp', '-fopenmp=libomp'):
| @@ -563,6 +563,12 @@ def consume_arg_file(): | |||
| else: | |||
| exit_with_error(f'invalid value for --output-eol: `{style}`') | |||
| # Record PTHREADS setting because it controls whether --shared-memory is passed to lld | |||
| elif arg == '-fopenmp' or arg == '-fopenmp=libomp': | |||
| settings.OPENMP = 1 | |||
| # Openmp needs pthreads, they are implied by -ofopenmp | |||
| @@ -431,4 +432,4 @@ def restore(self, previous): | |||
| self.attrs.update(previous) | |||
|
|
|||
|
|
|||
| settings = SettingsManager() | |||
| settings = SettingsManager() | |||
There was a problem hiding this comment.
Looks like the final newline here got deleted?
| shutil.copyfile(os.path.join(build_dir, "lib/libomp.a"), out_filename) | ||
|
|
||
| if not shared.DEBUG: | ||
| utils.delete_dir(build_dir) |
There was a problem hiding this comment.
We use 2-space indentation in our python code (for better or worse :(
| "cmake", | ||
| path, | ||
| "-G", | ||
| "Ninja", |
There was a problem hiding this comment.
We don't currently have any system libraries or ports that depend on external tools such as cmake or ninja to build. We basically build everything ourselves to avoid depending on these tools.
There was a problem hiding this comment.
I know, I currently don't have the time to figure out what files need to be compiled with what flags. The CMake files are too large to spit through for me ATM. Moving off CMake would be great, as currently a lot of code is in the libomp directory, almost none of which is actually necessary (I did not realize this was +150k before opening the PR).
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. |
2 participants
This makes
emccrecognize-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