Add DebugSSAUpdater class to track debug value liveness #135349
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 patch adds a class that uses SSA construction, with debug values as definitions, to determine whether and which debug values for a particular variable are live at each point in an IR function. This will be used by the IR reader of llvm-debuginfo-analyzer to compute variable ranges and coverage, although it may be applicable to other debug info IR analyses.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-debuginfo Author: Stephen Tozer (SLTozer) ChangesThis patch adds a class that uses SSA construction, with debug values as definitions, to determine whether and which debug values for a particular variable are live at each point in an IR function. This will be used by the IR reader of llvm-debuginfo-analyzer to compute variable ranges and coverage, although it may be applicable to other debug info IR analyses. Patch is 37.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135349.diff 8 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 174ff09f56bdf..43246d3615b24 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -4349,6 +4349,7 @@ template <> struct DenseMapInfo<DebugVariable> {
class DebugVariableAggregate : public DebugVariable {
public:
DebugVariableAggregate(const DbgVariableIntrinsic *DVI);
+ DebugVariableAggregate(const DbgVariableRecord *DVR);
DebugVariableAggregate(const DebugVariable &V)
: DebugVariable(V.getVariable(), std::nullopt, V.getInlinedAt()) {}
};
diff --git a/llvm/include/llvm/Transforms/Utils/DebugSSAUpdater.h b/llvm/include/llvm/Transforms/Utils/DebugSSAUpdater.h
new file mode 100644
index 0000000000000..27a56274cd3d9
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/DebugSSAUpdater.h
@@ -0,0 +1,351 @@
+//===- DebugSSAUpdater.h - Debug SSA Update Tool ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares the DebugSSAUpdater class, which is used to evaluate the
+// live values of debug variables in IR. This uses SSA construction, treating
+// debug value records as definitions, to determine at each point in the program
+// which definition(s) are live at a given point. This is useful for analysis of
+// the state of debug variables, such as measuring the change in values of a
+// variable over time, or calculating coverage stats.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_UTILS_DEBUGSSAUPDATER_H
+#define LLVM_TRANSFORMS_UTILS_DEBUGSSAUPDATER_H
+
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/CFG.h"
+#include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/IR/DebugProgramInstruction.h"
+#include "llvm/IR/Instruction.h"
+
+namespace llvm {
+
+////////////////////////////////////////
+// SSAUpdater specialization classes
+
+class DbgSSAPhi;
+template <typename T> class SmallVectorImpl;
+template <typename T> class SSAUpdaterTraits;
+
+/// A definition of a variable; can represent either a debug value, no
+/// definition (the variable has not yet been defined), or a phi value*.
+/// *Meaning multiple definitions that are live-in to a block from different
+/// predecessors, not a debug value that uses an IR PHINode.
+struct DbgValueDef {
+ DbgSSAPhi *Phi;
+ bool IsUndef;
+ bool IsMemory;
+ Metadata *Locations;
+ DIExpression *Expression;
+
+ DbgValueDef()
+ : Phi(nullptr), IsUndef(true), IsMemory(false), Locations(nullptr),
+ Expression(nullptr) {}
+ DbgValueDef(int)
+ : Phi(nullptr), IsUndef(true), IsMemory(false), Locations(nullptr),
+ Expression(nullptr) {}
+ DbgValueDef(bool IsMemory, Metadata *Locations, DIExpression *Expression)
+ : Phi(nullptr), IsUndef(false), IsMemory(IsMemory), Locations(Locations),
+ Expression(Expression) {}
+ DbgValueDef(DbgVariableRecord *DVR) : Phi(nullptr) {
+ assert(!DVR->isDbgAssign() && "#dbg_assign not yet supported");
+ IsUndef = DVR->isKillLocation();
+ IsMemory = DVR->isAddressOfVariable();
+ Locations = DVR->getRawLocation();
+ Expression = DVR->getExpression();
+ }
+ DbgValueDef(DbgSSAPhi *Phi)
+ : Phi(Phi), IsUndef(false), IsMemory(false), Locations(nullptr),
+ Expression(nullptr) {}
+
+ bool agreesWith(DbgValueDef Other) const {
+ if (IsUndef && Other.IsUndef)
+ return true;
+ return std::tie(Phi, IsUndef, IsMemory, Locations, Expression) ==
+ std::tie(Other.Phi, Other.IsUndef, Other.IsMemory, Other.Locations,
+ Other.Expression);
+ }
+
+ operator bool() const { return !IsUndef; }
+ bool operator==(DbgValueDef Other) const { return agreesWith(Other); }
+ bool operator!=(DbgValueDef Other) const { return !agreesWith(Other); }
+
+ void print(raw_ostream &OS) const;
+};
+
+class DbgSSABlock;
+class DebugSSAUpdater;
+
+/// Represents the live-in definitions of a variable to a block with multiple
+/// predecessors.
+class DbgSSAPhi {
+public:
+ SmallVector<std::pair<DbgSSABlock *, DbgValueDef>, 4> IncomingValues;
+ DbgSSABlock *ParentBlock;
+ DbgSSAPhi(DbgSSABlock *ParentBlock) : ParentBlock(ParentBlock) {}
+
+ DbgSSABlock *getParent() { return ParentBlock; }
+ unsigned getNumIncomingValues() const { return IncomingValues.size(); }
+ DbgSSABlock *getIncomingBlock(size_t Idx) {
+ return IncomingValues[Idx].first;
+ }
+ DbgValueDef getIncomingValue(size_t Idx) {
+ return IncomingValues[Idx].second;
+ }
+ void addIncoming(DbgSSABlock *BB, DbgValueDef DV) {
+ IncomingValues.push_back({BB, DV});
+ }
+
+ void print(raw_ostream &OS) const;
+};
+
+inline raw_ostream &operator<<(raw_ostream &OS, const DbgValueDef &DV) {
+ DV.print(OS);
+ return OS;
+}
+inline raw_ostream &operator<<(raw_ostream &OS, const DbgSSAPhi &PHI) {
+ PHI.print(OS);
+ return OS;
+}
+
+/// Thin wrapper around a block successor iterator.
+class DbgSSABlockSuccIterator {
+public:
+ succ_iterator SuccIt;
+ DebugSSAUpdater &Updater;
+
+ DbgSSABlockSuccIterator(succ_iterator SuccIt, DebugSSAUpdater &Updater)
+ : SuccIt(SuccIt), Updater(Updater) {}
+
+ bool operator!=(const DbgSSABlockSuccIterator &OtherIt) const {
+ return OtherIt.SuccIt != SuccIt;
+ }
+
+ DbgSSABlockSuccIterator &operator++() {
+ ++SuccIt;
+ return *this;
+ }
+
+ DbgSSABlock *operator*();
+};
+
+/// Thin wrapper around a block successor iterator.
+class DbgSSABlockPredIterator {
+public:
+ pred_iterator PredIt;
+ DebugSSAUpdater &Updater;
+
+ DbgSSABlockPredIterator(pred_iterator PredIt, DebugSSAUpdater &Updater)
+ : PredIt(PredIt), Updater(Updater) {}
+
+ bool operator!=(const DbgSSABlockPredIterator &OtherIt) const {
+ return OtherIt.PredIt != PredIt;
+ }
+
+ DbgSSABlockPredIterator &operator++() {
+ ++PredIt;
+ return *this;
+ }
+
+ DbgSSABlock *operator*();
+};
+
+class DbgSSABlock {
+public:
+ BasicBlock &BB;
+ DebugSSAUpdater &Updater;
+ using PHIListT = SmallVector<DbgSSAPhi, 1>;
+ /// List of PHIs in this block. There should only ever be one, but this needs
+ /// to be a list for the SSAUpdater.
+ PHIListT PHIList;
+
+ DbgSSABlock(BasicBlock &BB, DebugSSAUpdater &Updater)
+ : BB(BB), Updater(Updater) {}
+
+ DbgSSABlockPredIterator pred_begin() {
+ return DbgSSABlockPredIterator(llvm::pred_begin(&BB), Updater);
+ }
+
+ DbgSSABlockPredIterator pred_end() {
+ return DbgSSABlockPredIterator(llvm::pred_end(&BB), Updater);
+ }
+
+ iterator_range<DbgSSABlockPredIterator> predecessors() {
+ return iterator_range(pred_begin(), pred_end());
+ }
+
+ DbgSSABlockSuccIterator succ_begin() {
+ return DbgSSABlockSuccIterator(llvm::succ_begin(&BB), Updater);
+ }
+
+ DbgSSABlockSuccIterator succ_end() {
+ return DbgSSABlockSuccIterator(llvm::succ_end(&BB), Updater);
+ }
+
+ iterator_range<DbgSSABlockSuccIterator> successors() {
+ return iterator_range(succ_begin(), succ_end());
+ }
+
+ /// SSAUpdater has requested a PHI: create that within this block record.
+ DbgSSAPhi *newPHI() {
+ assert(PHIList.empty() &&
+ "Only one PHI should exist per-block per-variable");
+ PHIList.emplace_back(this);
+ return &PHIList.back();
+ }
+
+ /// SSAUpdater wishes to know what PHIs already exist in this block.
+ PHIListT &phis() { return PHIList; }
+};
+
+/// Class used to determine the live ranges of debug variables in IR using
+/// SSA construction (via the SSAUpdaterImpl class), used for analysis purposes.
+class DebugSSAUpdater {
+ friend class SSAUpdaterTraits<DebugSSAUpdater>;
+
+private:
+ /// This keeps track of which value to use on a per-block basis. When we
+ /// insert PHI nodes, we keep track of them here.
+ void *AV = nullptr;
+
+ SmallVectorImpl<DbgSSAPhi *> *InsertedPHIs;
+
+ DenseMap<BasicBlock *, DbgSSABlock *> BlockMap;
+
+public:
+ /// If InsertedPHIs is specified, it will be filled
+ /// in with all PHI Nodes created by rewriting.
+ explicit DebugSSAUpdater(
+ SmallVectorImpl<DbgSSAPhi *> *InsertedPHIs = nullptr);
+ DebugSSAUpdater(const DebugSSAUpdater &) = delete;
+ DebugSSAUpdater &operator=(const DebugSSAUpdater &) = delete;
+ ~DebugSSAUpdater();
+
+ void reset() {
+ for (auto &Block : BlockMap)
+ delete Block.second;
+
+ if (InsertedPHIs)
+ InsertedPHIs->clear();
+ BlockMap.clear();
+ }
+
+ void initialize();
+
+ /// For a given BB, create a wrapper block for it. Stores it in the
+ /// DebugSSAUpdater block map.
+ DbgSSABlock *getDbgSSABlock(BasicBlock *BB) {
+ auto it = BlockMap.find(BB);
+ if (it == BlockMap.end()) {
+ BlockMap[BB] = new DbgSSABlock(*BB, *this);
+ it = BlockMap.find(BB);
+ }
+ return it->second;
+ }
+
+ /// Indicate that a rewritten value is available in the specified block
+ /// with the specified value.
+ void addAvailableValue(DbgSSABlock *BB, DbgValueDef DV);
+
+ /// Return true if the DebugSSAUpdater already has a value for the specified
+ /// block.
+ bool hasValueForBlock(DbgSSABlock *BB) const;
+
+ /// Return the value for the specified block if the DebugSSAUpdater has one,
+ /// otherwise return nullptr.
+ DbgValueDef findValueForBlock(DbgSSABlock *BB) const;
+
+ /// Construct SSA form, materializing a value that is live at the end
+ /// of the specified block.
+ DbgValueDef getValueAtEndOfBlock(DbgSSABlock *BB);
+
+ /// Construct SSA form, materializing a value that is live in the
+ /// middle of the specified block.
+ ///
+ /// \c getValueInMiddleOfBlock is the same as \c GetValueAtEndOfBlock except
+ /// in one important case: if there is a definition of the rewritten value
+ /// after the 'use' in BB. Consider code like this:
+ ///
+ /// \code
+ /// X1 = ...
+ /// SomeBB:
+ /// use(X)
+ /// X2 = ...
+ /// br Cond, SomeBB, OutBB
+ /// \endcode
+ ///
+ /// In this case, there are two values (X1 and X2) added to the AvailableVals
+ /// set by the client of the rewriter, and those values are both live out of
+ /// their respective blocks. However, the use of X happens in the *middle* of
+ /// a block. Because of this, we need to insert a new PHI node in SomeBB to
+ /// merge the appropriate values, and this value isn't live out of the block.
+ DbgValueDef getValueInMiddleOfBlock(DbgSSABlock *BB);
+
+private:
+ DbgValueDef getValueAtEndOfBlockInternal(DbgSSABlock *BB);
+};
+
+struct DbgRangeEntry {
+ BasicBlock::iterator Start;
+ BasicBlock::iterator End;
+ // Should be non-PHI.
+ DbgValueDef Value;
+};
+
+class DbgValueRangeTable {
+ DenseMap<DebugVariableAggregate, SmallVector<DbgRangeEntry>>
+ OrigVariableValueRangeTable;
+ DenseMap<DebugVariableAggregate, DbgValueDef> OrigSingleLocVariableValueTable;
+ // For the only initial user of this class, the mappings below are useful and
+ // are used in conjunction with the variable value ranges above, thus we track
+ // them as part of the same class. If we have more uses for variable value
+ // range tracking, then the line/variable name mapping should be moved out to
+ // a separate class.
+ DenseMap<BasicBlock::iterator, uint64_t> LineMapping;
+ DenseMap<uint64_t, std::string> VariableNameMapping;
+
+ using VarMapping = DenseMap<Value *, uint64_t>;
+ VarMapping VariableMapping;
+ uint64_t KeyIndex = 0;
+
+public:
+ void addVariable(Function *F, DebugVariableAggregate DVA);
+ bool hasVariableEntry(DebugVariableAggregate DVA) const {
+ return OrigVariableValueRangeTable.contains(DVA) ||
+ OrigSingleLocVariableValueTable.contains(DVA);
+ }
+ bool hasSingleLocEntry(DebugVariableAggregate DVA) const {
+ return OrigSingleLocVariableValueTable.contains(DVA);
+ }
+ ArrayRef<DbgRangeEntry> getVariableRanges(DebugVariableAggregate DVA) {
+ return OrigVariableValueRangeTable[DVA];
+ }
+ DbgValueDef getSingleLoc(DebugVariableAggregate DVA) {
+ return OrigSingleLocVariableValueTable[DVA];
+ }
+
+ void addLine(BasicBlock::iterator I, uint64_t LineAddr) {
+ LineMapping[I] = LineAddr;
+ }
+ uint64_t getLine(BasicBlock::iterator I) {
+ return LineMapping.contains(I) ? LineMapping[I] : (uint64_t)-1;
+ }
+
+ uint64_t addVariableName(Value *V, uint64_t Size);
+ std::string getVariableName(uint64_t Key) {
+ assert(VariableNameMapping.contains(Key) && "Why not here?");
+ return VariableNameMapping[Key];
+ }
+
+ void printValues(DebugVariableAggregate DVA, raw_ostream &OS);
+};
+
+} // end namespace llvm
+
+#endif // LLVM_TRANSFORMS_UTILS_DEBUGSSAUPDATER_H
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index f8c24d896df32..e7e79cd64c954 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -55,6 +55,10 @@ DebugVariableAggregate::DebugVariableAggregate(const DbgVariableIntrinsic *DVI)
: DebugVariable(DVI->getVariable(), std::nullopt,
DVI->getDebugLoc()->getInlinedAt()) {}
+DebugVariableAggregate::DebugVariableAggregate(const DbgVariableRecord *DVR)
+ : DebugVariable(DVR->getVariable(), std::nullopt,
+ DVR->getDebugLoc()->getInlinedAt()) {}
+
DILocation::DILocation(LLVMContext &C, StorageType Storage, unsigned Line,
unsigned Column, ArrayRef<Metadata *> MDs,
bool ImplicitCode)
diff --git a/llvm/lib/Transforms/Utils/CMakeLists.txt b/llvm/lib/Transforms/Utils/CMakeLists.txt
index 78cad0d253be8..7e61f47094151 100644
--- a/llvm/lib/Transforms/Utils/CMakeLists.txt
+++ b/llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -20,6 +20,7 @@ add_llvm_component_library(LLVMTransformUtils
CtorUtils.cpp
CountVisits.cpp
Debugify.cpp
+ DebugSSAUpdater.cpp
DemoteRegToStack.cpp
DXILUpgrade.cpp
EntryExitInstrumenter.cpp
diff --git a/llvm/lib/Transforms/Utils/DebugSSAUpdater.cpp b/llvm/lib/Transforms/Utils/DebugSSAUpdater.cpp
new file mode 100644
index 0000000000000..f646fdea90968
--- /dev/null
+++ b/llvm/lib/Transforms/Utils/DebugSSAUpdater.cpp
@@ -0,0 +1,415 @@
+//===- DebugSSAUpdater.cpp - Debug Variable SSA Update Tool ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the DebugSSAUpdater class.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/DebugSSAUpdater.h"
+#include "llvm/IR/CFG.h"
+#include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/Transforms/Utils/SSAUpdaterImpl.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "debug-ssa-updater"
+
+void DbgValueDef::print(raw_ostream &OS) const {
+ OS << "DbgVal{ ";
+ if (IsUndef) {
+ OS << "undef }";
+ return;
+ }
+ if (Phi) {
+ OS << *Phi << "}";
+ return;
+ }
+ OS << (IsMemory ? "Mem: " : "Def: ") << *Locations << " - " << *Expression
+ << " }";
+}
+
+void DbgSSAPhi::print(raw_ostream &OS) const {
+ OS << "DbgPhi ";
+ for (auto &[BB, DV] : IncomingValues)
+ OS << "[" << BB->BB.getName() << ", " << DV << "] ";
+}
+
+using AvailableValsTy = DenseMap<DbgSSABlock *, DbgValueDef>;
+
+static AvailableValsTy &getAvailableVals(void *AV) {
+ return *static_cast<AvailableValsTy *>(AV);
+}
+
+DebugSSAUpdater::DebugSSAUpdater(SmallVectorImpl<DbgSSAPhi *> *NewPHI)
+ : InsertedPHIs(NewPHI) {}
+
+DebugSSAUpdater::~DebugSSAUpdater() {
+ delete static_cast<AvailableValsTy *>(AV);
+}
+
+void DebugSSAUpdater::initialize() {
+ if (!AV)
+ AV = new AvailableValsTy();
+ else
+ getAvailableVals(AV).clear();
+}
+
+bool DebugSSAUpdater::hasValueForBlock(DbgSSABlock *BB) const {
+ return getAvailableVals(AV).count(BB);
+}
+
+DbgValueDef DebugSSAUpdater::findValueForBlock(DbgSSABlock *BB) const {
+ return getAvailableVals(AV).lookup(BB);
+}
+
+void DebugSSAUpdater::addAvailableValue(DbgSSABlock *BB, DbgValueDef DV) {
+ getAvailableVals(AV)[BB] = DV;
+}
+
+DbgValueDef DebugSSAUpdater::getValueAtEndOfBlock(DbgSSABlock *BB) {
+ DbgValueDef Res = getValueAtEndOfBlockInternal(BB);
+ return Res;
+}
+
+DbgValueDef DebugSSAUpdater::getValueInMiddleOfBlock(DbgSSABlock *BB) {
+ // If there is no definition of the renamed variable in this block, just use
+ // 'getValueAtEndOfBlock' to do our work.
+ if (!hasValueForBlock(BB))
+ return getValueAtEndOfBlock(BB);
+
+ // Otherwise, we have the hard case. Get the live-in values for each
+ // predecessor.
+ SmallVector<std::pair<DbgSSABlock *, DbgValueDef>, 8> PredValues;
+ DbgValueDef SingularValue;
+
+ bool IsFirstPred = true;
+ for (DbgSSABlock *PredBB : BB->predecessors()) {
+ DbgValueDef PredVal = getValueAtEndOfBlock(PredBB);
+ PredValues.push_back(std::make_pair(PredBB, PredVal));
+
+ // Compute SingularValue.
+ if (IsFirstPred) {
+ SingularValue = PredVal;
+ IsFirstPred = false;
+ } else if (!PredVal.agreesWith(SingularValue))
+ SingularValue = DbgValueDef();
+ }
+
+ // If there are no predecessors, just return undef.
+ if (PredValues.empty())
+ return DbgValueDef();
+
+ // Otherwise, if all the merged values are the same, just use it.
+ if (!SingularValue.IsUndef)
+ return SingularValue;
+
+ // Ok, we have no way out, insert a new one now.
+ DbgSSAPhi *InsertedPHI = BB->newPHI();
+
+ // Fill in all the predecessors of the PHI.
+ for (const auto &PredValue : PredValues)
+ InsertedPHI->addIncoming(PredValue.first, PredValue.second);
+
+ // See if the PHI node can be merged to a single value. This can happen in
+ // loop cases when we get a PHI of itself and one other value.
+
+ // If the client wants to know about all new instructions, tell it.
+ if (InsertedPHIs)
+ InsertedPHIs->push_back(InsertedPHI);
+
+ LLVM_DEBUG(dbgs() << " Inserted PHI: " << *InsertedPHI << "\n");
+ return InsertedPHI;
+}
+
+DbgSSABlock *DbgSSABlockSuccIterator::operator*() {
+ return Updater.getDbgSSABlock(*SuccIt);
+}
+DbgSSABlock *DbgSSABlockPredIterator::operator*() {
+ return Updater.getDbgSSABlock(*PredIt);
+}
+
+namespace llvm {
+
+template <> class SSAUpdaterTraits<DebugSSAUpdater> {
+public:
+ using BlkT = DbgSSABlock;
+ using ValT = DbgValueDef;
+ using PhiT = DbgSSAPhi;
+ using BlkSucc_iterator = DbgSSABlockSuccIterator;
+
+ static BlkSucc_iterator BlkSucc_begin(BlkT *BB) { return BB->succ_begin(); }
+ static BlkSucc_iterator BlkSucc_end(BlkT *BB) { return BB->succ_end(); }
+
+ class PHI_iterator {
+ private:
+ DbgSSAPhi *PHI;
+ unsigned Idx;
+
+ public:
+ explicit PHI_iterator(DbgSSAPhi *P) // begin iterator
+ : PHI(P), Idx(0) {}
+ PHI_iterator(DbgSSAPhi *P, bool) // end iterator
+ : PHI(P), Idx(PHI->getNumIncomingValues()) {}
+
+ PHI_iterator &operator++() {
+ ++Idx;
+ return *this;
+ }
+ bool operator==(const PHI_iterator &X) const { return Idx == X.Idx; }
+ bool operator!=(const PHI_iterator &X) const { return !operator==(X); }
+
+ DbgValueDef getIncomingValue() { return PHI->getIncomingValue(Idx); }
+ DbgSSABlock *getIncomingBlock() { return PHI->getIncomingBlock(Idx); }
+ };
+
+ static PHI_iterator PHI_begin(PhiT *PHI) { return PHI_iterator(PHI); }
+ static PHI_iterator PHI_end(PhiT *PHI) { return PHI_iterator(PHI, true); }
+
+ /// FindPredecessorBlocks - Put the predecessors of Info->BB into the Preds
+ /// vector, set Info->NumPreds, and allocate space in Info->Preds.
+ static void FindPredecessorBlocks(DbgSSABlock *BB,
+ SmallVectorImpl<DbgSSABlock *> *Preds) {
+ for (auto PredIt = BB->pred_begin(); PredIt != BB->pred_end(); ++PredIt)
+ Preds->push_back(*PredIt);
+ }
+
+ /// GetPoisonVal - Get an undefined value of the same type as the value
+ /// being handled.
+ static DbgValueDef GetPoisonVal(DbgSSABlock *BB, DebugSSAUpdater *Updater) {
+ return DbgValueDef();
+ }
+
+ /// CreateEmptyPHI - Create a new PHI instruction in the specified block.
+ /// Reserve space for the operands (?) but do not fill them in yet.
+ static DbgSSAPhi *CreateEmptyPHI(DbgSSABloc...
[truncated]
|
CarlosAlbertoEnciso
added a commit
to CarlosAlbertoEnciso/llvm-project
that referenced
this pull request
Apr 11, 2025
Add support for the LLVM IR format and be able to generate logical views. Both textual representation (.ll) and bitcode (.bc) format are supported. Note: This patch requires: Add DebugSSAUpdater class to track debug value liveness llvm#135349
jmorse
reviewed
Apr 29, 2025
There was a problem hiding this comment.
This is looking good with various nits inline. I didn't look at the unit test as I ran out of time. Rhyme.
Various bits here are duplicated from SSAUpdater, which I believe is fine as trying to abstract it further would probably be a false economy. Some kind of comment is in order to signal to readers where it's duplicated from, IMO, not sure where or how to word it though.
| // which definition(s) are live at a given point. This is useful for analysis of | ||
| // the state of debug variables, such as measuring the change in values of a | ||
| // variable over time, or calculating coverage stats. | ||
| // |
There was a problem hiding this comment.
I feel like we should add a rider/warning about how "If you're using this in an optimisation pass you're doing it wrong". i.e., computing the dominance frontiers of the debug variables is expensive and shouldn't be done lightly. Adding such a warning will discourage people from submitting patches to optimisations using DebugSSAUpdater that we then have to reject on the basis of the compile-time cost.
Comment on lines
+33
to
+34
| template <typename T> class SmallVectorImpl; | ||
| template <typename T> class SSAUpdaterTraits; |
There was a problem hiding this comment.
How come the fwd-decl is needed instead of just including SmallVector.h?
| /// insert PHI nodes, we keep track of them here. | ||
| void *AV = nullptr; | ||
|
|
||
| SmallVectorImpl<DbgSSAPhi *> *InsertedPHIs; |
There was a problem hiding this comment.
Wants a docu-comment as a pointer-to-a-vector is moderately abnormal
Comment on lines
+302
to
+303
| DenseMap<DebugVariableAggregate, SmallVector<DbgRangeEntry>> | ||
| OrigVariableValueRangeTable; |
There was a problem hiding this comment.
A map containing a vector will be large; this might be alright as it's not designed to happen during optimisation passes.
| DbgValueDef Value; | ||
| }; | ||
|
|
||
| class DbgValueRangeTable { |
There was a problem hiding this comment.
Wants a docu-comment as to the purpose
| for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange())) { | ||
| if (DVR.getVariable() == Var && | ||
| DVR.getDebugLoc().getInlinedAt() == InlinedAt) { | ||
| assert(!DVR.isDbgAssign() && "No support for #dbg_declare yet."); |
There was a problem hiding this comment.
Assertion message out of date, #dbg_declare is handled immediately below?
Comment on lines
+273
to
+280
| if (!FoundInstructionInScope && I.getDebugLoc()) { | ||
| if (I.getDebugLoc().getInlinedAt() == InlinedAt && | ||
| isContained(cast<DILocalScope>(I.getDebugLoc().getScope()), | ||
| Var->getScope())) { | ||
| FoundInstructionInScope = true; | ||
| HasAnyInstructionsInScope.insert(&BB); | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicate of condition below, just this one runs once per DbgRecord? (Doesn't feel right)
| return; | ||
| } | ||
|
|
||
| // We don't have a single location, so let's have fun with liveness. |
There was a problem hiding this comment.
IMO we should mention "location list" here, as that's what's essentially being built with the code, and will be familiar to the reader. (Although I endorse this comment).
| LLVM_DEBUG(dbgs() << "Create set of ranges with " << BlockDbgRanges.size() | ||
| << " entries!\n"); | ||
| if (!BlockDbgRanges.empty()) | ||
| OrigVariableValueRangeTable[DVA].append(BlockDbgRanges); |
There was a problem hiding this comment.
I feel some kind of move-semantics appending / emplacing / whatever here is going to be worth installing for speed purposes, or just on principle.
| } | ||
| } | ||
|
|
||
| uint64_t DbgValueRangeTable::addVariableName(Value *V, uint64_t Size) { |
There was a problem hiding this comment.
Not called by anything -- is this unused? (It's also unclear what it's doing).
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 patch adds a class that uses SSA construction, with debug values as definitions, to determine whether and which debug values for a particular variable are live at each point in an IR function. This will be used by the IR reader of llvm-debuginfo-analyzer to compute variable ranges and coverage, although it may be applicable to other debug info IR analyses.