[SYCL][sycl-post-link] Implement transformation of sycl-properties annotations #5940
Conversation
…notations This commit adds the transformation from llvm.ptr.annotations calls with "sycl-properties" as the annotation string into llvm.ptr.annotations calls with decoration IDs and values in a format the SPIR-V translator is able to translate into SPIR-V decorations. Signed-off-by: Steffen Larsen <[email protected]>
| assert(IntrInst->getNumUses() == 0); | ||
| IntrInst->eraseFromParent(); | ||
| } | ||
|
|
||
| // The pass just adds some metadata to the module, it should not ruin |
There was a problem hiding this comment.
I'm not sure if this comment is actual now because the pass doesn't just add some metadata anymore. Can rewriting the @llvm.ptr.annotation intrinsics have any influence on the analysis?
There was a problem hiding this comment.
I don't imagine changing the annotation strings of a llvm.ptr.annotation with our own "sycl-properties" special-name should affect any analyses. However, I don't know if removing an annotation could affect any previous analyses. @AlexeySachkov - Do you know?
There was a problem hiding this comment.
@kbobrovs - Do you have some insight into this?
There was a problem hiding this comment.
I think we really care if this change can affect transformation (and therefore lead to different IR). Theoretically it could affect subsequent IR t-forms happening before SPIRV generation if they depend on llvm.ptr.annotations (maybe transitively via Analyses). But AFAICS, this is the last tform before SPIRV.
BTW, If this transformation does not require fully linked program, then sycl-post-link would be a wrong place for it. It is always preferable to put additional IR passes to compilation (via BakendUtil.cpp) over putting them into post-link.
There was a problem hiding this comment.
BTW, If this transformation does not require fully linked program, then sycl-post-link would be a wrong place for it. It is always preferable to put additional IR passes to compilation (via BakendUtil.cpp) over putting them into post-link.
At the moment I don't believe it needs a fully linked program, but we could have a similar situation in the future to the devce_global where one of the properties need metadata information propagated to the runtime. We could of course create a new representation sycl-post-link would need to look for, but it does complicate the design.
Likewise, we would be splitting the property-translation logic, that is some would be done in additional IR pass (annotations -> annotations) while others would be done in sycl-post-link (IR attributes -> metadata). Would there be a good way for the two to share some logic?
@gmlueck - Ping in case you have some opinions on this.
There was a problem hiding this comment.
I assume this is the pass that converts @llvm.ptr.annotation from the form that contains the property names (strings) into the form that contains SPIR-V decoration numbers? (This follows the design in #5884.) Correct?
That pass is part of the general logic for converting LLVM IR into a "SPIR-V friendly form", and I thought we did all of this logic in sycl-post-link, just before we translate to SPIR-V. One advantage of this, I suppose, is that we can avoid doing this translation for targets that don't use SPIR-V.
There was a problem hiding this comment.
This is indeed the implementation of that pass, currently shared with the logic for translating IR attributes.
Since @kbobrovs suggests for the pass to be part of BackendUtil.cpp, I believe we should be able to make it an elective pass that is only run when SPIR-V is the target, while other backend targets can use their own pass for the translations. However, for device_global sycl-post-link still needs to create the unique ID and communicate it to the runtime. We may be able to split the generation of the unique ID and the generation of the corresponding metadata to an earlier IR pass, but in that case sycl-post-link would have to parse the "SPIR-V friendly" metadata and base the information for the runtime on that.
There was a problem hiding this comment.
We may be able to split the generation of the unique ID and the generation of the corresponding metadata to an earlier IR pass, but in that case sycl-post-link would have to parse the "SPIR-V friendly" metadata and base the information for the runtime on that.
Isn't this a reason not to move the translation to "SPIR-V friendly" form to BackendUtil.cpp? If we did this, then sycl-post-link needs to understand either the SPIR-V friendly form or the normal LLVM IR form (depending on whether the target supports SPIR-V)?
There was a problem hiding this comment.
If we did this, then sycl-post-link needs to understand either the SPIR-V friendly form or the normal LLVM IR form (depending on whether the target supports SPIR-V)?
Either that, or we would make a common part of the pass (or another pass) that adds some new information that sycl-post-link will consume. It does complicate the design, but it would make backend-specific parsing of properties easier.
Either way, I think it should be considered for the entire pass rather than the changes made in this. @kbobrovs do you think there would be a problem in going with it as part of sycl-post-link for now and then discuss the transition to BackendUtil.cpp separately?
There was a problem hiding this comment.
@steffenlarsen, I think transition to BackendUtil.cpp should be discussed separately, my comment was not meant to hold this PR.
A note about BackendUtil.cpp vs sycl-post-link.cpp. The original intent of sycl-post-link was to perform transformations which can not be done w/o knowing the entire program - such as assigning specialization constant IDs. The more transformations are implemented in sycl-post-link, the farther SYCL is from other compilers many of which don't even use any additional link-time IR transformations by default. It is like having a "hidden pass manager" in the form of the sycl-post-link, which bypasses clang optimization/transformation control mechanisms. There are some other disadvantages: extra link time (important for device libraries, which get compiled once and linked many times), compiler developers need to run post-link to see final code before SPIRV.
Signed-off-by: Steffen Larsen <[email protected]>
Signed-off-by: Steffen Larsen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
This commit adds the transformation from llvm.ptr.annotations calls with "sycl-properties" as the annotation string into llvm.ptr.annotations calls with decoration IDs and values in a format the SPIR-V translator is able to translate into SPIR-V decorations.