[SYCL][ESIMD] Remove LowerESIMDVecArg pass and update opaque pointer tests #10738
Conversation
|
@sarnex, thanks for the detailed description. I think we need to do follow-up code clean-ups.
Is there a check for "if opaque pointers are enabled"? If so, we should remove it as only opaque pointers are supported.
It makes sense to drop |
sarnex
changed the title
sarnex
changed the title
|
CUDA failures seem not related, some CI issue |
| @@ -1,8 +1,12 @@ | |||
| // RUN: %clangxx -O0 -fsycl -fno-sycl-esimd-force-stateless-mem -fsycl-device-only -Xclang -emit-llvm -Xclang -no-opaque-pointers %s -o %t | |||
| // RUN: %clangxx -O0 -fsycl -fsycl-device-only -fno-sycl-esimd-force-stateless-mem -Xclang -emit-llvm %s -o %t | |||
There was a problem hiding this comment.
this (and all the tests) are just a file rename from _opaque with a possible -opaque-pointer flag removal, you don't have to review the test contents itself, they are all already in-tree.
|
@LU-JOHN hey, do you mind reviewing this from the tools pov? changes should be trivial. thanks |
|
@maksimsab thanks! @intel/llvm-gatekeepers i think we are good to merge, cuda failures unrelated! thanks! |
Please merge in latest origin/sycl. I'd like to ensure the testing works. |
After intel@0c51f60, opaque pointers are enabled by default for device code. This means that LowerESIMDVecArg is now never run because we do a runtime check in sycl-post-link and only run it if opaque pointers are off. Remove the pass since it's now dead code, and remove related tests. The tests check for changes unique to LowerESIMDVecArg and cannot be ported to instead use the pass that is run when opaque pointers is enabled, ESIMDOptimizeVecArgCallConvPass, and that pass has its own tests already. For the tests removed under sycl/test, all of those tests already have opaque pointer versions in-tree with the _opaque suffix, so there is no test coverage loss. Signed-off-by: Sarnie, Nick <[email protected]>
Signed-off-by: Sarnie, Nick <[email protected]>
Signed-off-by: Sarnie, Nick <[email protected]>
|
Done, CI running now |
|
@aelovikov-intel CI passed, so we should be good to merge, thanks |
After 0c51f60, opaque pointers are enabled by default for device code.
This means that LowerESIMDVecArg is now never run because we do a runtime check in sycl-post-link and only run it if opaque pointers are off.
Remove the pass since it's now dead code, and remove related tests. The tests check for changes unique to LowerESIMDVecArg and cannot be ported to instead use the pass that is run when opaque pointers is enabled, ESIMDOptimizeVecArgCallConvPass, and that pass has its own tests already and it does something completely different anyway.
For the tests removed under sycl/test/esimd, all of those tests already have opaque pointer versions in-tree with the _opaque suffix, so there is no test coverage loss.
I also updated tests in sycl/test that have the _opaque suffix to not have the suffix, removed the typed pointer versions, and removed the opaque pointer flags.