From 90b4b7c793ff8284364820d18e7d17c13cf7b97b Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Tue, 18 Mar 2025 16:50:17 +0000 Subject: [PATCH 1/6] [KeyInstr] Add Atom Group waterline to LLVMContext The waterline is managed through DILocation creation, LLVMContext::incNextAtomGroup, and LLVMContext::updateAtomGroupWaterline. Add unittest. --- llvm/include/llvm/IR/LLVMContext.h | 8 +++++++ llvm/lib/IR/DebugInfoMetadata.cpp | 3 +++ llvm/lib/IR/LLVMContext.cpp | 8 +++++++ llvm/lib/IR/LLVMContextImpl.h | 10 ++++++++ llvm/unittests/IR/MetadataTest.cpp | 38 ++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+) diff --git a/llvm/include/llvm/IR/LLVMContext.h b/llvm/include/llvm/IR/LLVMContext.h index bbd125fd38cf1..d3cdd31e0b12f 100644 --- a/llvm/include/llvm/IR/LLVMContext.h +++ b/llvm/include/llvm/IR/LLVMContext.h @@ -335,6 +335,14 @@ class LLVMContext { StringRef getDefaultTargetFeatures(); void setDefaultTargetFeatures(StringRef Features); + /// Key Instructions: update the highest number atom group emitted for any + /// function. + void updateAtomGroupWaterline(uint64_t G); + + /// Key Instructions: get the next free atom group number and increment + /// the global tracker. + uint64_t incNextAtomGroup(); + private: // Module needs access to the add/removeModule methods. friend class Module; diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index e8a58f12e3e57..7c7033e398c40 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -74,6 +74,9 @@ DILocation::DILocation(LLVMContext &C, StorageType Storage, unsigned Line, #ifdef EXPERIMENTAL_KEY_INSTRUCTIONS assert(AtomRank <= 7 && "AtomRank number should fit in 3 bits"); #endif + if (AtomRank) + C.updateAtomGroupWaterline(AtomGroup + 1); + assert((MDs.size() == 1 || MDs.size() == 2) && "Expected a scope and optional inlined-at"); // Set line and column. diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp index 447e5d92e0b99..4781085b30431 100644 --- a/llvm/lib/IR/LLVMContext.cpp +++ b/llvm/lib/IR/LLVMContext.cpp @@ -377,3 +377,11 @@ StringRef LLVMContext::getDefaultTargetFeatures() { void LLVMContext::setDefaultTargetFeatures(StringRef Features) { pImpl->DefaultTargetFeatures = Features; } + +void LLVMContext::updateAtomGroupWaterline(uint64_t V) { + pImpl->NextAtomGroup = std::max(pImpl->NextAtomGroup, V); +} + +uint64_t LLVMContext::incNextAtomGroup() { + return pImpl->NextAtomGroup++; +} diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h index 5c2b5cd3a19cc..21f5c06ea24f3 100644 --- a/llvm/lib/IR/LLVMContextImpl.h +++ b/llvm/lib/IR/LLVMContextImpl.h @@ -1880,6 +1880,16 @@ class LLVMContextImpl { std::string DefaultTargetCPU; std::string DefaultTargetFeatures; + + /// The next available source atom group number. The front end is responsible + /// for assigning source atom numbers, but certain optimisations need to + /// assign new group numbers to a set of instructions. Most often code + /// duplication optimisations like loop unroll. Tracking a global maximum + /// value means we can know (cheaply) we're never using a group number that's + /// already used within this function. + /// + /// Start a 1 because 0 means the source location isn't part of an atom group. + uint64_t NextAtomGroup = 1; }; } // end namespace llvm diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp index 45860d60d8f4c..8e5b34d0ed8be 100644 --- a/llvm/unittests/IR/MetadataTest.cpp +++ b/llvm/unittests/IR/MetadataTest.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "../lib/IR/LLVMContextImpl.h" #include "llvm/IR/Metadata.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" @@ -1591,6 +1592,43 @@ TEST_F(DILocationTest, discriminatorSpecialCases) { EXPECT_EQ(std::nullopt, L4->cloneByMultiplyingDuplicationFactor(0x1000)); } +TEST_F(DILocationTest, KeyInstructions) { + Context.pImpl->NextAtomGroup = 1; + + EXPECT_EQ(Context.pImpl->NextAtomGroup, 1u); + DILocation *A1 = DILocation::get(Context, 1, 0, getSubprogram(), nullptr, false, 1, 2); + // The group is only applied to the DILocation if the build has opted into + // the additional DILocation fields needed for the feature. +#ifdef EXPERIMENTAL_KEY_INSTRUCTIONS + EXPECT_EQ(A1->getAtomGroup(), 1u); + EXPECT_EQ(A1->getAtomRank(), 2u); +#else + EXPECT_EQ(A1->getAtomGroup(), 0u); + EXPECT_EQ(A1->getAtomRank(), 0u); +#endif + + // Group number 1 has been "used" so next available is 2. + EXPECT_EQ(Context.pImpl->NextAtomGroup, 2u); + + // Set a group number higher than current + 1, then check the waterline. + DILocation::get(Context, 2, 0, getSubprogram(), nullptr, false, 5, 1); + EXPECT_EQ(Context.pImpl->NextAtomGroup, 6u); + + // The waterline should be unchanged (group <= next). + DILocation::get(Context, 3, 0, getSubprogram(), nullptr, false, 4, 1); + EXPECT_EQ(Context.pImpl->NextAtomGroup, 6u); + DILocation::get(Context, 3, 0, getSubprogram(), nullptr, false, 5, 1); + EXPECT_EQ(Context.pImpl->NextAtomGroup, 6u); + + // Check the waterline gets incremented by 1. + EXPECT_EQ(Context.incNextAtomGroup(), 6u); + EXPECT_EQ(Context.pImpl->NextAtomGroup, 7u); + + Context.updateAtomGroupWaterline(8); + EXPECT_EQ(Context.pImpl->NextAtomGroup, 8u); + Context.updateAtomGroupWaterline(7); + EXPECT_EQ(Context.pImpl->NextAtomGroup, 8u); +} typedef MetadataTest GenericDINodeTest; From 09900e23082bbbf406b492b43fc14cf6f508e0be Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 1 May 2025 13:19:36 +0100 Subject: [PATCH 2/6] clang-format --- llvm/lib/IR/LLVMContext.cpp | 4 +--- llvm/unittests/IR/MetadataTest.cpp | 5 +++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp index 4781085b30431..5a11c28671409 100644 --- a/llvm/lib/IR/LLVMContext.cpp +++ b/llvm/lib/IR/LLVMContext.cpp @@ -382,6 +382,4 @@ void LLVMContext::updateAtomGroupWaterline(uint64_t V) { pImpl->NextAtomGroup = std::max(pImpl->NextAtomGroup, V); } -uint64_t LLVMContext::incNextAtomGroup() { - return pImpl->NextAtomGroup++; -} +uint64_t LLVMContext::incNextAtomGroup() { return pImpl->NextAtomGroup++; } diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp index 8e5b34d0ed8be..c405e3e0df59a 100644 --- a/llvm/unittests/IR/MetadataTest.cpp +++ b/llvm/unittests/IR/MetadataTest.cpp @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#include "../lib/IR/LLVMContextImpl.h" #include "llvm/IR/Metadata.h" +#include "../lib/IR/LLVMContextImpl.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" #include "llvm/IR/Constants.h" @@ -1596,7 +1596,8 @@ TEST_F(DILocationTest, KeyInstructions) { Context.pImpl->NextAtomGroup = 1; EXPECT_EQ(Context.pImpl->NextAtomGroup, 1u); - DILocation *A1 = DILocation::get(Context, 1, 0, getSubprogram(), nullptr, false, 1, 2); + DILocation *A1 = + DILocation::get(Context, 1, 0, getSubprogram(), nullptr, false, 1, 2); // The group is only applied to the DILocation if the build has opted into // the additional DILocation fields needed for the feature. #ifdef EXPERIMENTAL_KEY_INSTRUCTIONS From e2c671e6a6a38297e8e24c3a76ae6c55529f93ce Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 1 May 2025 13:40:03 +0100 Subject: [PATCH 3/6] rename functions --- llvm/include/llvm/IR/LLVMContext.h | 4 ++-- llvm/lib/IR/DebugInfoMetadata.cpp | 2 +- llvm/lib/IR/LLVMContext.cpp | 6 ++++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/IR/LLVMContext.h b/llvm/include/llvm/IR/LLVMContext.h index d3cdd31e0b12f..6dde7a968b570 100644 --- a/llvm/include/llvm/IR/LLVMContext.h +++ b/llvm/include/llvm/IR/LLVMContext.h @@ -337,11 +337,11 @@ class LLVMContext { /// Key Instructions: update the highest number atom group emitted for any /// function. - void updateAtomGroupWaterline(uint64_t G); + void updateDILocationAtomGroupWaterline(uint64_t G); /// Key Instructions: get the next free atom group number and increment /// the global tracker. - uint64_t incNextAtomGroup(); + uint64_t incNextDILocationAtomGroup(); private: // Module needs access to the add/removeModule methods. diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index 7c7033e398c40..bddc1ae3c2fcb 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -75,7 +75,7 @@ DILocation::DILocation(LLVMContext &C, StorageType Storage, unsigned Line, assert(AtomRank <= 7 && "AtomRank number should fit in 3 bits"); #endif if (AtomRank) - C.updateAtomGroupWaterline(AtomGroup + 1); + C.updateDILocationAtomGroupWaterline(AtomGroup + 1); assert((MDs.size() == 1 || MDs.size() == 2) && "Expected a scope and optional inlined-at"); diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp index 5a11c28671409..57532cd491dd6 100644 --- a/llvm/lib/IR/LLVMContext.cpp +++ b/llvm/lib/IR/LLVMContext.cpp @@ -378,8 +378,10 @@ void LLVMContext::setDefaultTargetFeatures(StringRef Features) { pImpl->DefaultTargetFeatures = Features; } -void LLVMContext::updateAtomGroupWaterline(uint64_t V) { +void LLVMContext::updateDILocationAtomGroupWaterline(uint64_t V) { pImpl->NextAtomGroup = std::max(pImpl->NextAtomGroup, V); } -uint64_t LLVMContext::incNextAtomGroup() { return pImpl->NextAtomGroup++; } +uint64_t LLVMContext::incNextDILocationAtomGroup() { + return pImpl->NextAtomGroup++; +} From a4623976afa3504c4695a69aaa9e0c9ef8e616fb Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 1 May 2025 13:41:48 +0100 Subject: [PATCH 4/6] fix my fixme comment --- llvm/lib/IR/DebugInfoMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index bddc1ae3c2fcb..0973fcf94ee90 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -74,7 +74,7 @@ DILocation::DILocation(LLVMContext &C, StorageType Storage, unsigned Line, #ifdef EXPERIMENTAL_KEY_INSTRUCTIONS assert(AtomRank <= 7 && "AtomRank number should fit in 3 bits"); #endif - if (AtomRank) + if (AtomGroup) C.updateDILocationAtomGroupWaterline(AtomGroup + 1); assert((MDs.size() == 1 || MDs.size() == 2) && From 25b323f8f10d65dadd2c9cc4c54c50cda43e4c1f Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 1 May 2025 13:46:57 +0100 Subject: [PATCH 5/6] simplify comment --- llvm/unittests/IR/MetadataTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp index c405e3e0df59a..786a5afd3dd9e 100644 --- a/llvm/unittests/IR/MetadataTest.cpp +++ b/llvm/unittests/IR/MetadataTest.cpp @@ -1598,8 +1598,8 @@ TEST_F(DILocationTest, KeyInstructions) { EXPECT_EQ(Context.pImpl->NextAtomGroup, 1u); DILocation *A1 = DILocation::get(Context, 1, 0, getSubprogram(), nullptr, false, 1, 2); - // The group is only applied to the DILocation if the build has opted into - // the additional DILocation fields needed for the feature. + // The group is only applied to the DILocation if we've built LLVM with + // EXPERIMENTAL_KEY_INSTRUCTIONS. #ifdef EXPERIMENTAL_KEY_INSTRUCTIONS EXPECT_EQ(A1->getAtomGroup(), 1u); EXPECT_EQ(A1->getAtomRank(), 2u); From 93fa11d350e4358dfd9eb268df8eeeea7f173365 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 1 May 2025 14:37:38 +0100 Subject: [PATCH 6/6] update function names in unittests too --- llvm/unittests/IR/MetadataTest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp index 786a5afd3dd9e..9b25d0b91abdd 100644 --- a/llvm/unittests/IR/MetadataTest.cpp +++ b/llvm/unittests/IR/MetadataTest.cpp @@ -1622,12 +1622,12 @@ TEST_F(DILocationTest, KeyInstructions) { EXPECT_EQ(Context.pImpl->NextAtomGroup, 6u); // Check the waterline gets incremented by 1. - EXPECT_EQ(Context.incNextAtomGroup(), 6u); + EXPECT_EQ(Context.incNextDILocationAtomGroup(), 6u); EXPECT_EQ(Context.pImpl->NextAtomGroup, 7u); - Context.updateAtomGroupWaterline(8); + Context.updateDILocationAtomGroupWaterline(8); EXPECT_EQ(Context.pImpl->NextAtomGroup, 8u); - Context.updateAtomGroupWaterline(7); + Context.updateDILocationAtomGroupWaterline(7); EXPECT_EQ(Context.pImpl->NextAtomGroup, 8u); }