[SYCL][CUDA][libclc] Add support for generic AS in atomics #5849
Conversation
t4c1
changed the title
There was a problem hiding this comment.
@t4c1, please, fix tests on HIP platform and verify your patch with updated tests (i.e. add a comment "/verify with intel/llvm-test-suite#929").
|
These unexpected passes on HIP do not seem to be related to my changes. I can reproduce them even with |
|
/verify with intel/llvm-test-suite#929 |
|
@bader I don't think this worked. |
I think they are related. These tests fail in all on other pre-commit runs and this patch makes multiple fixes into functionality covered by these tests, so it's very likely that these unexpected passes are caused by your changes. Even if they are not related, we should remove XFAIL to fix the test status. |
|
Yep, I did that. It is just that the command to run the tests from specific tests suite PR does not work when I comment it. |
@tfzhu, could you check why this comment #5849 (comment) haven't triggered PRs validation, please? |
Hm... I just found the same results in pre-commit for #5782 - https://github.com/intel/llvm/runs/5631254024?check_suite_focus=true. Interesting... |
|
I think timeouts for L0 tests are likely unrelated to changes in this PR. |
|
/verify with intel/llvm-test-suite#929 |
|
@t4c1, could you take a look at test results on Windows, please?
Are you able to reproduce this failure? |
@intel/llvm-external-read, this issue has been fixed and you should be able to tests PRs to llvm and llvm-test-suite together by adding a comment "/verify with ", where is either link to the PR from another repository. |
There was a problem hiding this comment.
@t4c1, could you take a look at test results on Windows, please?
ptxas fatal : Unresolved extern function '_Z22__spirv_GroupAsyncCopyjPU3AS3Dv2_aPU3AS1KS_yy9ocl_event'
Are you able to reproduce this failure?
@t4c1, I restarted to check if this some env. issue, but the same test failed again, so it looks like a regression. Please, take a look.
|
Sorry for forgetting to respond. We believe this is an issue with the change to remangler and @AidanBeltonS is looking into it. |
The diffs are quite hard to follow, but in an essence this patch brings: * a new entry, implementing a generic address space for multiple `__CLC_NVVM_ATOMIC_XYZ_IMPL`, where `XYZ` stands for `CAS`, `INDEC`, `LOAD`, `MAX`, `MIN`, `STORE` and `SUB`, * fixes the name of mangled function that the IMPL uses, * the rest is just formatting to 80 chars. This patch supersedes: #5849 but it requires the fixes to the remangler from: #7220 Fixes: #7658
|
Yes, I think so. |
Adds support for generic address space in atomics.
Also fixes
__attribute__((always_inline))being in wrong place for some of the atomics (it was placed so it applied to some forward declaration of functions used in the implementation of the atomic, instead of the definition of atomic).Also contains a fix for a bug in libclc remangler that caused wrong remangled names for some of the new functions in this PR (by @AidanBeltonS).
Closes #5215. Closes #5647.
Tests for this are enabled in intel/llvm-test-suite#929.