Emit call site info only in the case of '-g' + 'O>0' level.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
As I mentioned on D74904, I think it makes sense to avoiding production of call site info in the case of non-debug builds, since it is being used only for Debug Entry Values feature.
The Call Site Info potentially could be used by some other LLVM modules, but if we find such case, we can enable it for that purpose as well.
The separate option makes sense to me, since the Call Site Info is conceptually different thing, and the Debug Entry Values feature just uses the Call Site Info utility.
If this seems reasonable, I will rebase the re-landing of D73534 on top of this.
One downside with this is that if you compile the compilation unit containing the caller with -O0, and the compilation unit containing the callee with optimizations enabled, it will not be possible to evaluate the entry values in the callee. However, GCC (8.3.0) seems to behave in the same way; at least for the small examples I tried.
llvm/include/llvm/Target/TargetOptions.h | ||
---|---|---|
261 | Nit: optimize -> optimized | |
266 | What do you think about keeping these as two separate things, and query one of EmitCallSiteInfo / EnableDebugEntryValues at each use? I think that it might make it easier to understand the code if they are treated as separate throughout the compiler rather than being merged here. |
Seems reasonable.
clang/include/clang/Basic/CodeGenOptions.def | ||
---|---|---|
68 | I think this comment should go into the help text for the command line parameter instead? |
Yes, but this will restrict only call-site-info (callSites: MF attribute) production. The DWARF (tags for call_sites and call_site_params) was already controlled by the DISubprogram flag (all-calls-described), so I think with or without this that will be the same?
clang/include/clang/Basic/CodeGenOptions.def | ||
---|---|---|
68 | That sounds good to me. Thanks! |
llvm/include/llvm/Target/TargetOptions.h | ||
---|---|---|
266 | It is good idea actually! thanks! |
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
1437 | Is this a replacement for the EnableDebugEntryValues codegenopt in clang? |
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
1437 | It is not a full replacement, since it applies only to the Call Site Info. It could end up acting similar to EnableDebugEntryValues, but it is conceptually different. But it should go above where the EnableDebugEntryValues was set. |
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
1437 | But I would keep the EnableDebugEntryValue Target option as well, if that was the question. |
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
793 | I don't understand how Opts.EnableDebugEntryValues and Opts.EmitCallSiteInfo are different. What is the second option supposed to control, that the first option does not? I thought we could already suppress entry values in llvm at -O0 + -g. Is EmitCallSiteInfo = false supposed to disable TAG_call_site emission? |
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
793 | I'll try to describe the difference between Call Site Info and entry values. Entry values (call-site-params and dbg-values with dw_op_entry_val) are features from debug info. The call-site-params are extension of the call-site (TAG_call_site) debug info. Since the emission of the TAG_call_site was already restricted to -O>0 + -g in the constructCallSiteEntryDIEs() (there is a code checking the existence of the AllCallsDescribed), that implies the collectCallSiteParameters() will be called for that combination only (since it is being called in the constructCallSiteEntryDIEs()). Further on, the collectCallSiteParameters() will call the describeLoadedValue(), which is target-dependent method, and it is implemented only for x86, arm and aarch64 targets at the moment, so we need to restrict the emission of call-site-params only to the targets. The Call Site Info is utility helping us implementing the entry values. We did not have the information about arguments forwarding (necessary for implementing the describeLoadedValue() by searching for the values loaded into the forwarding registers), so the conclusion was to add such info as MF attribute (callSites:). In addition, there were thoughts the attribute could be used by other places in LLVM, but currently it is being used only by Debug Entry Values feature. My understanding is the MF attribute is different thing to the Debug Entry Values, and we can not restrict the production of such attribute by looking at the Debug Info Metadata (for example by looking at the AllCallsDescribed flag), so that is why I added the additional flag.
No, it should restrict only the callSites MF attribute. |
llvm/include/llvm/CodeGen/CommandFlags.inc | ||
---|---|---|
280 | Let's not use a wording like Currently, it will start looking awkward soon. "Emit call site debug information, if debug information is enabled." | |
282 | Again, this text can't be in LLVM since it is Clang's decision when to enable this. (At least I'm assuming that it is Clang :-). A different fronted could conceivably make different choices. |
Sorry for the delay here.
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
793 | Thanks for explaining, this sgtm. |
I think this comment should go into the help text for the command line parameter instead?