xe: enable source code information for OpenCL kernels #2339
Conversation
github-actions
bot
added
platform:gpu-intel
rjoursler
force-pushed
the
rjoursle/debug_ocl
branch
from
eb5c3df to
2f603aa
Compare
rjoursler
force-pushed
the
rjoursle/debug_ocl
branch
from
2f603aa to
d91f4d4
Compare
|
make test |
| @@ -46,6 +46,11 @@ endfunction() | |||
| function(gen_gpu_kernel_list ker_list_templ ker_list_src ker_sources headers) | |||
| set(_sources "${SOURCES}") | |||
|
|
|||
| set(MINIFY "ON") | |||
| if(DNNL_DEV_MODE OR CMAKE_BUILD_TYPE STREQUAL "Debug") | |||
| set(MINIFY "OFF") | |||
There was a problem hiding this comment.
Can this impact compile time or does it impact library size only?
There was a problem hiding this comment.
In testing, I do not see any measurable impact on compilation time.
|
|
||
| const int name_size = 29; | ||
| char name[name_size] = "/tmp/dnnl_ocl_jit_src.XXXXXX"; | ||
| int fd = mkstemp(name); |
There was a problem hiding this comment.
Is creating temporary files this way compatible with our SDL?
| @@ -249,6 +250,9 @@ status_t ocl_gpu_engine_t::build_program_from_source( | |||
| std::string pp_code_str = pp_code.str(); | |||
| const char *pp_code_str_ptr = pp_code_str.c_str(); | |||
|
|
|||
| src = {pp_code_str}; | |||
| if (src) { options += " -g -s " + std::string(src.name()); } | |||
There was a problem hiding this comment.
could you clarify what the -s options does here, I could not find it in opencl list of options.
There was a problem hiding this comment.
The -s specifies the source location to use for debug information. For more details, see https://www.intel.com/content/www/us/en/docs/distribution-for-gdb/tutorial-debugging-dpcpp-linux/2023-0/debugging-an-opencl-application.html.
| @@ -249,6 +250,9 @@ status_t ocl_gpu_engine_t::build_program_from_source( | |||
| std::string pp_code_str = pp_code.str(); | |||
| const char *pp_code_str_ptr = pp_code_str.c_str(); | |||
|
|
|||
| src = {pp_code_str}; | |||
| if (src) { options += " -g -s " + std::string(src.name()); } | |||
There was a problem hiding this comment.
It seems to be the only place where program_src_t is needed.
Why introduce program_src_t, and not do everything in this function directly?
There was a problem hiding this comment.
The goal here is that this file is temporary and is deleted after it is no longer necessary. In particular, this means that this file is necessary so long the resulting compute::kernels is instantiated, so that this file can be referenced by a debugger. The program_src_t structure is largely responsible for handling this lifetime management. We will also need a similar file if we are to introduce debug information for our ir based kernels as currently all nGEN debug information just maps to the code which lowers the IR to nGEN.
As the difficult case for handling nGEN was already handled in #2297, it seems only fitting to enable debug information in the simpler case where we use OpenCL C kernels. This PR limits the scope of support for this to Linux due to the nuances required for handling temporary files. It should be fairly easy to extend with Windows support when it becomes useful.