ychen marked 2 inline comments as done. ychen added a comment. Thanks a lot for reviewing the patch!
In D118428#3281721 <https://reviews.llvm.org/D118428#3281721>, @aganea wrote: > Cool! :) Seems good generally. > Sounds like an expensive flag for the runtime perf :-D But I guess it makes > the debugging experience a bit nicer. Exactly. I had the same thoughts. For MSVC it is what it is. For ELF, the other choice from using the same scheme as MSVC is that we could inline the `__CheckForDebuggerJustMyCode` call (its definition is here: https://github.com/ojdkbuild/tools_toolchain_vs2017bt_1416/blob/master/VC/Tools/MSVC/14.16.27023/crt/src/vcruntime/debugger_jmc.c) before the function ABI prologue. For example, to instrument a function `_foo`: andb r11l, _jmc_global_flag, _jmc_enable_per_thread_step andb r11l, r11, _jmc_per_module_flag andb r11l, r11, _jmc_per_file_flag cmpb r11l, 0 jne _foo int 3 _foo: The parameter of `__CheckForDebuggerJustMyCode` is `_jmc_per_file_flag` above. `int 3` is basically a per-function flag. `_jmc_per_module_flag` and `_jmc_global_flag` control the JMC behavior at module-level or program-level for better performance. Otherwise, the debugger needs the gather this information using debuginfo. This scheme is target-dependent and increases the .text slightly. But it eliminates the call and the flag modification at different granularity is fast. With that being said, experts working on ELF debuggers have ideas about which scheme is better. I'll send an RFC for ELF after this patch goes in. ================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7457 + /*Default=*/false)) { + if (EnabledZ7) + CmdArgs.push_back("-fjmc"); ---------------- aganea wrote: > Can we do here `if (*EmitCodeView && *DebugInfoKind >= > codegenoptions::DebugInfoConstructor)`? Is it worth introducing a new > variable? Yeah, I think that works better. No need for a new variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118428/new/ https://reviews.llvm.org/D118428 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
