It looks like the two platforms are using different sleep duration? Other than that, LGTM.

In D146642#4229069, @josemonsalve2 wrote:

In D146642#4229067, @tianshilei1992 wrote:

In D146642#4229048, @josemonsalve2 wrote:

In D146642#4228996, @tianshilei1992 wrote:

I think it's better to guard the entire related code with macro.

What do you mean by this Shilei? To disable this feature?

Like a opt-in feature.

Why do you think so? What's the harm of leaving it enable as this is just an API function? Are you saying this to save space in the structs?

In D146642#4229048, @josemonsalve2 wrote:

In D146642#4228996, @tianshilei1992 wrote:

I think it's better to guard the entire related code with macro.

What do you mean by this Shilei? To disable this feature?

I think it's better to guard the entire related code with macro.

Sure

In D146725#4216706, @jhuber6 wrote:

In D146725#4216704, @tianshilei1992 wrote:

Does it have to be sys? Does gl (kernel level) work?

It should be sys as far as I understand because this is intended to be used on the Nvidia USM to implement RPC. Also I believe __atomic_thread_fence defaults to system scope on AMDPGU as well.

Does it have to be sys? Does gl (kernel level) work?

LG

Do we support in reduction yet?

Looks reasonable to me.

LG

Can you update the patch with context?

LG

Only llvm/test/Transforms/OpenMP/spmdization_constant_prop.ll is left.

update tests in LLVM

differentiate between no callback and unknown value

I think this looks reasonable to me. @jdoerfert WDYT? I'm not sure if you need to fix some clang tests. Let's see if Buildbot is happy.

rebase and fix comments

Pass two benchmarks: XSBench and RSBench.
The patch will break the old plugins as well. I'm wondering we might want to land it after D142820.

rebase, update tests, fix comments

update

remove hash

I prefer to follow established convention.

unify check prefix(es)

fix the WA, rebase, and update some tests

Seems like the schedule is the end of Feb. I'll update the patch properly by then.

rebase and fix comments

I don't know too much on Windows, but it looks reasonable to me. Better to wait for @jlpeyton and @natgla.

LG

LGTM

FWIW, there are methods in CMake to check if features (such as if a struct contains specific data member, or if a library contains APIs). I think that would be the right way to fix the issue.

I don't know about the specific assembly but I assume it can fix the issue.

In D143527#4111295, @josemonsalve2 wrote:

I’m confused that Shilei said in the issue in GitHub that this is supported already. But front end is just throwing an error message.

Our Sema can support it but CodeGen doesn't. Here we want to error it out instead of "silently" pass if assertion is disabled.

Can you add a test please?

Can you update the patch with context?

In D142593#4102328, @arsnyder16 wrote:

In D142593#4101532, @tianshilei1992 wrote:

Is it feasible to test it somehow? I doubt the regular way to invoke tests can work with WASM.

Yes, the compiled wasm can be run through node. In our product we use gtest and we only ship our wasm in the browser so we build the gtest projects using emrun so we test within a headless chrome browser.

I think node would make sense here. Which tests specifically would you like to get working? runtime/test? Can you point me to any docs or examples of running the tests. As i get time i can poke at working through that.

The build of plugins don't need device runtime right? We set dependences in CMake for build and test running only.

Is it feasible to test it somehow? I doubt the regular way to invoke tests can work with WASM.

I don't understand why a host library depends on a device library?

LG, thanks!

rebase

LG

We have seen a couple of reports bout the new APIs introduced in this patch, such as https://github.com/llvm/llvm-project/issues/60393 and https://github.com/llvm/llvm-project/issues/58472. Can you please fix it? We have the issue in both LLVM 15 and 16 now.

LGTM

rebase and fix bugs

In D142820#4088307, @jhuber6 wrote:

Where's dynamic_cuda and dynamic_hsa. Also why not replace plugins with nextgen instead of keeping the name.

fix test line again

optimize run lines

fix typo

rebase and fix bugs

In D142684#4084862, @mstorsjo wrote:

Thanks for the fix! It does indeed seem to fix the issue.

Do you happen to know why this worked before, but now suddenly stopped working after D142297 - was the stack usage so close to the edges of what's allowed, that any minor tweak pushed it over the limit, or did the change in D142297 affect the amount of stack used in a bigger way? (I presume that the main int result[N][N] array here only gets allocated once on the stack, and the OpenMP threads just refer to that? I presume the change didn't cause the array to be allocated in more places than before?)