[SYCL] Do not emit "llvm.used" to output module(s) in the post link tool #2995
Conversation
This special global variable may cause problems for tools runing later in pipeline. Signed-off-by: Sergey Dmitriev <[email protected]>
sndmitriev
requested review from
AlexeySachkov,
bader and
kbobrovs
as code owners
Signed-off-by: Sergey Dmitriev <[email protected]>
|
/summary:run |
|
@AlexeySachkov, @bader, please review as soon as return from holidays. |
| @@ -688,6 +688,15 @@ int main(int argc, char **argv) { | |||
| Err.print(argv[0], errs()); | |||
| return 1; | |||
| } | |||
|
|
|||
| // Special "llvm.used" variable which holds references to global values in the | |||
| // module is known to cause problems for tools which run later in pipeline, so | |||
There was a problem hiding this comment.
@sndmitriev, could you clarify what kind of problems might be caused, please?
There was a problem hiding this comment.
After linking device bitcode llvm.used holds references to the kernels that are defined in the device image. But after splitting device image into separate kernels (with -fsycl-device-code-split=per_kernel option) we may end up with having references to kernel declaration originating from “llvm.used” in the IR that is passed to llvm-spirv tool, and these declarations cause an assertion in llvm-spirv
$ cat test.ll
target triple = "spir64-unknown-unknown-sycldevice"
@llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* @foo to i8*), i8* bitcast (void ()* @bar to i8*)], section "llvm.metadata"
declare spir_kernel void @foo()
define spir_kernel void @bar() {
ret void
}
$ llvm-as -o test.bc test.ll
$ llvm-spirv -o test.spv -spirv-max-version=1.1 -spirv-debug-info-version=legacy -spirv-allow-extra-diexpressions -spirv-ext=+all,-SPV_INTEL_usm_storage_classes test.bc
llvm-spirv: …/llvm/llvm-spirv/lib/SPIRV/SPIRVWriter.cpp:3438: void SPIRV::LLVMToSPIRV::transFPContract(): Assertion `FPC != FPContract::UNDEF' failed.
…
There was a problem hiding this comment.
I'm a bit confused with the reasoning.
That FP-contract related assertion seems like an unhandled situation in the translator which should be fixed there.
However, I do agree that @llvm.used might cause problems with the translation, because it contains function pointers and can't be translated into SPIR-V without corresponding extension (SPV_INTEL_function_pointers) enabled
There was a problem hiding this comment.
I do not argue with that, the problem needs to be fixed in the translator as well.
There was a problem hiding this comment.
How do we get this metadata in device code? It's not emitted in SYCL mode: aa35161. Is it emitted in other modes?
If so, shouldn't we drop @llvm.used during FE IR generation?
Anyway, it would be nice if we add clarification with reasoning to drop @llvm.used in sycl-post-link tool from this comment: #2995 (comment).
There was a problem hiding this comment.
How do we get this metadata in device code?
"llvm.used" variable may appear in the IR after linking with "deps" bitcode file (please see #2872 for more details).
Anyway, it would be nice if we add clarification with reasoning to drop @llvm.used in sycl-post-link tool from this comment: #2995 (comment).
I have created #3012 to address your comment. Please review it.
…e IR To address review comment intel#2995 (comment) Signed-off-by: Sergey Dmitriev <[email protected]>
…e IR (#3012) To address review comment #2995 (comment) Signed-off-by: Sergey Dmitriev <[email protected]>
This special global variable may cause problems for tools runing later in pipeline.
Signed-off-by: Sergey Dmitriev [email protected]