On Thu, 30 Jul 2026 05:09:52 +0200 Anton Johansson <[email protected]> wrote:
> Debug information is collected before any non-preserving optimization > pass is ran. Currently, only variable definitions are parsed, mapping a > Value * to a name and typename. What's the goal of this debug info processing? Clearly specify in the commit message and somewhere in the code what's the end purpose. In particular, at this stage, it's unclear whether these are just for debugging purposes (i.e., they are optional) or not. I know it's not optional, but clearly stating it here is helpful. > Signed-off-by: Anton Johansson <[email protected]> > --- > .../helper-to-tcg/include/DebugInfo.hpp | 46 ++++++++++++++++ > .../include/PrepareForOptPass.hpp | 10 ++-- > subprojects/helper-to-tcg/src/Pipeline.cpp | 3 +- > .../PrepareForOptPass/PrepareForOptPass.cpp | 52 +++++++++++++++++++ > 4 files changed, 107 insertions(+), 4 deletions(-) > create mode 100644 subprojects/helper-to-tcg/include/DebugInfo.hpp > > diff --git a/subprojects/helper-to-tcg/include/DebugInfo.hpp > b/subprojects/helper-to-tcg/include/DebugInfo.hpp > new file mode 100644 > index 0000000000..27e545c6b7 > --- /dev/null > +++ b/subprojects/helper-to-tcg/include/DebugInfo.hpp > @@ -0,0 +1,46 @@ > +// > +// Copyright(c) 2026 rev.ng Labs Srl. All Rights Reserved. > +// > +// This program is free software; you can redistribute it and/or modify > +// it under the terms of the GNU General Public License as published by > +// the Free Software Foundation; either version 2 of the License, or > +// (at your option) any later version. > +// > +// This program is distributed in the hope that it will be useful, > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +// GNU General Public License for more details. > +// > +// You should have received a copy of the GNU General Public License > +// along with this program; if not, see <http://www.gnu.org/licenses/>. > +// > + > +#pragma once > + > +#include <llvm/ADT/StringRef.h> > +#include <llvm/IR/ValueMap.h> > + > +namespace llvm { > +class Value; > +} > + > +// StringRefs will refer do debug metadata fields which have the same > +// lifetime as the LLVMContext and survive accross optimizations and > +// possible deletions of functions/variables. > +struct DebugInfo { > + llvm::StringRef VarName; > + llvm::StringRef BaseTypeName; > +}; > + > +using DebugInfoMapTy = llvm::ValueMap<const llvm::Value *, DebugInfo>; > + > +// Helper to get the variable name from debug info associated with a > particular > +// value, or default construct an empty name. > +inline llvm::StringRef getDebugVarName(const DebugInfoMapTy &DM, > + const llvm::Value *V) { > + auto It = DM.find(V); > + if (It != DM.end()) { > + return It->second.VarName; > + } > + return {}; > +} This looks like a class under disguise. Something like... ``` class DebugInfoMap { private: DebugInfoMapTy Map; public: StringRef getDebugVarName(const DebugInfoMapTy &DM, const llvm::Value *V); }; ``` > diff --git a/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp > b/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp > index 08ca9a43bb..43cf77190a 100644 > --- a/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp > +++ b/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp > @@ -17,6 +17,7 @@ > > #pragma once > > +#include "DebugInfo.hpp" > #include "FunctionAnnotation.hpp" > #include <llvm/IR/PassManager.h> > > @@ -29,10 +30,13 @@ > > class PrepareForOptPass : public llvm::PassInfoMixin<PrepareForOptPass> { > AnnotationMapTy &ResultAnnotations; > + DebugInfoMapTy &ResultDebugInfo; > > - public: > - PrepareForOptPass(AnnotationMapTy &ResultAnnotations) > - : ResultAnnotations(ResultAnnotations) {} > +public: > + PrepareForOptPass(AnnotationMapTy &ResultAnnotations, > + DebugInfoMapTy &ResultDebugInfo) > + : ResultAnnotations(ResultAnnotations), > + ResultDebugInfo(ResultDebugInfo) {} > llvm::PreservedAnalyses run(llvm::Module &M, > llvm::ModuleAnalysisManager &MAM); > }; > diff --git a/subprojects/helper-to-tcg/src/Pipeline.cpp > b/subprojects/helper-to-tcg/src/Pipeline.cpp > index 89637eaec6..a11c5fd353 100644 > --- a/subprojects/helper-to-tcg/src/Pipeline.cpp > +++ b/subprojects/helper-to-tcg/src/Pipeline.cpp > @@ -193,7 +193,8 @@ int main(int argc, char **argv) { > // but is correlct in LLVM-terms. > > AnnotationMapTy Annotations; > - MPM.addPass(PrepareForOptPass(Annotations)); > + DebugInfoMapTy DebugInfo; > + MPM.addPass(PrepareForOptPass(Annotations, DebugInfo)); > > { > FunctionPassManager FPM; > diff --git > a/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp > b/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp > index a7bee53582..7a9b954d7c 100644 > --- a/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp > +++ b/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp > @@ -26,6 +26,9 @@ > #include <llvm/ADT/StringSet.h> > #include <llvm/Demangle/Demangle.h> > #include <llvm/IR/Constants.h> > +#if LLVM_VERSION_MAJOR >= 19 > +#include <llvm/IR/DebugProgramInstruction.h> > +#endif > #include <llvm/IR/Function.h> > #include <llvm/IR/Instruction.h> > #include <llvm/IR/Instructions.h> > @@ -288,6 +291,53 @@ static void replaceRetaddrWithUndef(Module &M) { > } > } > > +static void collectDebugInfo(Function &F, DebugInfoMapTy &ResultDebugInfo) { > + StringSet EncounteredNames; > + for (auto &BB : F) { > + for (Instruction &I : BB) { > +#if LLVM_VERSION_MAJOR >= 19 > + for (DbgVariableRecord &DVR : > + filterDbgVars(I.getDbgRecordRange())) { > + if (!DVR.isDbgValue()) { > + continue; > + } > + StringRef BaseType{}; > + StringRef VarName{}; `{}` is not really necessary here. I'd avoid it altogether. You can use it to 0-initialize scalars, but even then I'd just add `= 0`. > + if (auto *Derived = > + > dyn_cast<DIDerivedType>(DVR.getVariable()->getType())) { > + BaseType = Derived->getBaseType()->getName(); > + } > + VarName = DVR.getVariable()->getName(); > + if (ResultDebugInfo.find(DVR.getValue(0)) == > + ResultDebugInfo.end() and > + !EncounteredNames.contains(VarName.data())) { > + ResultDebugInfo[DVR.getValue(0)] = {VarName, BaseType}; > + EncounteredNames.insert(VarName.data()); What's the logic for having a variable name considered only once? It should be documented here. > + } > + } > +#else > + if (I.isDebugOrPseudoInst()) { > + if (const auto *Dbg = dyn_cast<DbgValueInst>(&I)) { > + StringRef BaseType{}; > + StringRef VarName{}; > + if (auto *Derived = dyn_cast<DIDerivedType>( > + Dbg->getVariable()->getType())) { > + BaseType = Derived->getBaseType()->getName(); > + } > + VarName = Dbg->getVariable()->getName(); > + if (ResultDebugInfo.find(Dbg->getValue(0)) == > + ResultDebugInfo.end() and > + !EncounteredNames.contains(VarName.data())) { > + ResultDebugInfo[Dbg->getValue(0)] = {VarName, > BaseType}; > + EncounteredNames.insert(VarName.data()); > + } > + } > + } > +#endif > + } > + } > +} > + > PreservedAnalyses PrepareForOptPass::run(Module &M, > ModuleAnalysisManager &MAM) { > demangleFunctionNames(M); > @@ -305,6 +355,8 @@ PreservedAnalyses PrepareForOptPass::run(Module &M, > if (F.getReturnType()->isStructTy()) { > F.addFnAttr(Attribute::AttrKind::AlwaysInline); > } > + // Populate variable and type names for `Value`s from debug info. > + collectDebugInfo(F, ResultDebugInfo); > } > > return PreservedAnalyses::none(); > -- > 2.52.0 Reviewed-by: Alessandro Di Federico <[email protected]> -- Alessandro Di Federico rev.ng Labs
