[PATCH] D58033: Add option for emitting dbg info for call sites

2019-04-17 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro marked 2 inline comments as done.
djtodoro added a comment.

@probinson @aprantl Thanks a lot for your comments!

Let's clarify some things. I'm sorry about the confusion.

Initial patch for the functionality can be restricted by this option (like we 
pushed here), since **LLDB** doesn't read this type of debug info (yet).
The plan is, when  **LLDB** has all necessary info and we are sure that 
everything works fine during using this info in debugging sessions, we can turn 
it to `ON` by default.

Does that make sense ? :)




Comment at: include/clang/Driver/Options.td:921
+   Group,
+   Flags<[CC1Option]>,
+   HelpText<"Enables debug info about call site and call 
site parameters">;

probinson wrote:
> I believe that by marking this with `[CC1Option]` cc1 will automatically 
> understand it and you won't need to define a separate option in CC1Options.td.
I can't remember what problem we occurred with this on 4.0, but we definitely 
don't need both definitions.
Thanks for this!  


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

https://reviews.llvm.org/D58033



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58033: Add option for emitting dbg info for call sites

2019-04-16 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

And if we plan to enable it by default, too, perhaps not adding a driver option 
(CC1 only) is preferable, since we need to maintain compatibility with driver 
options indefinitely.


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

https://reviews.llvm.org/D58033



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58033: Add option for emitting dbg info for call sites

2019-04-16 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

I guess I'm not clear what your final goal is for the option.  Keep it, even 
though GCC doesn't have one like it?  Eliminate it?  Please clearly state what 
you intend to have in the end, and what you might have in the short term in 
case that is different.




Comment at: include/clang/Driver/CC1Options.td:362
 HelpText<"Use public LTO visibility for classes in std and stdext 
namespaces">;
+def emit_param_entry_values_cc1:
+Flag<["-"], "emit-param-entry-values-cc1">,

I think this is not necessary, see comment on Options.td.



Comment at: include/clang/Driver/Options.td:919
   HelpText<"Do not use jump tables for lowering switches">;
+def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">,
+   Group,

djtodoro wrote:
> aprantl wrote:
> > I assume that this is the same -f option that GCC uses?
> Actually in GCC production of call_site and call_site_parameters debug info 
> is enabled by default.
> Production of entry_values is implemented on top of variable's value tracking 
> system and there is no particular option just for entry_values.
> 
> Basically, we introduce this option as experimental one. As soon as we test 
> everything we should get rid of this or turn it ON by default (and maybe add 
> '-fno-emit-param-entry-values' in order to have an option for disabling the 
> functionality). That will be ideal scenario.
By convention, the name of the option should start with 'f' and match the 
option spelling (with hyphens changed to underscore).



Comment at: include/clang/Driver/Options.td:921
+   Group,
+   Flags<[CC1Option]>,
+   HelpText<"Enables debug info about call site and call 
site parameters">;

I believe that by marking this with `[CC1Option]` cc1 will automatically 
understand it and you won't need to define a separate option in CC1Options.td.


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

https://reviews.llvm.org/D58033



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58033: Add option for emitting dbg info for call sites

2019-04-16 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro marked an inline comment as done.
djtodoro added inline comments.



Comment at: include/clang/Driver/Options.td:919
   HelpText<"Do not use jump tables for lowering switches">;
+def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">,
+   Group,

aprantl wrote:
> I assume that this is the same -f option that GCC uses?
Actually in GCC production of call_site and call_site_parameters debug info is 
enabled by default.
Production of entry_values is implemented on top of variable's value tracking 
system and there is no particular option just for entry_values.

Basically, we introduce this option as experimental one. As soon as we test 
everything we should get rid of this or turn it ON by default (and maybe add 
'-fno-emit-param-entry-values' in order to have an option for disabling the 
functionality). That will be ideal scenario.


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

https://reviews.llvm.org/D58033



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58033: Add option for emitting dbg info for call sites

2019-04-15 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments.



Comment at: include/clang/Driver/Options.td:919
   HelpText<"Do not use jump tables for lowering switches">;
+def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">,
+   Group,

I assume that this is the same -f option that GCC uses?


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

https://reviews.llvm.org/D58033



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58033: Add option for emitting dbg info for call sites

2019-04-15 Thread Djordje Todorovic via Phabricator via cfe-commits
djtodoro updated this revision to Diff 195188.
djtodoro edited the summary of this revision.
djtodoro added a comment.

-Rebase
-Add all_call_sites flag in the case of GNU extensions


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

https://reviews.llvm.org/D58033

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/CodeGen/CGDebugInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -746,6 +746,7 @@
 
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
+  Opts.EnableParamEntryValues = Args.hasArg(OPT_emit_param_entry_values_cc1);
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3396,6 +3396,10 @@
   if (DebuggerTuning == llvm::DebuggerKind::SCE)
 CmdArgs.push_back("-dwarf-explicit-import");
 
+  // Enable param entry values functionlaity.
+  if (Args.hasArg(options::OPT_emit_param_entry_values))
+CmdArgs.push_back("-emit-param-entry-values-cc1");
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
 }
 
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -4558,7 +4558,10 @@
   // were part of DWARF v4.
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
-  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
+   (CGM.getCodeGenOpts().EnableParamEntryValues &&
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+
   if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
 return llvm::DINode::FlagZero;
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -916,6 +916,10 @@
 def fjump_tables : Flag<["-"], "fjump-tables">, Group;
 def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, 
Flags<[CC1Option]>,
   HelpText<"Do not use jump tables for lowering switches">;
+def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">,
+   Group,
+   Flags<[CC1Option]>,
+   HelpText<"Enables debug info about call site and call 
site parameters">;
 def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">,
   Group, Flags<[CC1Option]>,
   HelpText<"Enable support for int128_t type">;
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -359,6 +359,9 @@
 def flto_visibility_public_std:
 Flag<["-"], "flto-visibility-public-std">,
 HelpText<"Use public LTO visibility for classes in std and stdext 
namespaces">;
+def emit_param_entry_values_cc1:
+Flag<["-"], "emit-param-entry-values-cc1">,
+HelpText<"Enables debug info about call site and call site parameters">;
 def flto_unit: Flag<["-"], "flto-unit">,
 HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable 
opt)">;
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
Index: include/clang/Basic/CodeGenOptions.def
===
--- include/clang/Basic/CodeGenOptions.def
+++ include/clang/Basic/CodeGenOptions.def
@@ -61,6 +61,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(EnableParamEntryValues, 1, 0) ///< Emit any call site dbg info
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
  ///< is specified.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///< Do not emit tail calls.


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -746,6 +746,7 @@
 
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
+  Opts.EnableParamEntryValues =