[PATCH] D58518: [HIP] change kernel stub name

2019-02-26 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL354948: [HIP] change kernel stub name (authored by yaxunl, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D58518

[PATCH] D58518: [HIP] change kernel stub name

2019-02-26 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision. tra added subscribers: jyknight, bkramer. tra added inline comments. This revision is now accepted and ready to land. Comment at: lib/CodeGen/CodeGenModule.cpp:1059 +FD->hasAttr()) + MangledName = MangledName + ".stub"; +

[PATCH] D58518: [HIP] change kernel stub name

2019-02-22 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1059 +FD->hasAttr()) + MangledName = MangledName + ".stub"; + tra wrote: > Changing mangled name exposes this change to a wider scope of potential > issues. Is the mangled name

[PATCH] D58518: [HIP] change kernel stub name

2019-02-22 Thread Artem Belevich via Phabricator via cfe-commits
tra requested changes to this revision. tra added a subscriber: echristo. tra added inline comments. This revision now requires changes to proceed. Comment at: lib/CodeGen/CodeGenModule.cpp:1059 +FD->hasAttr()) + MangledName = MangledName + ".stub"; + ---

[PATCH] D58518: [HIP] change kernel stub name

2019-02-22 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 187980. yaxunl added a comment. Fixed regressions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58518/new/ https://reviews.llvm.org/D58518 Files: lib/CodeGen/CGCUDANV.cpp lib/CodeGen/CodeGenModule.cpp test/CodeGenCUDA/kernel-stub-name.cu In

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D58518#1406274 , @t-tye wrote: > To clarify, I am saying that the stub does have a different name since it is > conceptually part of the implementation of doing the call to the device > function implementation, and is not in fact

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC354615: [HIP] change kernel stub name (authored by yaxunl, committed by ). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D58518?vs=187815&id=187840#toc Repository:

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Tony Tye via Phabricator via cfe-commits
t-tye added a comment. To clarify, I am saying that the stub does have a different name since it is conceptually part of the implementation of doing the call to the device function implementation, and is not in fact the the device function being called itself. However, when we generate code for

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D58518#1406202 , @t-tye wrote: > Yes this relates to supporting the debugger. > > For the same function being present on both host and device, having the same > name is correct as the debugger must set a breakpoint at both places.

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Tony Tye via Phabricator via cfe-commits
t-tye added a comment. Yes this relates to supporting the debugger. For the same function being present on both host and device, having the same name is correct as the debugger must set a breakpoint at both places. This is similar to needing to set a breakpoint at every place a function is inli

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done. yaxunl added a comment. In D58518#1406124 , @tra wrote: > My guess is that this is needed because HIP debugger can see symbols from > both host and device executables at the same time. Is that so? > > If that's the

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. My guess is that this is needed because HIP debugger can see symbols from both host and device executables at the same time. Is that so? If that's the case, I guess HIP may have similar naming probl

[PATCH] D58518: [HIP] change kernel stub name

2019-02-21 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision. yaxunl added reviewers: t-tye, tra. Add .stub to kernel stub function name so that it is different from kernel name in device code. This is necessary to let debugger find correct symbol for kernel https://reviews.llvm.org/D58518 Files: lib/CodeGen/CGCUDANV.cpp