LG. All of my concerns have been resolved.

In D145831#4211430, @JonChesterfield wrote:

OK then as written this is definitely going to blow up on us. We shouldn't implement the general purpose lock API if it deadlocks unless called in a very specific situation.

Probably best to emit the CAS in line as part of the IR transform, but otherwise we could add more runtime functions specific to critical. Uses of the general purpose omp_lock should be a compile time error on platforms that can't do it (it's unfortunate that lock returns void), but until then builtin_trap at least looks clearer when debugging than deadlock.

Removed attributes (as original) from clang test OpenMP/target_map_codegen_hold.cpp.

Fixed clang test OpenMP/target_map_codegen_hold.cpp.

Fixed LIT test failure, added new clang test OpenMP/bug60602.cpp.

Addressed comment. Using the alloca insert point for kernel args alloca.

In D145820#4185695, @jhuber6 wrote:

No tests updated?

TODOs: Update existing LIT tests, add a new one.

In D127372#4179027, @jplehr wrote:

Just to reiterate what I think this does is: It adds tests which currently ensure that the call backs can not be registered (as it should).
The tests would then be updated for the other patches, when these call backs can be registered, right?

ping. @jhuber6 @dreachem

@jhuber6 Is this patch ok to land? This will provide some test coverage to the existing OMPT target functionality, the device-independent part. As plugin-specific changes land, these tests will have to be updated.

@mhalk Can this be landed on top of trunk today? I would be interested in landing this one on top of what we have today so that we get some test coverage as part of "make check". You will need to update the tests to match the current code and update the tests as patches 4-7 land.

After rebasing on top of main today and regenerating all the auto-update clang tests, here are the test results. The AST tests have to be updated manually as Johannes mentioned earlier. I haven't looked at the other clang test failures.

Rebased.

I don't know why the problem is showing up now. ompt_start_tool has been there for a long time. While libomp_ompt_connect is new, it is unconditionally defined.

Can you please describe at a high level what data "Device Time Profile" is supposed to collect and how you are collecting it? Is there a specification somewhere for this functionality?

@tianshilei1992 I tested builds w/ and w/o -DLIBOMPTARGET_ENABLE_DEBUG=ON. Any concern if I land the updated patch?

Fixed build failure with LIBOMPTARGET_ENABLE_DEBUG.

I can repro the failure with -DLIBOMPTARGET_ENABLE_DEBUG=ON added. I will update the patch.

In D123974#3965638, @tianshilei1992 wrote:

Looks like this patch breaks the build.

Anyone else has any comment before I land this patch?

Variable name changed.

Rebased. Please review.

Rebased.

In D123572#3921397, @jhuber6 wrote:

Make sure the new files in libomptarget match the LLVM style for the variables and filenames. Otherwise looks file

Changed new file name from ompt-connector.h to ompt_connector.h.

Turns out a rebase on top of trunk had ~200 test conflicts. During my last update in Sep, I had resolved all of the clang test conflicts and failures, there were only llvm test failures.

Rebased.

In D102107#3921948, @jhuber6 wrote:

@dhruvachak Do you still need help updating the LLVM tests?

Fixed issues reported in https://github.com/llvm/llvm-project/issues/58721

This patch had to be reverted because of a link failure https://github.com/llvm/llvm-project/issues/58721

Rebased

Rebased

Rebase

In D102107#3812946, @jdoerfert wrote:

In D102107#3812582, @dhruvachak wrote:

In D102107#3812554, @dhruvachak wrote:

Updated llvm tests. The following 3 tests still fail:

LLVM :: Transforms/OpenMP/spmdization_constant_prop.ll
LLVM :: Transforms/OpenMP/spmdization_guarding_two_reaching_kernels.ll
LLVM :: Transforms/OpenMP/spmdization_remarks.ll

@jdoerfert @jhuber6
I updated the LLVM tests except one, Transforms/OpenMP/spmdization_constant_prop.ll. There is no C source snippet in there. Can you help as to how to update it? Please review the diffs for all the updated LLVM tests as well.

In addition, the other 2 tests above fail even after updating. Looks like something is wrong. Can you help as to how to fix them?

Similar to clang tests, we are seeing remarks differences. We already decided to file an issue (after this patch lands) and look at them after-the-fact.

Did you recreate the tests from the C snipped? That is probably not a good idea. We should modify the IR. If we start with C code we can't do it like this anyway. I mean:

• the IR is totally different,
• the debug info is missing,
• lots of unrelated metadata,
• part of the device runtime was merged in,
• ...

In D102107#3812554, @dhruvachak wrote:

Updated llvm tests. The following 3 tests still fail:

LLVM :: Transforms/OpenMP/spmdization_constant_prop.ll
LLVM :: Transforms/OpenMP/spmdization_guarding_two_reaching_kernels.ll
LLVM :: Transforms/OpenMP/spmdization_remarks.ll

Updated llvm tests. The following 3 tests still fail:

This patch was reverted.

I reverted this commit while I fix the failing tests.

In D102107#3791292, @vitalybuka wrote:

check-llvm fails bunch of test for me

---

Failed Tests (12):

LLVM :: Transforms/OpenMP/custom_state_machines.ll
LLVM :: Transforms/OpenMP/custom_state_machines_remarks.ll
LLVM :: Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
LLVM :: Transforms/OpenMP/gpu_state_machine_function_ptr_replacement.ll
LLVM :: Transforms/OpenMP/is_spmd_exec_mode_fold.ll
LLVM :: Transforms/OpenMP/parallel_level_fold.ll
LLVM :: Transforms/OpenMP/spmdization.ll
LLVM :: Transforms/OpenMP/spmdization_assumes.ll
LLVM :: Transforms/OpenMP/spmdization_constant_prop.ll
LLVM :: Transforms/OpenMP/spmdization_guarding.ll
LLVM :: Transforms/OpenMP/spmdization_guarding_two_reaching_kernels.ll
LLVM :: Transforms/OpenMP/spmdization_remarks.ll

Rebased

Pointing out the recent changes at the corresponding source locations.

@jdoerfert Attached are the device IR files, generated with -save-temps.

All changes from my end are in. Please review.

@jdoerfert With this patch, additional remarks are being generated. Please check whether the new OMP121 remarks in the following tests are OK.

Rebased on top of a recent commit. Both check-clang and check-openmp (on amdgpu) pass.

Regenerated clang tests, make check-clang passes

ping

In D102107#3648219, @jdoerfert wrote:

make check-openmp passes on amdgpu. Need to check on nvptx.

Results from "make check-clang":

In D102107#3640232, @jdoerfert wrote:

In D102107#3640198, @dhruvachak wrote:

Thanks. I followed the above steps and regenerated a couple of the AST tests but they still fail. Perhaps I am missing some options?

I currently have a handful of clang test failures where regen did not work. I am going to update the patch, post the current test results, and we can figure out how to regen the rest before we land this patch.

So, generate check lines for new tests in a separate patch first.

Fixed opaque pointer miscompile.
Added alloc_aggregate_arg entry point to OpenMPOpt SPMD list.
Fixed nocapture attribute of kmpc_alloc_aggregate_arg,
Added align attribute for call to kmpc_alloc_shared.
Updated (most) failing clang tests.