[KeyInstr][Clang] Add Clang option -g[no-]key-instructions #134627
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 7, 2025
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Orlando Cazalet-Hyams (OCHyams) ChangesThis needs to be driver level to pass an -mllvm flag to LLVM. Keep the flag help-hidden as the feature is under development. This patch is part of a stack that teaches Clang to generate Key Instructions The Key Instructions project is introduced, including a "quick summary" section The feature is only functional in LLVM if LLVM is built with CMake flag The Clang-side work is demoed here: Full diff: https://github.com/llvm/llvm-project/pull/134627.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def
index bc96d5dfdf890..7a9d2e838c1ca 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -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.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 3af072242d039..68d70af15ca02 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4588,6 +4588,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,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 26fa234dd4e9b..0778db857172c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4767,6 +4767,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");
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("-dwarf-use-key-instructions");
+ }
+
if (EmitCodeView) {
CmdArgs.push_back("-gcodeview");
diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index af3bc3853edfc..0bdce62a8716c 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -23,6 +23,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(
diff --git a/clang/test/KeyInstructions/flag.cpp b/clang/test/KeyInstructions/flag.cpp
new file mode 100644
index 0000000000000..93503dd4bdb4c
--- /dev/null
+++ b/clang/test/KeyInstructions/flag.cpp
@@ -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.
diff --git a/clang/test/KeyInstructions/lit.local.cfg b/clang/test/KeyInstructions/lit.local.cfg
new file mode 100644
index 0000000000000..482bd5c8ac251
--- /dev/null
+++ b/clang/test/KeyInstructions/lit.local.cfg
@@ -0,0 +1,2 @@
+if not config.has_key_instructions:
+ config.unsupported = True
diff --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 80cded2625df4..19fb217c6355f 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -45,6 +45,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)
|
|
@llvm/pr-subscribers-clang-driver Author: Orlando Cazalet-Hyams (OCHyams) ChangesThis needs to be driver level to pass an -mllvm flag to LLVM. Keep the flag help-hidden as the feature is under development. This patch is part of a stack that teaches Clang to generate Key Instructions The Key Instructions project is introduced, including a "quick summary" section The feature is only functional in LLVM if LLVM is built with CMake flag The Clang-side work is demoed here: Full diff: https://github.com/llvm/llvm-project/pull/134627.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def
index bc96d5dfdf890..7a9d2e838c1ca 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -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.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 3af072242d039..68d70af15ca02 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4588,6 +4588,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,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 26fa234dd4e9b..0778db857172c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4767,6 +4767,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");
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("-dwarf-use-key-instructions");
+ }
+
if (EmitCodeView) {
CmdArgs.push_back("-gcodeview");
diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index af3bc3853edfc..0bdce62a8716c 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -23,6 +23,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(
diff --git a/clang/test/KeyInstructions/flag.cpp b/clang/test/KeyInstructions/flag.cpp
new file mode 100644
index 0000000000000..93503dd4bdb4c
--- /dev/null
+++ b/clang/test/KeyInstructions/flag.cpp
@@ -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.
diff --git a/clang/test/KeyInstructions/lit.local.cfg b/clang/test/KeyInstructions/lit.local.cfg
new file mode 100644
index 0000000000000..482bd5c8ac251
--- /dev/null
+++ b/clang/test/KeyInstructions/lit.local.cfg
@@ -0,0 +1,2 @@
+if not config.has_key_instructions:
+ config.unsupported = True
diff --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 80cded2625df4..19fb217c6355f 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -45,6 +45,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)
|
This was referenced Apr 7, 2025
This was referenced Apr 7, 2025
jmorse
reviewed
Apr 23, 2025
There was a problem hiding this comment.
Looks good; I understand we have to support driver flags forever because they get baked into peoples build systems. Would we be able to get away with a cc1 flag instead?
I suppose this strays into the topic of "how are we going to deploy this", which as everything is behind a compile time flag, we probably don't need to immediately answer. IMO we don't have to support driver flags forever that were in turn behind their own experimental compile-time flag.
| @@ -4767,6 +4767,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"); | |||
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Comment on lines
1
to
2
| if not config.has_key_instructions: | ||
| config.unsupported = True |
There was a problem hiding this comment.
Is there anywhere deeper in the clang/test directory we can squirrel this -- most of the existing subdirectories are quite substantive pieces of the compiler, which of course debug-info is too, but possibly not key instructions by itself.
(RIP your future tree conflicts).
There was a problem hiding this comment.
(RIP your future tree conflicts).
Yeah... 😢
I went looking for clang/test/DebugInfo, but that did not exist. Our existing debug info IRgen tests are split by language into CodeGen/CodeGenCXX/CodeGenObj, which is not great organization.
I think clang/test/DebugInfo/KeyInstrs would be a good final home for these tests, and organizing the existing tests can be future (never?) work.
OCHyams
force-pushed
the
users/OCHyams/ki-llvm-dwarf-emission
branch
from
0b206ae to
0b5166a
Compare
This needs to be driver level to pass an -mllvm flag to LLVM. Keep the flag help-hidden as the feature is under development. --- This patch is part of a stack that teaches Clang to generate Key Instructions metadata for C and C++. The Key Instructions project is introduced, including a "quick summary" section at the top which adds context for this PR, here: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668 The feature is only functional in LLVM if LLVM is built with CMake flag LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed. The Clang-side work is demoed here: #130943
OCHyams
force-pushed
the
users/OCHyams/ki-clang-flag
branch
from
7132dd3 to
5c9d5e0
Compare
|
Thanks for the reviews.
@jmorse Does the in-line conversation change your stance on this? If not, it would be easier to make this cc1 after I've uploaded bitcode handling patches, if it's alright to have it in this state temporarily to keep things moving - as all the front end patches are blocked on this one. |
jmorse
approved these changes
May 19, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/15511 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/3698 Here is the relevant piece of the build log for the reference |
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this pull request
May 23, 2025
This needs to be driver level to pass an -mllvm flag to LLVM, though this may change soon as the -mllvm flag will soon not be necessary. Keep the flag help-hidden as the feature is under development. This patch is part of a stack that teaches Clang to generate Key Instructions metadata for C and C++. RFC: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668 The feature is only functional in LLVM if LLVM is built with CMake flag LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed.
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this pull request
May 23, 2025
This needs to be driver level to pass an -mllvm flag to LLVM, though this may change soon as the -mllvm flag will soon not be necessary. Keep the flag help-hidden as the feature is under development. This patch is part of a stack that teaches Clang to generate Key Instructions metadata for C and C++. RFC: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668 The feature is only functional in LLVM if LLVM is built with CMake flag LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This needs to be driver level to pass an -mllvm flag to LLVM.
Keep the flag help-hidden as the feature is under development.
This patch is part of a stack that teaches Clang to generate Key Instructions
metadata for C and C++.
The Key Instructions project is introduced, including a "quick summary" section
at the top which adds context for this PR, here:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
The feature is only functional in LLVM if LLVM is built with CMake flag
LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed.
The Clang-side work is demoed here:
#130943