I don't see anything in here that would block later extension for supporting offloading, so LGTM from my POV.

LGTM, but I don't feel like I have the experience to "formally" approve. I left a nice-to have suggestion too, but feel free to ignore, if you feel its out of scope.

Fixed by D142249.

Looks good, also works for my original program, thank you! Just one nit and a question:

This change avoids the deadlock, but now library registration happens before (during) the device runtimes are loaded essentially skipping the registration.

In D142008#4068271, @JonChesterfield wrote:

I'm moderately surprised that ctors are called when dlopen opens something that is already open. Hopefully that's not platform specific.

In D142008#4067013, @JonChesterfield wrote:

We have provisional consensus that this is a bug in HSA. It shouldn't be calling dlopen on various processes it finds in the address space and a patch to stop it doing that is in progress.

Replace thread local with class member + atomic thread Id. Also factor out the delayed handling into member functions.
The reason the thread id checks are necessary is 1. avoiding race conditions. 2. not returning before the library is registered in threads that are not doing loadRtls.

I think you're missing some of the changes, I should have added a loop around the registration of the library to the plugins that goes over DelayedDescs and registers each, maybe it did not upload right?

In D142008#4062624, @jdoerfert wrote:

In D142008#4062615, @Maetveis wrote:

In D142008#4062603, @jdoerfert wrote:

This is too complex and we should not need static thread local stuff at all.
Why don't we set the flag that we loaded libomptarget early. If we end up loading it again we should just not initialize it. All we really want is to reduce the scope of the lock guarding the init flag, no?

Yes but the problem is that we didn't finish initializing it yet when the second call comes in, we're still inside the initialization of the amdgpu plugin so that one and any others after it didn't finish initialization yet. I.e. with your proposal the inner call would skip the "second" initialization then try to register the library to partially initialized plugins.

I don't see it. In either scheme the "inner/second" initialization of libomptarget will not actually initialize anything. You also return for the inner call and you will never come back, or how would it ever come back? Even if you do now, why would we. The second initialization is not necessary/wanted.

In D142008#4062603, @jdoerfert wrote:

This is too complex and we should not need static thread local stuff at all.
Why don't we set the flag that we loaded libomptarget early. If we end up loading it again we should just not initialize it. All we really want is to reduce the scope of the lock guarding the init flag, no?

In D142008#4062289, @JonChesterfield wrote:

That's a really surprising thing for HSA to be doing. It looks like it's crawling all dynamic libraries that happen to be in the host process address space, but that seems unlikely to be necessary.

In D142008#4062051, @jhuber6 wrote:

So, this occurs when we load the plugin shared library?

Add test.

In D131469#3808758, @dongjunduo wrote:

I'm sorry that I didn't see the comments until now because of some personal work... Thank you for your kindness and suppot, I will improve this part of work according to your idea in D133662 : )

As discussed with @jamieschmeiser on D133662, I have left suggestions regarding the approach I took for handling -o and passing the option to the jobs.

In D133662#3798276, @jamieschmeiser wrote:

I had an email exchange with @dongjunduo about this situation. He was a GSoC student that @Whitney and I were mentoring for the past summer. He agrees that your approach is cleaner. There appears to be two parts to your work. First, you implemented the determining and passing of the options differently, and secondly, you improved the handling of off-loading and system specific file handling. Based on your earlier response, we proposed to him the following and he agrees that it seems appropriate. Could you please add comments to https://reviews.llvm.org/D131469 and he will work with you to change his code to reflect searching for -o and using the virtual functions. Then, if @MaskRay agrees, he can land his code and finish up his GSoC work. You can then add your extensions of off-loading and file-handling. Is this acceptable to you?

Remove inline elements from the map. Add tests. With the tests I now consider this complete, so removed the draft from the title.

In D133662#3787250, @jamieschmeiser wrote:

I'm a little confused as to what is being proposed here. Is this building on D131469 or is it an alternative? It seems that there are portions of the code from D131469 included in these changes, which implies that you are building on it. I think this patch is premature in that the other patch has not yet landed (@MaskRay has asked for revisions that @dongjunduo has made but is waiting for review). When that patch has landed, this could be reposted based on those changes.

Sorry, I somehow missed the big obvious checkbox to mark the comments, fixed now.
I don't have commit access could you commit if the last change seems okay to you?

Address formatting and stylistic comments.

In D130108#3706550, @MyDeveloperDay wrote:

So trying to understand the problem statement, is it:

1. you have some files staged
2. and then you have to change them locally (review comment)
3. but forget to git add them,
4. you run git clang-format --staged
5. and it formats the new file (not added) based on the line numbers that are changing in the already staged file, which could be a different range to the original (is that correct?)
6. You think you've formatted the new file correctly (but likely missed a line or two)
7. You perform git add (as you now notice you've missed adding it)
8. You commit, but a commit hook tells you that you didn't clang format. (or worse still you are able to commit and push but you are pushing a badly formatted file, potentially)

is that correct?

Friendly Ping.

Updated to add missing comma after 'git ls-tree'. It broke normal (non staged) operation.

In D130108#3685620, @owenpan wrote:

Can you open an issue at https://github.com/llvm/llvm-project/issues? See https://github.com/llvm/llvm-project/issues/56736 for an example.