https://github.com/OCHyams updated https://github.com/llvm/llvm-project/pull/134632
>From ae032c5dd3537662508d77bbb447808f52481f5d Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <orlando.hy...@sony.com> Date: Tue, 1 Apr 2025 11:59:24 +0100 Subject: [PATCH 1/4] [KeyInstr][Clang] Add ApplyAtomGroup This is a scoped helper similar to ApplyDebugLocation that creates a new source atom group which instructions can be added to. A source atom is a source construct that is "interesting" for debug stepping purposes. We use an atom group number to track the instruction(s) that implement the functionality for the atom, plus backup instructions/source locations. --- 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: https://github.com/llvm/llvm-project/pull/130943 --- clang/lib/CodeGen/CGDebugInfo.cpp | 119 +++++++++++++++++++++++++++- clang/lib/CodeGen/CGDebugInfo.h | 50 ++++++++++++ clang/lib/CodeGen/CodeGenFunction.h | 14 ++++ 3 files changed, 182 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 2a11eebf1b682..f4c5c57a38b3e 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -43,6 +43,7 @@ #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Instruction.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/Metadata.h" @@ -52,6 +53,7 @@ #include "llvm/Support/SHA1.h" #include "llvm/Support/SHA256.h" #include "llvm/Support/TimeProfiler.h" +#include <cstdint> #include <optional> using namespace clang; using namespace clang::CodeGen; @@ -119,6 +121,114 @@ CGDebugInfo::~CGDebugInfo() { "Region stack mismatch, stack not empty!"); } +void CGDebugInfo::addInstSourceAtomMetadata(llvm::Instruction *I, + uint64_t Group, uint8_t Rank) { + if (!I->getDebugLoc() || Group == 0 || !I->getDebugLoc()->getLine()) + return; + + // Saturate the 3-bit rank. + Rank = std::min<uint8_t>(Rank, 7); + + const llvm::DebugLoc &DL = I->getDebugLoc(); + + // Each instruction can only be attributed to one source atom (a limitation of + // the implementation). If this instruction is already part of a source atom, + // pick the group in which it has highest precedence (lowest rank). + if (DL.get()->getAtomGroup() && DL.get()->getAtomRank() && + DL.get()->getAtomRank() < Rank) { + Group = DL.get()->getAtomGroup(); + Rank = DL.get()->getAtomRank(); + } + + // Update the function-local watermark so we don't reuse this number for + // another atom. + KeyInstructionsInfo.HighestEmittedAtom = + std::max(Group, KeyInstructionsInfo.HighestEmittedAtom); + + // Apply the new DILocation to the instruction. + llvm::DILocation *NewDL = llvm::DILocation::get( + I->getContext(), DL.getLine(), DL.getCol(), DL.getScope(), + DL.getInlinedAt(), DL.isImplicitCode(), Group, Rank); + I->setDebugLoc(NewDL); +}; + +void CGDebugInfo::addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, + llvm::Value *Backup) { + if (!CGM.getCodeGenOpts().DebugKeyInstructions) + return; + + uint64_t Group = KeyInstructionsInfo.CurrentAtom; + if (!Group) + return; + + addInstSourceAtomMetadata(KeyInstruction, Group, /*Rank=*/1); + + llvm::Instruction *BackupI = + llvm::dyn_cast_or_null<llvm::Instruction>(Backup); + if (!BackupI) + return; + + // Add the backup instruction to the group. + addInstSourceAtomMetadata(BackupI, Group, /*Rank=*/2); + + // Look through chains of casts too, as they're probably going to evaporate. + // FIXME: And other nops like zero length geps? + // FIXME: Should use Cast->isNoopCast()? + uint8_t Rank = 3; + while (auto *Cast = dyn_cast<llvm::CastInst>(BackupI)) { + BackupI = dyn_cast<llvm::Instruction>(Cast->getOperand(0)); + if (!BackupI) + break; + addInstSourceAtomMetadata(BackupI, Group, Rank++); + } +} + +void CGDebugInfo::addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, + llvm::Value *Backup) { + if (KeyInstructionsInfo.RetAtomOverride) { + uint64_t CurrentAtom = KeyInstructionsInfo.CurrentAtom; + KeyInstructionsInfo.CurrentAtom = KeyInstructionsInfo.RetAtomOverride; + addInstToCurrentSourceAtom(Ret, Backup); + KeyInstructionsInfo.CurrentAtom = CurrentAtom; + KeyInstructionsInfo.RetAtomOverride = 0; + } else { + auto Grp = ApplyAtomGroup(this); + addInstToCurrentSourceAtom(Ret, Backup); + } +} + +void CGDebugInfo::setRetInstSourceAtomOverride(uint64_t Group) { + assert(KeyInstructionsInfo.RetAtomOverride == 0); + KeyInstructionsInfo.RetAtomOverride = Group; +} + +void CGDebugInfo::completeFunction() { + // Reset the atom group number tracker as the numbers are function-local. + KeyInstructionsInfo.NextAtom = 1; + KeyInstructionsInfo.HighestEmittedAtom = 0; + KeyInstructionsInfo.CurrentAtom = 0; + KeyInstructionsInfo.RetAtomOverride = 0; +} + +ApplyAtomGroup::ApplyAtomGroup(CGDebugInfo *DI) : DI(DI) { + if (!DI) + return; + OriginalAtom = DI->KeyInstructionsInfo.CurrentAtom; + DI->KeyInstructionsInfo.CurrentAtom = DI->KeyInstructionsInfo.NextAtom++; +} + +ApplyAtomGroup::~ApplyAtomGroup() { + if (!DI) + return; + + // We may not have used the group number at all. + DI->KeyInstructionsInfo.NextAtom = + std::min(DI->KeyInstructionsInfo.HighestEmittedAtom + 1, + DI->KeyInstructionsInfo.NextAtom); + + DI->KeyInstructionsInfo.CurrentAtom = OriginalAtom; +} + ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, SourceLocation TemporaryLocation) : CGF(&CGF) { @@ -174,8 +284,15 @@ ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF, llvm::DebugLoc Loc) return; } OriginalLocation = CGF.Builder.getCurrentDebugLocation(); - if (Loc) + if (Loc) { + // Key Instructions: drop the atom group and rank to avoid accidentally + // propagating it around. + if (Loc->getAtomGroup()) + Loc = llvm::DILocation::get(Loc->getContext(), Loc.getLine(), + Loc->getColumn(), Loc->getScope(), + Loc->getInlinedAt(), Loc.isImplicitCode()); CGF.Builder.SetCurrentDebugLocation(std::move(Loc)); + } } ApplyDebugLocation::~ApplyDebugLocation() { diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index b287ce7b92eee..d9081d9bb3ffb 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -58,6 +58,8 @@ class CGBlockInfo; class CGDebugInfo { friend class ApplyDebugLocation; friend class SaveAndRestoreLocation; + friend class ApplyAtomGroup; + CodeGenModule &CGM; const llvm::codegenoptions::DebugInfoKind DebugKind; bool DebugTypeExtRefs; @@ -179,6 +181,17 @@ class CGDebugInfo { /// The key is coroutine real parameters, value is DIVariable in LLVM IR. Param2DILocTy ParamDbgMappings; + /// Key Instructions bookkeeping. + /// Source atoms are identified by a {AtomGroup, InlinedAt} pair, meaning + /// AtomGroup numbers can be repeated across different functions. + struct { + uint64_t NextAtom = 1; + uint64_t HighestEmittedAtom = 0; + uint64_t CurrentAtom = 0; + uint64_t RetAtomOverride = 0; + } KeyInstructionsInfo; + +private: /// Helper functions for getOrCreateType. /// @{ /// Currently the checksum of an interface includes the number of @@ -643,7 +656,30 @@ class CGDebugInfo { llvm::DILocation *CreateSyntheticInlineAt(llvm::DebugLoc Location, StringRef FuncName); + /// Reset internal state. + void completeFunction(); + + /// Add \p KeyInstruction and an optional \p Backup instruction to the + /// current atom group, created using ApplyAtomGroup. + void addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, + llvm::Value *Backup); + + /// Add \p Ret and an optional \p Backup instruction to the + /// saved override used for some ret instructions if it exists, or a new atom. + void addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, + llvm::Value *Backup); + + /// Set an atom group override for use in addRetToOverrideOrNewSourceAtom. + void setRetInstSourceAtomOverride(uint64_t Group); + private: + /// Amend \p I's DebugLoc with \p Group (its source atom group) and \p + /// Rank (lower nonzero rank is higher precedence). Does nothing if \p I + /// has no DebugLoc, and chooses the atom group in which the instruction + /// has the highest precedence if it's already in one. + void addInstSourceAtomMetadata(llvm::Instruction *I, uint64_t Group, + uint8_t Rank); + /// Emit call to llvm.dbg.declare for a variable declaration. /// Returns a pointer to the DILocalVariable associated with the /// llvm.dbg.declare, or nullptr otherwise. @@ -860,6 +896,20 @@ class CGDebugInfo { } }; +/// A scoped helper to set the current source atom group for +/// CGDebugInfo::addInstToCurrentSourceAtom. A source atom is a source construct +/// that is "interesting" for debug stepping purposes. We use an atom group +/// number to track the instruction(s) that implement the functionality for the +/// atom, plus backup instructions/source locations. +class ApplyAtomGroup { + uint64_t OriginalAtom = 0; + CGDebugInfo *DI = nullptr; + +public: + ApplyAtomGroup(CGDebugInfo *DI); + ~ApplyAtomGroup(); +}; + /// A scoped helper to set the current debug location to the specified /// location or preferred location of the specified Expr. class ApplyDebugLocation { diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 603e38d763aa4..aec29d4ea61e8 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1740,6 +1740,20 @@ class CodeGenFunction : public CodeGenTypeCache { /// recently incremented counter. uint64_t getCurrentProfileCount() { return PGO.getCurrentRegionCount(); } + /// See CGDebugInfo::addInstToCurrentSourceAtom. + void addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, + llvm::Value *Backup) { + if (CGDebugInfo *DI = getDebugInfo()) + DI->addInstToCurrentSourceAtom(KeyInstruction, Backup); + } + + /// See CGDebugInfo::addRetToOverrideOrNewSourceAtom. + void addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, + llvm::Value *Backup) { + if (CGDebugInfo *DI = getDebugInfo()) + DI->addRetToOverrideOrNewSourceAtom(Ret, Backup); + } + private: /// SwitchInsn - This is nearest current switch instruction. It is null if /// current context is not in a switch. >From 69995e53f547279c887b498a6597043a1d4a8be5 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <orlando.hy...@sony.com> Date: Tue, 20 May 2025 17:09:56 +0100 Subject: [PATCH 2/4] fix rebase fallout (CGDebugInfo.h removed from this header) --- clang/lib/CodeGen/CodeGenFunction.cpp | 12 ++++++++++++ clang/lib/CodeGen/CodeGenFunction.h | 10 ++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 4e79cdf0ef089..092346b627e6a 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -3326,3 +3326,15 @@ CodeGenFunction::EmitPointerAuthAuth(const CGPointerAuthInfo &PointerAuth, return EmitPointerAuthCommon(*this, PointerAuth, Pointer, llvm::Intrinsic::ptrauth_auth); } + +void CodeGenFunction::addInstToCurrentSourceAtom( + llvm::Instruction *KeyInstruction, llvm::Value *Backup) { + if (CGDebugInfo *DI = getDebugInfo()) + DI->addInstToCurrentSourceAtom(KeyInstruction, Backup); +} + +void CodeGenFunction::addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, + llvm::Value *Backup) { + if (CGDebugInfo *DI = getDebugInfo()) + DI->addRetToOverrideOrNewSourceAtom(Ret, Backup); +} \ No newline at end of file diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index aec29d4ea61e8..ecd0362dd6f15 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1742,17 +1742,11 @@ class CodeGenFunction : public CodeGenTypeCache { /// See CGDebugInfo::addInstToCurrentSourceAtom. void addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, - llvm::Value *Backup) { - if (CGDebugInfo *DI = getDebugInfo()) - DI->addInstToCurrentSourceAtom(KeyInstruction, Backup); - } + llvm::Value *Backup); /// See CGDebugInfo::addRetToOverrideOrNewSourceAtom. void addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, - llvm::Value *Backup) { - if (CGDebugInfo *DI = getDebugInfo()) - DI->addRetToOverrideOrNewSourceAtom(Ret, Backup); - } + llvm::Value *Backup); private: /// SwitchInsn - This is nearest current switch instruction. It is null if >From 8e7f643e9d6bef7a54f48c33c7938611fe86d497 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <orlando.hy...@sony.com> Date: Tue, 20 May 2025 17:38:29 +0100 Subject: [PATCH 3/4] replace the ret-override mechanisms with addInstToSpecificSourceAtom --- clang/lib/CodeGen/CGDebugInfo.cpp | 31 ++++++--------------------- clang/lib/CodeGen/CGDebugInfo.h | 12 ++++------- clang/lib/CodeGen/CodeGenFunction.cpp | 6 +++--- clang/lib/CodeGen/CodeGenFunction.h | 6 +++--- 4 files changed, 17 insertions(+), 38 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index f4c5c57a38b3e..1cc2c55a19f11 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -154,11 +154,14 @@ void CGDebugInfo::addInstSourceAtomMetadata(llvm::Instruction *I, void CGDebugInfo::addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, llvm::Value *Backup) { - if (!CGM.getCodeGenOpts().DebugKeyInstructions) - return; + addInstToSpecificSourceAtom(KeyInstruction, Backup, + KeyInstructionsInfo.CurrentAtom); +} - uint64_t Group = KeyInstructionsInfo.CurrentAtom; - if (!Group) +void CGDebugInfo::addInstToSpecificSourceAtom(llvm::Instruction *KeyInstruction, + llvm::Value *Backup, + uint64_t Group) { + if (!Group || !CGM.getCodeGenOpts().DebugKeyInstructions) return; addInstSourceAtomMetadata(KeyInstruction, Group, /*Rank=*/1); @@ -183,31 +186,11 @@ void CGDebugInfo::addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, } } -void CGDebugInfo::addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, - llvm::Value *Backup) { - if (KeyInstructionsInfo.RetAtomOverride) { - uint64_t CurrentAtom = KeyInstructionsInfo.CurrentAtom; - KeyInstructionsInfo.CurrentAtom = KeyInstructionsInfo.RetAtomOverride; - addInstToCurrentSourceAtom(Ret, Backup); - KeyInstructionsInfo.CurrentAtom = CurrentAtom; - KeyInstructionsInfo.RetAtomOverride = 0; - } else { - auto Grp = ApplyAtomGroup(this); - addInstToCurrentSourceAtom(Ret, Backup); - } -} - -void CGDebugInfo::setRetInstSourceAtomOverride(uint64_t Group) { - assert(KeyInstructionsInfo.RetAtomOverride == 0); - KeyInstructionsInfo.RetAtomOverride = Group; -} - void CGDebugInfo::completeFunction() { // Reset the atom group number tracker as the numbers are function-local. KeyInstructionsInfo.NextAtom = 1; KeyInstructionsInfo.HighestEmittedAtom = 0; KeyInstructionsInfo.CurrentAtom = 0; - KeyInstructionsInfo.RetAtomOverride = 0; } ApplyAtomGroup::ApplyAtomGroup(CGDebugInfo *DI) : DI(DI) { diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index d9081d9bb3ffb..10739194fd70f 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -188,7 +188,6 @@ class CGDebugInfo { uint64_t NextAtom = 1; uint64_t HighestEmittedAtom = 0; uint64_t CurrentAtom = 0; - uint64_t RetAtomOverride = 0; } KeyInstructionsInfo; private: @@ -664,13 +663,10 @@ class CGDebugInfo { void addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, llvm::Value *Backup); - /// Add \p Ret and an optional \p Backup instruction to the - /// saved override used for some ret instructions if it exists, or a new atom. - void addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, - llvm::Value *Backup); - - /// Set an atom group override for use in addRetToOverrideOrNewSourceAtom. - void setRetInstSourceAtomOverride(uint64_t Group); + /// Add \p KeyInstruction and an optional \p Backup instruction to the atom + /// group \p Atom. + void addInstToSpecificSourceAtom(llvm::Instruction *KeyInstruction, + llvm::Value *Backup, uint64_t Atom); private: /// Amend \p I's DebugLoc with \p Group (its source atom group) and \p diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 092346b627e6a..30bcf782c71e4 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -3333,8 +3333,8 @@ void CodeGenFunction::addInstToCurrentSourceAtom( DI->addInstToCurrentSourceAtom(KeyInstruction, Backup); } -void CodeGenFunction::addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, - llvm::Value *Backup) { +void CodeGenFunction::addInstToSpecificSourceAtom( + llvm::Instruction *KeyInstruction, llvm::Value *Backup, uint64_t Atom) { if (CGDebugInfo *DI = getDebugInfo()) - DI->addRetToOverrideOrNewSourceAtom(Ret, Backup); + DI->addInstToSpecificSourceAtom(KeyInstruction, Backup, Atom); } \ No newline at end of file diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index ecd0362dd6f15..1743f558b37e9 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1744,9 +1744,9 @@ class CodeGenFunction : public CodeGenTypeCache { void addInstToCurrentSourceAtom(llvm::Instruction *KeyInstruction, llvm::Value *Backup); - /// See CGDebugInfo::addRetToOverrideOrNewSourceAtom. - void addRetToOverrideOrNewSourceAtom(llvm::ReturnInst *Ret, - llvm::Value *Backup); + /// See CGDebugInfo::addInstToSpecificSourceAtom. + void addInstToSpecificSourceAtom(llvm::Instruction *KeyInstruction, + llvm::Value *Backup, uint64_t Atom); private: /// SwitchInsn - This is nearest current switch instruction. It is null if >From ca03a49ab660472728ea3b3e951bf319b8f24f92 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <orlando.hy...@sony.com> Date: Tue, 20 May 2025 17:43:23 +0100 Subject: [PATCH 4/4] whitespace --- clang/lib/CodeGen/CodeGenFunction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 30bcf782c71e4..2256cc08e2212 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -3337,4 +3337,4 @@ void CodeGenFunction::addInstToSpecificSourceAtom( llvm::Instruction *KeyInstruction, llvm::Value *Backup, uint64_t Atom) { if (CGDebugInfo *DI = getDebugInfo()) DI->addInstToSpecificSourceAtom(KeyInstruction, Backup, Atom); -} \ No newline at end of file +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
