[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-23 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 updated this revision to Diff 507922. augusto2112 added a comment. Addressed feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146595/new/ https://reviews.llvm.org/D146595 Files: clang/include/clang/Basic/Attr.td clang/include

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-23 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 added a comment. >>> What would happen if, instead, these trampolining functions were annotated >>> nodebug? I guess then you wouldn't have the top level one as an entry point >>> for the user, as there would be no debug info for it? >> >> These trampoline functions can call other

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D146595#4217855 , @augusto2112 wrote: >> Should there be some check that that symbol exists? (the current test uses >> the unmangled name "bar" for instance - which gives the wrong impression?) > > You mean as part of the at

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-23 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 marked 4 inline comments as done. augusto2112 added a comment. > Should there be some check that that symbol exists? (the current test uses > the unmangled name "bar" for instance - which gives the wrong impression?) You mean as part of the attribute validation? > What would happen

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. >> What about function overloading & namespaces? Is the debugger expected to >> figure out which `bar` function the DWARF/user is referring to? > > The symbol name that's passed in the attribute is supposed to be the mangled > name, so there shouldn't be any ambiguity t

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment. > If it's the latter, then, yeah, some "transparent to debugger" source > attribute might be appropriate - that lowers to a bit on the DISubprogram and > instructs LLVM to, if the function definition ends up lowering to a > trampoline, mark that for the debugger so it's

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 added a comment. > This is quite fragile, for a few reasons. > It's too easy for the user to get undiagnosed typos. e.g., they want to > dispatch to bar but accidentally dispatch to barf or bar() instead. > How does this work with overloaded functions? Templated functions with > spec

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. So looking at the DWARF spec for this - I see what it's going for, but yeah, I wouldn't want to use a textual representation either at the source or bitcode level, ideally... - perhaps this should just be implemented in LLVM when something's being lowered to a trampoli

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Yeah, this seems a bit burdensome for both the frontend (encoding more information, the author has to duplicate the name of the implementation function so there's a risk they get out of sync?) and the debugger (is the debugger expected to jump directly and not call the

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. This revision now requires changes to proceed. I can see some minor utility to this for some kinds of libraries perhaps, but I'm on the fence about adding the attribute. Is

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 updated this revision to Diff 507422. augusto2112 added a comment. Herald added a subscriber: jdoerfert. Fix pragma-attribute-supported-attributes-list.test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146595/new/ https://reviews.llvm.

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 added a comment. In D146595#4213412 , @aprantl wrote: > So this attribute will lower into a `DW_AT_trampoline("target_func_name")` > attribute on the `DW_TAG_subprogram` of the function definition? Exactly, there's already functionality to l

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. So this attribute will lower into a `DW_AT_trampoline("target_func_name")` attribute on the `DW_TAG_subprogram` of the function definition? The debug info parts LGTM. It would be nice to hea

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-21 Thread Augusto Noronha via Phabricator via cfe-commits
augusto2112 created this revision. augusto2112 added reviewers: aprantl, arphaman, jingham. Herald added a subscriber: hiraditya. Herald added a reviewer: aaron.ballman. Herald added a project: All. augusto2112 requested review of this revision. Herald added projects: clang, LLVM. Herald added subs