[PATCH] D75175: [CallSiteInfo] Enable the call site info only for -g + optimizations

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro marked an inline comment as done.
djtodoro added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:793
     Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
+    Opts.EmitCallSiteInfo = true;
+  }
----------------
vsk wrote:
> 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?
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.


> Is EmitCallSiteInfo = false supposed to disable TAG_call_site emission?
No, it should restrict only the `callSites` `MF` attribute.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75175/new/

https://reviews.llvm.org/D75175