- User Since
- Apr 2 2015, 4:52 AM (324 w, 4 d)
Wed, Jun 9
Tue, Jun 8
Mon, Jun 7
I think at least this patch needs a clang-format, maybe also applies to the other recent patches.
Sun, Jun 6
May 19 2021
Apr 7 2021
Can we get this fixed somehow? It's annoying that there is a test failure in Clang without building the OpenMP runtime, just because I have CUDA installed on my machine...
friendly ping :)
Mar 30 2021
Superseded by D87413, I think.
Long addressed by D72779, but I can't seem to be able to close this (?)
Feb 4 2021
Feb 3 2021
My proposal would be to cache the return value of the three routines in ToolChain. This has the advantage that the values get parsed only once and there is at most one warning. I don't know how this plays with parallelization efforts, but I don't think we should worry about this right now, given the current code.
Jan 9 2021
LGTM, thanks for the changes!
Jan 7 2021
Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?
Dec 16 2020
Dec 15 2020
Add comment for find_first_existing_vc_file, thanks @scott.linder.
Dec 13 2020
Dec 5 2020
Nov 13 2020
Oct 22 2020
By the way, there's no commit list subscribed which sounds bad. Adding llvm-commits to give people a chance to be aware of this.
Sep 17 2020
Sep 3 2020
Jul 8 2020
Jul 7 2020
This is definitely not NFC and breaks API compatibility (but apparently nobody cares anymore?).
Jul 6 2020
Jul 5 2020
The summary also talks about tools/multiplex/tests/*, is that information obsolete?
Jul 2 2020
Jun 29 2020
(In general, all patches must be sent to the respective -commits list. This also makes feedback more likely.)
Jun 19 2020
LIBOMP_HEADERS_INSTALL_PATH is the internal compiler where for example omp.h. I think the header tool is of relevance outside of Clang, so IMO it should go the general include/ directory.
May 18 2020
I don't feel comfortable reviewing this change, maybe @phosek can take a look.
May 15 2020
Does -fopenmp-version work with the Intel Compiler? How will this work if the GCC eventually gets support for OpenMP 5.0?
Apr 21 2020
LGTM as well
This patch makes not visible in all of the openmp project's test
suites. In all but libomptarget/test, it should be possible for a
test author to insert not before a use of the lit substitution for
running a test program.
Apr 10 2020
Apr 9 2020
Mar 30 2020
@thakis thanks for taking care so quickly!
Mar 26 2020
Mar 25 2020
Yes, this looks related to memory consistency. As far as I understand the threads synchronize on th_spin_here, so this is guaranteed to be updated. Any other write before this in __kmp_release_queuing_lock is not guaranteed to be synchronized by a weak memory model. This includes th_next_waiting (which triggers the assertion), but also writes by the user application. That's particularly bad because this should be taken care of by the runtime!
Mar 14 2020
Mar 6 2020
Feb 25 2020
Feb 11 2020
Feb 8 2020
This should have a RFC on openmp-dev. As far as I understand the linked patch, the aim is to use std::make_unique which is just syntactic sugar IMO. Unless there are good reasons to switch, I'd object because it requires newer versions of compilers than many HPC systems have.
Feb 3 2020
LGTM from the code, I didn't test that the issue is indeed fixed on i386 though
Feb 2 2020
@protze.joachim for 64 bit, we still need at least 5 threads IIRC
Jan 29 2020
As a general comment, I think this patch should be split into at least two separate ones:
- Load plugins from the same directory as libomptarget.so is found.
- Quick fail mechanism, although I'm not sure how much this saves? In most cases, we won't have the plugins for a foreign architecture (PPC on x86) anyway, that might only be a concern for accelerators.
Jan 23 2020
I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/. Maybe it would be better to move the option to the top-level openmp/CMakeList.txt and rename it to OPENMP_OMPT_SUPPORT (with compatibility support for LIBOMP_OMPT_SUPPORT)
Jan 16 2020
Jan 15 2020
Jan 14 2020
This revision was not accepted before being committed!
Jan 6 2020
Can you please also submit a patch to libc++ where we copied this code from?
Dec 31 2019
Dec 30 2019
Yes, this must not be changed for compatibility reasons. If this really brings a benefit, we need new entry functions and update the compiler(s).
Dec 11 2019
Dec 7 2019
Dec 6 2019
Dec 2 2019
Nov 29 2019
Nov 19 2019
Random drive-by comments to parts of the patch, I won't review in full.
Nov 16 2019
Testing of openmp is currently very lacking, but I don't think this change will apply cleanly anymore...