Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[KeyInstr][Clang] Add Clang option -g[no-]key-instructions #134627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 20, 2025
Merged

[KeyInstr][Clang] Add Clang option -g[no-]key-instructions #134627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d6e182
[KeyInstr][Clang] Add Clang option -g[no-]key-instructions
OCHyams Mar 31, 2025
5c9d5e0
Move clang/test/KeyInstructions -> clang/test/DebugInfo/KeyInstructions
OCHyams May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions 3 clang/include/clang/Basic/DebugOptions.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ DEBUGOPT(DebugOmitUnreferencedMethods, 1, 0) ///< Omit unreferenced member
BENIGN_ENUM_DEBUGOPT(AssignmentTrackingMode, AssignmentTrackingOpts, 2,
AssignmentTrackingOpts::Disabled)

/// Whether or not to use Key Instructions to determine breakpoint locations.
BENIGN_DEBUGOPT(DebugKeyInstructions, 1, 0)

DEBUGOPT(DebugColumnInfo, 1, 0) ///< Whether or not to use column information
///< in debug info.

Expand Down
6 changes: 6 additions & 0 deletions 6 clang/include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4664,6 +4664,12 @@ def gembed_source : Flag<["-"], "gembed-source">, Group<g_flags_Group>,
def gno_embed_source : Flag<["-"], "gno-embed-source">, Group<g_flags_Group>,
Flags<[NoXarchOption]>,
HelpText<"Restore the default behavior of not embedding source text in DWARF debug sections">;
defm key_instructions : BoolGOption<"key-instructions",
CodeGenOpts<"DebugKeyInstructions">, DefaultFalse,
NegFlag<SetFalse>, PosFlag<SetTrue, [], [],
"Enable Key Instructions, which reduces the jumpiness of optimized code stepping (DWARF only)."
" Requires LLVM built with LLVM_EXPERIMENTAL_KEY_INSTRUCTIONS.">,
BothFlags<[HelpHidden], [ClangOption, CLOption, CC1Option]>>;
def headerpad__max__install__names : Joined<["-"], "headerpad_max_install_names">;
def help : Flag<["-", "--"], "help">,
Visibility<[ClangOption, CC1Option, CC1AsOption,
Expand Down
7 changes: 7 additions & 0 deletions 7 clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4776,6 +4776,13 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
CmdArgs.push_back("-gembed-source");
}

if (Args.hasFlag(options::OPT_gkey_instructions,
options::OPT_gno_key_instructions, false)) {
CmdArgs.push_back("-gkey-instructions");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for passing this down to cc1? If there end up being two options to control things (gkey-instructions in frontend, dwarf-use-key-instructions in backend) one imagines we're setting ourselves up for accidentally turning it off. (On the other hand having one global option might be too complicated?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say "we avoid setting mllvm flags in the Driver" because LLVM option parsing handles duplicate options as an error conflict, rather than "last one wins silently", but I found lots of prior art contradicting that position, so I think you're in the clear:

$ git grep mllvm ../clang/lib/Driver/ | wc -l
100

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not something we encourage. -mllvm flags have significant problems: they're global variables, they go through a separate parser from the regular flag parser, and they usually interact badly with LTO. But in practice, a lot of code does it this way because the "correct" way of adding a flag requires more boilerplate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can ditch the -mllvm soon as it was mostly a convenience for development. I don't have a pull request yet that implements the necessary changes but plan to soon (such changes will necessary for bitcode handling, so that's not a nebulous soon).

It would be preferable to me if we can keep it temporarily (land this as is), if that's not a problem.

CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-dwarf-use-key-instructions");
}

if (EmitCodeView) {
CmdArgs.push_back("-gcodeview");

Expand Down
1 change: 1 addition & 0 deletions 1 clang/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ llvm_canonicalize_cmake_booleans(
PPC_LINUX_DEFAULT_IEEELONGDOUBLE
LLVM_TOOL_LLVM_DRIVER_BUILD
LLVM_INCLUDE_SPIRV_TOOLS_TESTS
LLVM_EXPERIMENTAL_KEY_INSTRUCTIONS
)

configure_lit_site_cfg(
Expand Down
15 changes: 15 additions & 0 deletions 15 clang/test/DebugInfo/KeyInstructions/flag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %clang -### -target x86_64 -c -gdwarf -gkey-instructions %s 2>&1 | FileCheck %s --check-prefixes=KEY-INSTRUCTIONS
// RUN: %clang -### -target x86_64 -c -gdwarf -gno-key-instructions %s 2>&1 | FileCheck %s --check-prefixes=NO-KEY-INSTRUCTIONS
//// Default: Off.
// RUN: %clang -### -target x86_64 -c -gdwarf %s 2>&1 | FileCheck %s --check-prefixes=NO-KEY-INSTRUCTIONS

//// Help hidden.
// RUN %clang --help | FileCheck %s --check-prefix=HELP
// HELP-NOT: key-instructions

// KEY-INSTRUCTIONS: "-gkey-instructions"
// KEY-INSTRUCTIONS: "-mllvm" "-dwarf-use-key-instructions"

// NO-KEY-INSTRUCTIONS-NOT: key-instructions

//// TODO: Add smoke test once some functionality has been added.
2 changes: 2 additions & 0 deletions 2 clang/test/DebugInfo/KeyInstructions/lit.local.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
if not config.has_key_instructions:
config.unsupported = True
1 change: 1 addition & 0 deletions 1 clang/test/lit.site.cfg.py.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ config.ppc_linux_default_ieeelongdouble = @PPC_LINUX_DEFAULT_IEEELONGDOUBLE@
config.have_llvm_driver = @LLVM_TOOL_LLVM_DRIVER_BUILD@
config.spirv_tools_tests = @LLVM_INCLUDE_SPIRV_TOOLS_TESTS@
config.substitutions.append(("%llvm-version-major", "@LLVM_VERSION_MAJOR@"))
config.has_key_instructions = @LLVM_EXPERIMENTAL_KEY_INSTRUCTIONS@

import lit.llvm
lit.llvm.initialize(lit_config, config)
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.