[PATCH] D58033: Add option for emitting dbg info for call sites
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
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
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
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
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
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 =