Skip to content

Conversation

@OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Mar 14, 2025

Key Instructions will start development behind a compile time flag to avoid
passing on the increased memory usage to all debug builds. We're working on
improving DILocation memory characteristics simultaneously; once that work lands
we can remove EXPERIMENTAL_KEY_INSTRUCTIONS.

This patch doesn't add any code, it's just so we can get the SIE buildbot
building with the new option right away.

…nition

Key Instructions will start development behind a compile time flag to avoid
passing on the increased memory usage to all debug builds. We're working on
improving DILocation memory characteristics simultaneously; once that work
lands we can remove `EXPERIMENTAL_KEY_INSTRUCTIONS`.

This patch doesn't add any code, it's just so we can get the SIE buildbot
building with the new option right away.
@OCHyams OCHyams requested review from SLTozer, dyung and jmorse March 14, 2025 15:15
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Mar 14, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 14, 2025

@llvm/pr-subscribers-debuginfo

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

Key Instructions will start development behind a compile time flag to avoid
passing on the increased memory usage to all debug builds. We're working on
improving DILocation memory characteristics simultaneously; once that work lands
we can remove EXPERIMENTAL_KEY_INSTRUCTIONS.

This patch doesn't add any code, it's just so we can get the SIE buildbot
building with the new option right away.


Full diff: https://github.com/llvm/llvm-project/pull/131344.diff

2 Files Affected:

  • (modified) llvm/CMakeLists.txt (+3)
  • (modified) llvm/cmake/modules/HandleLLVMOptions.cmake (+4)
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index c5d3e23a47f0e..18b6ee85fae8d 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -545,6 +545,9 @@ set(LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING "DISABLED" CACHE STRING
   "Enhance Debugify's line number coverage tracking; enabling this is ABI-breaking. Can be DISABLED, or COVERAGE.")
 set_property(CACHE LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING PROPERTY STRINGS DISABLED COVERAGE)
 
+option(LLVM_EXPERIMENTAL_KEY_INSTRUCTIONS
+  "Add additional fields to DILocations to support Key Instructions" OFF)
+
 set(WINDOWS_PREFER_FORWARD_SLASH_DEFAULT OFF)
 if (MINGW)
   # Cygwin doesn't identify itself as Windows, and thus gets path::Style::posix
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 5db06ccdadbeb..c2c04051ef3ce 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -206,6 +206,10 @@ else()
   message(FATAL_ERROR "Unknown value for LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING: \"${LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING}\"!")
 endif()
 
+if(LLVM_EXPERIMENTAL_KEY_INSTRUCTIONS)
+  add_compile_definitions(EXPERIMENTAL_KEY_INSTRUCTIONS)
+endif()
+
 if( LLVM_REVERSE_ITERATION )
   set( LLVM_ENABLE_REVERSE_ITERATION 1 )
 endif()

Copy link
Member

@jmorse jmorse left a comment

Choose a reason for hiding this comment

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

LGTM; this is how the RemoveDIs prototype was implemented, hopefully like RemoveDIs it'll avoid un-necessary interference with other folks while still getting test coverage.

@OCHyams
Copy link
Contributor Author

OCHyams commented Mar 17, 2025

I hope it's not too pushy in landing this, as it's not clear the RFC for key instructions is "accepted". This can easily be reverted; I'm just trying to set up the testing path.

@OCHyams OCHyams merged commit f4feab9 into llvm:main Mar 17, 2025
14 checks passed
chapuni added a commit that referenced this pull request Jul 11, 2025
It has been introduced in #131344 and turned on at #144324
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmake Build system in general and CMake in particular debuginfo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants