-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add the mangled_args property to kernel_api types. #1443
Conversation
1379e63
to
7490c0c
Compare
@mpuputti The PR has my latest GDB-related changes I have done based on our triaging of the GDB support on numba-dpex kernels. The changes do fix setting breakpoints on function names. I will investigate further if there are any unintended side effects. |
0f64099
to
19d42ba
Compare
@@ -119,6 +120,39 @@ class SPIRVDevice(GPU): | |||
target_registry[SPIRV_TARGET_NAME] = SPIRVDevice | |||
|
|||
|
|||
class _DIBuilder(debuginfo.DIBuilder): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be something specific to numba dpex is a better choice for naming?
class _DIBuilder(debuginfo.DIBuilder): | |
class SPIRVDIBuilder(debuginfo.DIBuilder): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the class into numba_dpex.core.debuginfo
and renamed it to DIBuilder
.
@@ -190,7 +190,7 @@ def foo(item, a, b): | |||
kernel_ir = kcres.library.get_llvm_str() | |||
|
|||
for tag in ir_tags: | |||
assert make_check(kernel_ir, tag) | |||
assert not make_check(kernel_ir, tag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was it add to bypass test or there was a bug here? Change looks suspicious)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After these changes the linkagename
tag is no longer used. So changed the test to a negative test case to make sure linkagename
is not set. I have added comments make the change clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome changes! Thank you!
Finally breakpoints by name are working! Just few nits, otherwise good to go!
19d42ba
to
71cc99f
Compare
Add the mangled_args property to kernel_api types. 72a3167
Add the mangled_args property to kernel_api types. 72a3167
Numba-dpex's custom types such as
ItemType
andNdItemType
are currently not properly demangled and makes the demangled function signature hard to read. The PR fixes the issue by adding themangled_args
property to these types.In addition, the
SpirvKernelTarget
was updated to use thenumba_dpex.core.utils.itanium_mangler
.Before (look at the first argument "Item"):
After:
Fixes #1242