1. Simplifies overall patch after D90248.
2. Removes MaxParallelLevel and thus target specific PrePostActionTy.
3. Removes ExternallyInitialized qualifier from shared variables for AMDGCN.

Ping.

Ping.
Merged patch D87841 uses the Gridvalue as described here.

1. Removed unnecessary formatting of untouched code.
2. Encapsulated addFieldToRecordDecl and createGlobalStruct methods in a class and made them static (triggered change at all calling sites).
3. Marked most of the member methods of CGOpenMPRuntimeAMDGCN as private (forgot to do same change in nvptx)
4. Fixed the memory leak
5. Marked appropriate member variables as protected in CGOpenMPRuntimeGPU

AFAIK, all files which are required/should be available in libomptarget reside in llvm/Frontend/OpenMP/* , so may be an option can be provided for only-tarball users to get files only from this directory.
This kind of approach will also enforce people exporting stuff from llvm to libomptarget, to put everything in one place.

Ping.

Moved the code to load plugins to a helper function and before loop on plugins.

First of two patches after splitting D73657

Ping.

In D85214#2243160, @gribozavr2 wrote:

@saiislam This change seems to have broken a buildbot, please take a look at your earliest convenience.

Before: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35354

After: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35355

1. Reformarting
2. Comments
3. Reduced scope of specialized PrePostActionTy

1. Moved amdgcn specific functions to CGOpenMPAMDGCN.cpp
2. Removed tautology condition
3. Corrected case of local variables
4. Restored original formatting
5. Changed back declaration of emit kernel methods as private
6. Added support of amdgcn specific PrePostActionTy implementation and its corresponding test cases
7. Changed static line numbers in new test cases with regex
8. Other small code corrections

In D86232#2226668, @JonChesterfield wrote:

This can't be used in the libomptarget plugins or in the deviceRTL, as both are required to compile without the llvm sources available.

Current preferences appears to be to copy and paste the numbers between the two files. At the least, we should have a test outside of llvm & openmp trees that verifies the numbers are the same in each.

@jdoerfert can you remember who was strongly against #include between openmp and llvm? I'm failing to find the discussion, and the mail "Sharing enums between llvm and openmp" didn't reach a conclusion

Please let me know if there are any changes/suggestions.

Time for check-clang has increased from ~51 sec to ~107 sec after this patch.
Putting it here before committing/closing the review.

Ping.

Ping.

Ping for this (part-1), D85150 (part-2), and D85214 (part-3).

Next two (independent) parts are: D85150 and D85214. More to come.

Abandoning in favor of D85214

This patch updates 101 out 320 test files. Remaining will be posted as a separate patch to ease review.

Abandoning in favor of D84844 and D85150 i.e. adding default version testing along with version 4.5 testing.

Removed redundant testing for version 5.0 with its version string.

Changes as requested.

Improved comment placement. Corrected check lines. Rebased.

Moved comments to header files. Used regexps for test cases.

Generalized regex in one of the test to pass harbormaster build.

Added InitTempAlloca address space cast.

LGTM. Please wait for @arsenm 's review.

Added final to specialized classes.

Thank you. LGTM.

It is causing various recent OpenMP harbormaster builds to fail like the one for D84148 and D84221.

Can you add test with version string 50 also?
So, tests should be there for version 45, version 50, and default version. It will ensure that this test doesn't silently stops testing for 50 when we upgrade to the next version.

In D64571#2162388, @ABataev wrote:

In D64571#2160362, @protze.joachim wrote:

The test fails for me with segfault in line 22.

$ gdb target_depend_nowait.cpp.tmp-x86_64-pc-linux-gnu
(gdb) run
...
Program received signal SIGSEGV, Segmentation fault.
(gdb) bt
#0  0x0000000000400fcf in .omp_outlined._debug__ (.global_tid.=0x2aaab96b9be0, .bound_tid.=0x2aaab96b9bd8) at target_depend_nowait.cpp:22
#1  0x0000000000401e8d in .omp_outlined..23 (.global_tid.=0x2aaab96b9be0, .bound_tid.=0x2aaab96b9bd8) at target_depend_nowait.cpp:18
#2  0x00002aaaab574213 in __kmp_invoke_microtask () from libomp.so
#3  0x00002aaaab51338e in __kmp_invoke_task_func () from libomp.so
#4  0x00002aaaab5126bf in __kmp_launch_thread () from libomp.so
#5  0x00002aaaab55d3d0 in __kmp_launch_worker(void*) () from libomp.so
#6  0x00002aaaabbd6e65 in start_thread () from libpthread.so.0
#7  0x00002aaaabee988d in clone () from libc.so.6

It is not the problem of this patch, looks like support for mappers causes failures

Corrected.

Removed getNVPTXWarpSize() changes for a separate patch and rebased.

In D83863#2153031, @ABataev wrote:

In D83863#2153021, @saiislam wrote:

In D83863#2153008, @ABataev wrote:

Not sure that this is really required. If we want to check for the default version, it would be good to check that it still works for 4.5. It means, need to add additional RUN lines. But I don't think it is profitable to test the same functionality several times without actual effect

These 67 files were testing only for version 4.5. As the default version is now 5.0, isn't it expected that the tests run against it?

It just means that there are no changes in functionality between 4.5 and 5.0. Sure, you can switch to the default version instead but I don't see real profit here. Anyway, it is up to you.

In D83863#2153008, @ABataev wrote:

Not sure that this is really required. If we want to check for the default version, it would be good to check that it still works for 4.5. It means, need to add additional RUN lines. But I don't think it is profitable to test the same functionality several times without actual effect

[Work in progress]
Among all the test files in clang/test/OpenMP which were running only for version 4.5, I am yet to fix following files (rest all are given in this patch):

Instead of adding additional test cases without version string, removed all occurrences of -fopenmp-version=50.

Thank you.
I will hold committing till llvm-11 branching so it doesn't break something in any downstream dependent project at the last moment.

Removed all references to AMDGCN to make it NFC. Will introduce AMDGCN specific things separately in a followup patch.

Removed all formatting and spacing changes introduced due to clang-format.

In D83492#2142086, @JonChesterfield wrote:

Changing to getGridValue would be useful for sharing parts of this with amdgcn.

The aomp toolchain handles codegen for amdgcn by adding if (isAMDGCN) to this file. Until such time as tregions obsoletes this code, I think we should go with layers instead of scattered conditionals.

I.e. rename CGOpenMPRuntimeNVPTX to CGOpenMPRuntimeGPU which contains code that is common to nvptx and amdgcn. That probably uses getGridValue() as a way to abstract over minor differences. Derive CGOpenMPRuntimeAMDGCN and CGOpenMPRuntimeNVPTX from CGOpenMPRuntimeGPU to implement (virtual) functions which are different between the two.

1. Removed unused functions
2. Changed to INLINE
3. Moved fence functions to target_impl.h

In D80804#2123183, @JonChesterfield wrote:

This patch declares the clang builtin as acting on signed values, but the IR intrinsic maps to an instruction which does an unsigned comparison. We don't have ISA support for a signed comparison equivalent. Addition is the same operation on signed or unsigned integers, but signed integer comparison is not equivalent to unsigned integer comparison.

// 32bit
 tmp = MEM[ADDR];
 MEM[ADDR] = (tmp >= DATA) ? 0 : tmp + 1; // unsigned
compare
 RETURN_DATA = tmp.

The builtins should be changed to take unsigned values, optionally making that clear from the naming scheme, perhaps __amdgcn_builtin_atomic_dec_u32.

Apologies for not reviewing this the first time around.

In D83077#2130159, @protze.joachim wrote:

I have no idea, what Harbormaster is trying to tell me.

https://buildkite.com/llvm-project/premerge-checks/builds/2535#_

shows a failure in ninja all, but the only information about the command is in the artifact, which shows [4042/4042] successfully built targets.

Removed fix for OpenMPTesting.cmake bug.