[SYCL][NVPTX][NFC] Refactor NVPTX target configuration #3535
Conversation
Avoid using `-sycldevice` for configuring NVPTX target.
|
/summary:run |
| @@ -48,7 +48,9 @@ class LocalAccessorToSharedMemory : public ModulePass { | |||
| // Access `nvvm.annotations` to determine which functions are kernel entry | |||
| // points. | |||
| auto NvvmMetadata = M.getNamedMetadata("nvvm.annotations"); | |||
| assert(NvvmMetadata && "IR compiled to PTX must have nvvm.annotations"); | |||
| if (!NvvmMetadata) | |||
There was a problem hiding this comment.
Is this a related change?
There was a problem hiding this comment.
Yes. We skipped this pass for modules w/o -sycldevice triple component in llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp and now this pass is executed regardless of the environment component value. SYCL compiler always added nvvm.annotations metadata, but it might be missing. In case when this metadata is not emitted, we just skip the transformation instead of crashing (with enabled assertions).
This patch doesn't add new functionality - it's a refactoring of existing features, so I think regression tests we already have should be enough. |
Thanks for explaining. But given the potential for affecting (one of your TODOs) non-sycl kernels, I would remove the [NFC] from the title, just to not mislead. |
|
@Naghasan, does it look okay to you? |
Avoid using
-sycldevicefor configuring NVPTX target.