User Details
- User Since
- Sep 24 2015, 4:45 AM (391 w, 1 d)
Jan 26 2023
Jan 25 2023
Explicitly link libdl as suggested by @vzakhari.
Jan 24 2023
Fix the special case handling for RunningOnValgrind pointed out by @Hahnfeld . We don't want to have output for a missing call, so it still has a separate macro.
Jan 23 2023
Jan 18 2023
Since you seem to have access to a Mac, can you at least paste the output?
I'm posting here, because this change broke LLVM tests on a machine only supporting SSE3.
Dec 12 2022
Why do we need __kmpc_omp_taskwait_51 ? Taskwait nowait only makes sense with dependencies. It should only result in additional nodes/edges in the task dependency graph without any code to execute. There is not taskwait (aka task barrier) in this case.
Dec 5 2022
lgtm
Nov 29 2022
I did not argue against adding ((char *)addr) - 12. But following the description in the comment, I think, ((char *)addr) - 4 should stay there, as this might still be valid for other cases (e.g., compiling with other optimization constraints?).
Nov 28 2022
Your change only allows offsets of 8 and 12 byte. But the comment indicates that 4 byte offset might still be possible? I think, you should just print the 12 byte offset in addition.
LGTM
Oct 31 2022
Oct 28 2022
lgtm
Sep 21 2022
This change bricks the old tsan runtime. Should we remove the call from rtl-old as well? Or is rtl-old not maintained at all?
Sep 7 2022
Lgtm
Without testing, I would say the variable is not visible in this CMake scope, as it is defined in openmp/runtime/src/CMakeLists.txt. I would assume that you need to set it with PARENT_SCOPE in openmp/runtime/src/CMakeLists.txt and openmp/runtime/CMakeLists.txt. Similar to LIBOMP_LIBRARY_DIR or LIBOMP_INCLUDE_DIR.
Aug 11 2022
Lgtm
Jul 20 2022
An alternative approach would be to install the python modules wheels and setuptools into the build directory using pip instead of bailing out.
Jul 15 2022
Jul 2 2022
Just one more change. Then the ompt part looks good to me.
Jul 1 2022
Jun 30 2022
Jun 28 2022
May 16 2022
Are the llvm version variables defined for stand-alone builds? So, will stand-alone builds of OpenMP continue to work and include libomptarget code?
May 3 2022
lgtm now
May 2 2022
Thanks for the fix! I'm quite confident that this change will fix the sporiadic test failures (most probably under high system load).
Apr 29 2022
Apr 6 2022
This patch LGTM.
I checked the runtime tests and found that openmp/runtime/test/barrier/omp_barrier.c already triggers different combinations of barriers. Do the combinations cover all configurations you tried manually?
Regarding reductions, I think it would make sense to add similar run lines to openmp/runtime/test/worksharing/for/omp_for_reduction.c in order to cover different code paths in the runtime.
I agree, the expected output does not fit the general case of reductions.
As a workaround, I would change the check prefix for reduction 2-n from CHECK to MULTIPLE and add --check-prefixes CHECK,MULTIPLE for the RUN-lines, where multiple pairs of reduction callbacks are expected. So the default would be "at least one" pair of callbacks.
I think, in addition to manually selecting the barrier implementation, we should add additional run lines to specific tests that trigger the different barrier implementations. I added a related, OMPT-specific comment to D123193.
Can you add extra run lines to openmp/runtime/test/ompt/synchronization/reduction/*_reduce.c in order to trigger the different barrier implementations?
Thanks for working on this!
Apr 1 2022
I think, the name of the variable is confusing. The tests triggered and influenced by your example command are not limited to libomp. Currently the LIBOMP_ prefix is mainly used for cmake variables. The make target to run the tests is called check-* rather than test-*.
Mar 7 2022
Mar 3 2022
For the test I suggest signalling like used for OMPT tests (see openmp/runtime/tests/ompt/ompt_signal.h) to enforce the execution ordering rather than sleep "synchronization".
Feb 24 2022
Feb 21 2022
I think in openmp/tools currently only libraries and header files get installed, but no binaries.
Jan 31 2022
Dec 15 2021
Thanks for the update. LGTM.
Can you commit yourself, or should I commit the fix for you?
Dec 8 2021
Just return, if (Data->Parent == nullptr).
Dec 7 2021
lgtm
Nov 22 2021
Nov 17 2021
I guess the question is, whether we expect more reserved locators will be introduced in the future. Since the spec actually introduced a new section, I thought this might really happen. During OMPT discussion, Deepak also raised the issue, that these future reserved locators might not be limited to inout/out dependencies.
Therefore, I thought it would make sense to let the flag just indicate that this involves a reserved locator and encode the kind of locator in the address argument.
Nov 16 2021
Nov 12 2021
I mainly checked the libomp code and put some inline comments.
Nov 10 2021
Nov 2 2021
Oct 29 2021
Oct 25 2021
Thanks for the clarification.
lgtm
@jlpeyton can you check with your compiler team, what is wrong with icc 2021? icx works fine.
Tests should not burn CPU cycles. We use my_sleep from omp_my_sleep.h or delay from ompt-signal.h instead to allow other tests make progress in the meantime.
Looks like we missed to update the code in kmp_csupport, when the management of lwt was refactored.
lgtm
In my local test execution I just got:
lgtm
Oct 22 2021
Writing up the OMPT interface for omp_all_memory, I realized that treating omp_all_memory individually might not be the best choice, considering that the spec talks about "Reserved Locators".
We will finish and discuss the spec ticket during the OMPT call next week. I'll commit after the call - just in case we decide for different names.
Oct 21 2021
Why are the types in __kmpc_atomic_val_4_cas_cpt different from the types in __kmpc_atomic_val_4_cas? I think, all the functions should use fixed-size integer to respect the width suggested by the name of the function.
start_tool = (ompt_start_tool_t)dlsym(h, "ompt_start_tool");
Oct 19 2021
Please update the diff with context (-U9999)
Please update the diff with context (-U9999)
Oct 18 2021
I already know, that OMPD folks will not be happy about this implementation. Nevertheless, it is a valid and better implementation than before.
Overall looks good to me besides one inline comment.
I successfully tested with gcc/11, icc/19 and latest clang.
lgtm