[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao updated this revision to Diff 204856. hliao added a comment. Just revise the interface for device kernel stubbing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63335/new/ https://reviews.llvm.org/D63335 Files: clang/lib/CodeGen/CGCUDANV.c

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment. In D63335#1544320 , @tra wrote: > In D63335#1544315 , @hliao wrote: > > > > Sorry, I still don't think I understand the reasons for this change. The > > > stub and the kernel do have a differ

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Artem Belevich via Phabricator via cfe-commits
tra requested changes to this revision. tra added a comment. This revision now requires changes to proceed. In D63335#1544315 , @hliao wrote: > > Sorry, I still don't think I understand the reasons for this change. The > > stub and the kernel do have a di

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D63335#1544026 , @hliao wrote: > Is it OK for us to mangle `__device_stub __` as the nested name into the > original one, says, we prepend `_ZN15__device_stub__E`, so that we have > `_ZN15__device_stub__E10kernelfuncIiEvv` > > and

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment. In D63335#1544311 , @tra wrote: > In D63335#1544019 , @hliao wrote: > > > In D63335#1543854 , @tra wrote: > > > > > In D63335#1543845

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D63335#1544019 , @hliao wrote: > In D63335#1543854 , @tra wrote: > > > In D63335#1543845 , @hliao wrote: > > > > > it's requested from debugger people

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment. In D63335#1543854 , @tra wrote: > In D63335#1543845 , @hliao wrote: > > > it's requested from debugger people. they don't want to the host-side stub > > could match the device-side kernel fun

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment. In D63335#1543854 , @tra wrote: > In D63335#1543845 , @hliao wrote: > > > it's requested from debugger people. they don't want to the host-side stub > > could match the device-side kernel fun

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment. In D63335#1543854 , @tra wrote: > In D63335#1543845 , @hliao wrote: > > > it's requested from debugger people. they don't want to the host-side stub > > could match the device-side kernel fun

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done. hliao added inline comments. Comment at: clang/lib/CodeGen/CGCUDANV.cpp:789 +return Name; + return std::move(("__device_stub__" + Name).str()); +} tra wrote: > I suspect `return "__device_stub__" + Name;` would do. Str

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. In D63335#1543845 , @hliao wrote: > it's requested from debugger people. they don't want to the host-side stub > could match the device-side kernel function name. the previous scheme cannot > prevent that. I understand that you wan

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done. hliao added inline comments. Comment at: clang/lib/CodeGen/CGCUDANV.cpp:222-226 + assert((CGF.CGM.getContext().getAuxTargetInfo() && + (CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI() != + CGF.CGM.getContext().getTa

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done. hliao added a comment. it's requested from debugger people. they don't want to the host-side stub could match the device-side kernel function name. the previous scheme cannot prevent that. Comment at: clang/lib/CodeGen/CGCUDANV.cpp:222-

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. Is there particular reason you need to switch to this naming scheme? One issue with this patch is that demanglers will no longer be able to deal with the name. While they do know to ignore .stub suffix, they can't deal with `__device_stub_` prefix. E.g: % c++filt __devic

[PATCH] D63335: [HIP] Change kernel stub name again

2019-06-14 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision. hliao added reviewers: yaxunl, tra. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Prefix kernel stub with `__device_stub__` to avoid potential symbol name conflicts in debugger. - Revise the interface to derive the stub name and simplify the