Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent adding entry point - cover of kernel function with same name #2029

Open
unDEFER opened this issue May 29, 2023 · 1 comment
Open

Comments

@unDEFER
Copy link

unDEFER commented May 29, 2023

I'm trying to compile the kernel from 05_spirvkernelfromfile of https://github.com/bashbaug/SimpleOpenCLSamples by commands:

$ clang -c -target spir64 -emit-llvm -o sample_kernel.bc sample_kernel.cl
$ llvm-spirv -o sample_kernel.spv sample_kernel.bc

And after this if I run

$ ./spirvkernelfromfile -p 1 --file sample_kernel.spv

I see in logs:

Kernel <Test.1> was successfully vectorized (8)

And the program not working, because the expected name of the kernel is just "Test" without ".1" suffix.

I can do

$ spirv-dis sample_kernel.spv > 2.asm

Manually change line

OpDecorate %Test LinkageAttributes "Test" Export

to

OpDecorate %Test LinkageAttributes "Test2" Export

And then:

$ spirv-as --target-env opencl2.1 -o 2.spv 2.asm
$ ./spirvkernelfromfile -p 1 --file 2.spv

If I compare assembly code of 2.asm and 1.asm from (sample_kernel64.spv is precompiled version from the sample):

$ spirv-dis sample_kernel64.spv > 1.asm

I see that llvm-spirv really creates two functions:

OpName %Test "Test"

with the body of function and the cover-entry-point with the same name:

OpEntryPoint Kernel %19 "Test" %__spirv_BuiltInGlobalInvocationId

which only call the first.

This behavior was added by commit

commit 85815e725ce5bdc970b812b4bbff73d4b2a44046
Author: Dave Airlie <airlied@redhat.com>
Date:   Tue Jan 25 10:17:44 2022 +1000

And I don't know how to fix it by right way.

MrSidims added a commit to MrSidims/SPIRV-LLVM-Translator that referenced this issue Aug 8, 2023
This patch is a result of a reflection about previously merged PR
KhronosGroup#1149
"add an entry point wrapper around functions (llvm pass)"
and is enspired by various reported translator, clang (OpenCL) and Intel
GPU drivers issues (see
KhronosGroup#2029 for
reference).

While SPIR-V spec states:
===
*OpName*
--//--. This has nosemantic impact and can safely be removed from a module.
===
yet having EntryPoint function and a function that shares the name via
OpName might be confusing by both (old) drivers and programmers, who
read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not
necessary to generate it aka if a kernel function is not called by other kernel.

We can do better in other cases as well, for example I have experiments
of renaming a wrapped function adding a previx, so it could be
distinguished from the actual kernel/entry point, but for now it doesn't
pass validation for E2E OpenCL tests.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>
@MrSidims
Copy link
Contributor

MrSidims commented Aug 8, 2023

This PR #2119 should address the issue in 99% of the cases. Rest 1% relates to a situation, when one kernel call another one. I have some internal attempts to make improvements there as well, but they are not yet successful.

MrSidims added a commit that referenced this issue Aug 9, 2023
* Don't wrap kernels that are not being called in the module

This patch is a result of a reflection about previously merged PR
#1149
"add an entry point wrapper around functions (llvm pass)"
and is enspired by various reported translator, clang (OpenCL) and Intel
GPU drivers issues (see
#2029 for
reference).

While SPIR-V spec states:
===
*OpName*
--//--. This has nosemantic impact and can safely be removed from a module.
===
yet having EntryPoint function and a function that shares the name via
OpName might be confusing by both (old) drivers and programmers, who
read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not
necessary to generate it aka if a kernel function is not called by other kernel.

We can do better in other cases as well, for example I have experiments
of renaming a wrapped function adding a previx, so it could be
distinguished from the actual kernel/entry point, but for now it doesn't
pass validation for E2E OpenCL tests.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

* prevent a copy

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

This patch is a result of a reflection about previously merged PR #1149 "add an entry point wrapper around functions (llvm pass)" and is enspired by various reported translator, clang (OpenCL) and Intel GPU drivers issues (see
#2029 for reference).

While SPIR-V spec states:
OpName
--//--. This has nosemantic impact and can safely be removed from a module.
yet having EntryPoint function and a function that shares the name via OpName might be confusing by both not-up-to-date drivers and programmers, who read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not necessary to generate it aka if a kernel function is not called by other kernel.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>
jsji pushed a commit to intel/llvm that referenced this issue Aug 10, 2023
* Don't wrap kernels that are not being called in the module

This patch is a result of a reflection about previously merged PR
KhronosGroup/SPIRV-LLVM-Translator#1149
"add an entry point wrapper around functions (llvm pass)"
and is enspired by various reported translator, clang (OpenCL) and Intel
GPU drivers issues (see
KhronosGroup/SPIRV-LLVM-Translator#2029 for
reference).

While SPIR-V spec states:
===
*OpName*
--//--. This has nosemantic impact and can safely be removed from a module.
===
yet having EntryPoint function and a function that shares the name via
OpName might be confusing by both (old) drivers and programmers, who
read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not
necessary to generate it aka if a kernel function is not called by other kernel.

We can do better in other cases as well, for example I have experiments
of renaming a wrapped function adding a previx, so it could be
distinguished from the actual kernel/entry point, but for now it doesn't
pass validation for E2E OpenCL tests.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

* prevent a copy

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

This patch is a result of a reflection about previously merged PR #1149 "add an entry point wrapper around functions (llvm pass)" and is enspired by various reported translator, clang (OpenCL) and Intel GPU drivers issues (see
#2029 for reference).

While SPIR-V spec states:
OpName
--//--. This has nosemantic impact and can safely be removed from a module.
yet having EntryPoint function and a function that shares the name via OpName might be confusing by both not-up-to-date drivers and programmers, who read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not necessary to generate it aka if a kernel function is not called by other kernel.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@46285e4
jsji pushed a commit to intel/llvm that referenced this issue Aug 11, 2023
* Don't wrap kernels that are not being called in the module

This patch is a result of a reflection about previously merged PR
KhronosGroup/SPIRV-LLVM-Translator#1149
"add an entry point wrapper around functions (llvm pass)"
and is enspired by various reported translator, clang (OpenCL) and Intel
GPU drivers issues (see
KhronosGroup/SPIRV-LLVM-Translator#2029 for
reference).

While SPIR-V spec states:
===
*OpName*
--//--. This has nosemantic impact and can safely be removed from a module.
===
yet having EntryPoint function and a function that shares the name via
OpName might be confusing by both (old) drivers and programmers, who
read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not
necessary to generate it aka if a kernel function is not called by other kernel.

We can do better in other cases as well, for example I have experiments
of renaming a wrapped function adding a previx, so it could be
distinguished from the actual kernel/entry point, but for now it doesn't
pass validation for E2E OpenCL tests.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

* prevent a copy

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

This patch is a result of a reflection about previously merged PR #1149 "add an entry point wrapper around functions (llvm pass)" and is enspired by various reported translator, clang (OpenCL) and Intel GPU drivers issues (see

While SPIR-V spec states:
OpName
--//--. This has nosemantic impact and can safely be removed from a module.
yet having EntryPoint function and a function that shares the name via OpName might be confusing by both not-up-to-date drivers and programmers, who read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not necessary to generate it aka if a kernel function is not called by other kernel.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@46285e4
franz pushed a commit to franz/SPIRV-LLVM-Translator that referenced this issue Jan 25, 2024
…oup#2119)

* Don't wrap kernels that are not being called in the module

This patch is a result of a reflection about previously merged PR
KhronosGroup#1149
"add an entry point wrapper around functions (llvm pass)"
and is enspired by various reported translator, clang (OpenCL) and Intel
GPU drivers issues (see
KhronosGroup#2029 for
reference).

While SPIR-V spec states:
===
*OpName*
--//--. This has nosemantic impact and can safely be removed from a module.
===
yet having EntryPoint function and a function that shares the name via
OpName might be confusing by both (old) drivers and programmers, who
read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not
necessary to generate it aka if a kernel function is not called by other kernel.

We can do better in other cases as well, for example I have experiments
of renaming a wrapped function adding a previx, so it could be
distinguished from the actual kernel/entry point, but for now it doesn't
pass validation for E2E OpenCL tests.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

* prevent a copy

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

This patch is a result of a reflection about previously merged PR KhronosGroup#1149 "add an entry point wrapper around functions (llvm pass)" and is enspired by various reported translator, clang (OpenCL) and Intel GPU drivers issues (see
KhronosGroup#2029 for reference).

While SPIR-V spec states:
OpName
--//--. This has nosemantic impact and can safely be removed from a module.
yet having EntryPoint function and a function that shares the name via OpName might be confusing by both not-up-to-date drivers and programmers, who read the SPIR-V file.

This patch prevents generation of the wrapper function when it's not necessary to generate it aka if a kernel function is not called by other kernel.

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants