[RemoveDI][NFC] Rename DPValue->DbgRecord in comments and varnames #84939
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
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-debuginfo Author: Stephen Tozer (SLTozer) ChangesThis patch continues the ongoing rename work, replacing DPValue with DbgRecord in comments and the names of variables, both members and fn-local. This is the most labour-intensive part of the rename, as it is where the most decisions have to be made about whether a given comment or variable is referring to DPValues (equivalent to debug variable intrinsics) or DbgRecords (a catch-all for all debug intrinsics); these decisions are not individually difficult, but comprise a fairly large amount of text to review. This patch still largely performs basic string substitutions followed by clang-format; there are almost* no places where, for example, a comment has been expanded or modified to reflect the semantic difference between DPValues and DbgRecords. I don't believe such a change is generally necessary in LLVM, but it may be useful in the docs, and so I'll be submitting docs changes as a separate patch. *In a few places, Patch is 83.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84939.diff 24 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h b/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
index bfac54a65c5b4e..29f675b2203b6b 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
@@ -205,7 +205,7 @@ class IRTranslator : public MachineFunctionPass {
bool translate(const Constant &C, Register Reg);
/// Examine any debug-info attached to the instruction (in the form of
- /// DPValues) and translate it.
+ /// DbgRecords) and translate it.
void translateDbgInfo(const Instruction &Inst,
MachineIRBuilder &MIRBuilder);
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 5bac113c9b7b42..71c1a8394896b7 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -78,13 +78,13 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
DPMarker *createMarker(InstListType::iterator It);
/// Convert variable location debugging information stored in dbg.value
- /// intrinsics into DPMarker / DPValue records. Deletes all dbg.values in
+ /// intrinsics into DPMarkers / DbgRecords. Deletes all dbg.values in
/// the process and sets IsNewDbgInfoFormat = true. Only takes effect if
/// the UseNewDbgInfoFormat LLVM command line option is given.
void convertToNewDbgValues();
/// Convert variable location debugging information stored in DPMarkers and
- /// DPValues into the dbg.value intrinsic representation. Sets
+ /// DbgRecords into the dbg.value intrinsic representation. Sets
/// IsNewDbgInfoFormat = false.
void convertFromNewDbgValues();
@@ -93,50 +93,50 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// if necessary.
void setIsNewDbgInfoFormat(bool NewFlag);
- /// Record that the collection of DPValues in \p M "trails" after the last
+ /// Record that the collection of DbgRecords in \p M "trails" after the last
/// instruction of this block. These are equivalent to dbg.value intrinsics
/// that exist at the end of a basic block with no terminator (a transient
/// state that occurs regularly).
void setTrailingDbgRecords(DPMarker *M);
- /// Fetch the collection of DPValues that "trail" after the last instruction
+ /// Fetch the collection of DbgRecords that "trail" after the last instruction
/// of this block, see \ref setTrailingDbgRecords. If there are none, returns
/// nullptr.
DPMarker *getTrailingDbgRecords();
- /// Delete any trailing DPValues at the end of this block, see
+ /// Delete any trailing DbgRecords at the end of this block, see
/// \ref setTrailingDbgRecords.
void deleteTrailingDbgRecords();
void dumpDbgValues() const;
- /// Return the DPMarker for the position given by \p It, so that DPValues can
- /// be inserted there. This will either be nullptr if not present, a DPMarker,
- /// or TrailingDPValues if It is end().
+ /// Return the DPMarker for the position given by \p It, so that DbgRecords
+ /// can be inserted there. This will either be nullptr if not present, a
+ /// DPMarker, or TrailingDbgRecords if It is end().
DPMarker *getMarker(InstListType::iterator It);
/// Return the DPMarker for the position that comes after \p I. \see
/// BasicBlock::getMarker, this can be nullptr, a DPMarker, or
- /// TrailingDPValues if there is no next instruction.
+ /// TrailingDbgRecords if there is no next instruction.
DPMarker *getNextMarker(Instruction *I);
- /// Insert a DPValue into a block at the position given by \p I.
+ /// Insert a DbgRecord into a block at the position given by \p I.
void insertDbgRecordAfter(DbgRecord *DPV, Instruction *I);
- /// Insert a DPValue into a block at the position given by \p Here.
+ /// Insert a DbgRecord into a block at the position given by \p Here.
void insertDbgRecordBefore(DbgRecord *DPV, InstListType::iterator Here);
- /// Eject any debug-info trailing at the end of a block. DPValues can
+ /// Eject any debug-info trailing at the end of a block. DbgRecords can
/// transiently be located "off the end" of a block if the blocks terminator
/// is temporarily removed. Once a terminator is re-inserted this method will
- /// move such DPValues back to the right place (ahead of the terminator).
- void flushTerminatorDbgValues();
+ /// move such DbgRecords back to the right place (ahead of the terminator).
+ void flushTerminatorDbgRecords();
/// In rare circumstances instructions can be speculatively removed from
/// blocks, and then be re-inserted back into that position later. When this
/// happens in RemoveDIs debug-info mode, some special patching-up needs to
/// occur: inserting into the middle of a sequence of dbg.value intrinsics
- /// does not have an equivalent with DPValues.
+ /// does not have an equivalent with DbgRecords.
void reinsertInstInDbgRecords(Instruction *I,
std::optional<DbgRecord::self_iterator> Pos);
@@ -522,7 +522,7 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
BasicBlock::iterator FromEndIt);
/// Perform any debug-info specific maintenence for the given splice
- /// activity. In the DPValue debug-info representation, debug-info is not
+ /// activity. In the DbgRecord debug-info representation, debug-info is not
/// in instructions, and so it does not automatically move from one block
/// to another.
void spliceDebugInfo(BasicBlock::iterator ToIt, BasicBlock *FromBB,
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 507b652feeb016..1afc9259241d50 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -1,4 +1,4 @@
-//===-- llvm/DebugProgramInstruction.h - Stream of debug info -------*- C++ -*-===//
+//===-- llvm/DebugProgramInstruction.h - Stream of debug info ---*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -15,10 +15,10 @@
// %bar = void call @ext(%foo);
//
// and all information is stored in the Value / Metadata hierachy defined
-// elsewhere in LLVM. In the "DPValue" design, each instruction /may/ have a
-// connection with a DPMarker, which identifies a position immediately before the
-// instruction, and each DPMarker /may/ then have connections to DPValues which
-// record the variable assignment information. To illustrate:
+// elsewhere in LLVM. In the "DbgRecord" design, each instruction /may/ have a
+// connection with a DPMarker, which identifies a position immediately before
+// the instruction, and each DPMarker /may/ then have connections to DbgRecords
+// which record the variable assignment information. To illustrate:
//
// %foo = add i32 1, %0
// ; foo->DbgMarker == nullptr
@@ -26,7 +26,7 @@
// ;; the instruction for %foo, therefore it has no DbgMarker.
// %bar = void call @ext(%foo)
// ; bar->DbgMarker = {
-// ; StoredDPValues = {
+// ; StoredDbgRecords = {
// ; DPValue(metadata i32 %foo, ...)
// ; }
// ; }
@@ -119,7 +119,7 @@ template <typename T> class DbgRecordParamRef {
/// Base class for non-instruction debug metadata records that have positions
/// within IR. Features various methods copied across from the Instruction
/// class to aid ease-of-use. DbgRecords should always be linked into a
-/// DPMarker's StoredDPValues list. The marker connects a DbgRecord back to
+/// DPMarker's StoredDbgRecords list. The marker connects a DbgRecord back to
/// it's position in the BasicBlock.
///
/// We need a discriminator for dyn/isa casts. In order to avoid paying for a
@@ -557,8 +557,8 @@ class DPMarker {
/// intrinsics. There is a one-to-one relationship between each debug
/// intrinsic in a block and each DbgRecord once the representation has been
/// converted, and the ordering is meaningful in the same way.
- simple_ilist<DbgRecord> StoredDPValues;
- bool empty() const { return StoredDPValues.empty(); }
+ simple_ilist<DbgRecord> StoredDbgRecords;
+ bool empty() const { return StoredDbgRecords.empty(); }
const BasicBlock *getParent() const;
BasicBlock *getParent();
@@ -576,54 +576,56 @@ class DPMarker {
void print(raw_ostream &O, bool IsForDebug = false) const;
void print(raw_ostream &ROS, ModuleSlotTracker &MST, bool IsForDebug) const;
- /// Produce a range over all the DPValues in this Marker.
+ /// Produce a range over all the DbgRecords in this Marker.
iterator_range<simple_ilist<DbgRecord>::iterator> getDbgRecordRange();
iterator_range<simple_ilist<DbgRecord>::const_iterator>
getDbgRecordRange() const;
- /// Transfer any DPValues from \p Src into this DPMarker. If \p InsertAtHead
- /// is true, place them before existing DPValues, otherwise afterwards.
+ /// Transfer any DbgRecords from \p Src into this DPMarker. If \p InsertAtHead
+ /// is true, place them before existing DbgRecords, otherwise afterwards.
void absorbDebugValues(DPMarker &Src, bool InsertAtHead);
- /// Transfer the DPValues in \p Range from \p Src into this DPMarker. If
- /// \p InsertAtHead is true, place them before existing DPValues, otherwise
+ /// Transfer the DbgRecords in \p Range from \p Src into this DPMarker. If
+ /// \p InsertAtHead is true, place them before existing DbgRecords, otherwise
// afterwards.
void absorbDebugValues(iterator_range<DbgRecord::self_iterator> Range,
DPMarker &Src, bool InsertAtHead);
- /// Insert a DPValue into this DPMarker, at the end of the list. If
+ /// Insert a DbgRecord into this DPMarker, at the end of the list. If
/// \p InsertAtHead is true, at the start.
void insertDbgRecord(DbgRecord *New, bool InsertAtHead);
- /// Insert a DPValue prior to a DPValue contained within this marker.
+ /// Insert a DbgRecord prior to a DbgRecord contained within this marker.
void insertDbgRecord(DbgRecord *New, DbgRecord *InsertBefore);
- /// Insert a DPValue after a DPValue contained within this marker.
+ /// Insert a DbgRecord after a DbgRecord contained within this marker.
void insertDbgRecordAfter(DbgRecord *New, DbgRecord *InsertAfter);
/// Clone all DPMarkers from \p From into this marker. There are numerous
/// options to customise the source/destination, due to gnarliness, see class
/// comment.
- /// \p FromHere If non-null, copy from FromHere to the end of From's DPValues
- /// \p InsertAtHead Place the cloned DPValues at the start of StoredDPValues
- /// \returns Range over all the newly cloned DPValues
+ /// \p FromHere If non-null, copy from FromHere to the end of From's
+ /// DbgRecords
+ /// \p InsertAtHead Place the cloned DbgRecords at the start of
+ /// StoredDbgRecords
+ /// \returns Range over all the newly cloned DbgRecords
iterator_range<simple_ilist<DbgRecord>::iterator>
cloneDebugInfoFrom(DPMarker *From,
std::optional<simple_ilist<DbgRecord>::iterator> FromHere,
bool InsertAtHead = false);
- /// Erase all DPValues in this DPMarker.
+ /// Erase all DbgRecords in this DPMarker.
void dropDbgRecords();
/// Erase a single DbgRecord from this marker. In an ideal future, we would
/// never erase an assignment in this way, but it's the equivalent to
/// erasing a debug intrinsic from a block.
void dropOneDbgRecord(DbgRecord *DR);
- /// We generally act like all llvm Instructions have a range of DPValues
+ /// We generally act like all llvm Instructions have a range of DbgRecords
/// attached to them, but in reality sometimes we don't allocate the DPMarker
- /// to save time and memory, but still have to return ranges of DPValues. When
- /// we need to describe such an unallocated DPValue range, use this static
- /// markers range instead. This will bite us if someone tries to insert a
- /// DPValue in that range, but they should be using the Official (TM) API for
- /// that.
+ /// to save time and memory, but still have to return ranges of DbgRecords.
+ /// When we need to describe such an unallocated DbgRecord range, use this
+ /// static markers range instead. This will bite us if someone tries to insert
+ /// a DbgRecord in that range, but they should be using the Official (TM) API
+ /// for that.
static DPMarker EmptyDPMarker;
static iterator_range<simple_ilist<DbgRecord>::iterator>
getEmptyDbgRecordRange() {
- return make_range(EmptyDPMarker.StoredDPValues.end(),
- EmptyDPMarker.StoredDPValues.end());
+ return make_range(EmptyDPMarker.StoredDbgRecords.end(),
+ EmptyDPMarker.StoredDbgRecords.end());
}
};
@@ -632,7 +634,7 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DPMarker &Marker) {
return OS;
}
-/// Inline helper to return a range of DPValues attached to a marker. It needs
+/// Inline helper to return a range of DbgRecords attached to a marker. It needs
/// to be inlined as it's frequently called, but also come after the declaration
/// of DPMarker. Thus: it's pre-declared by users like Instruction, then an
/// inlineable body defined here.
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 817abd6afbcae0..d6cf1557752386 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -64,47 +64,48 @@ class Instruction : public User,
/// Clone any debug-info attached to \p From onto this instruction. Used to
/// copy debugging information from one block to another, when copying entire
- /// blocks. \see DebugProgramInstruction.h , because the ordering of DPValues
- /// is still important, fine grain control of which instructions are moved and
- /// where they go is necessary.
+ /// blocks. \see DebugProgramInstruction.h , because the ordering of
+ /// DbgRecords is still important, fine grain control of which instructions
+ /// are moved and where they go is necessary.
/// \p From The instruction to clone debug-info from.
- /// \p from_here Optional iterator to limit DPValues cloned to be a range from
+ /// \p from_here Optional iterator to limit DbgRecords cloned to be a range
+ /// from
/// from_here to end().
- /// \p InsertAtHead Whether the cloned DPValues should be placed at the end
- /// or the beginning of existing DPValues attached to this.
- /// \returns A range over the newly cloned DPValues.
+ /// \p InsertAtHead Whether the cloned DbgRecords should be placed at the end
+ /// or the beginning of existing DbgRecords attached to this.
+ /// \returns A range over the newly cloned DbgRecords.
iterator_range<simple_ilist<DbgRecord>::iterator> cloneDebugInfoFrom(
const Instruction *From,
std::optional<simple_ilist<DbgRecord>::iterator> FromHere = std::nullopt,
bool InsertAtHead = false);
- /// Return a range over the DPValues attached to this instruction.
+ /// Return a range over the DbgRecords attached to this instruction.
iterator_range<simple_ilist<DbgRecord>::iterator> getDbgRecordRange() const {
return llvm::getDbgRecordRange(DbgMarker);
}
- /// Return an iterator to the position of the "Next" DPValue after this
+ /// Return an iterator to the position of the "Next" DbgRecord after this
/// instruction, or std::nullopt. This is the position to pass to
/// BasicBlock::reinsertInstInDbgRecords when re-inserting an instruction.
std::optional<simple_ilist<DbgRecord>::iterator> getDbgReinsertionPosition();
- /// Returns true if any DPValues are attached to this instruction.
+ /// Returns true if any DbgRecords are attached to this instruction.
bool hasDbgRecords() const;
- /// Transfer any DPValues on the position \p It onto this instruction,
- /// by simply adopting the sequence of DPValues (which is efficient) if
+ /// Transfer any DbgRecords on the position \p It onto this instruction,
+ /// by simply adopting the sequence of DbgRecords (which is efficient) if
/// possible, by merging two sequences otherwise.
void adoptDbgRecords(BasicBlock *BB, InstListType::iterator It,
bool InsertAtHead);
- /// Erase any DPValues attached to this instruction.
+ /// Erase any DbgRecords attached to this instruction.
void dropDbgRecords();
- /// Erase a single DPValue \p I that is attached to this instruction.
+ /// Erase a single DbgRecord \p I that is attached to this instruction.
void dropOneDbgRecord(DbgRecord *I);
/// Handle the debug-info implications of this instruction being removed. Any
- /// attached DPValues need to "fall" down onto the next instruction.
+ /// attached DbgRecords need to "fall" down onto the next instruction.
void handleMarkerRemoval();
protected:
diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h
index c03d49c3b7b978..ec8b809d40bfbc 100644
--- a/llvm/include/llvm/IR/PassManager.h
+++ b/llvm/include/llvm/IR/PassManager.h
@@ -227,7 +227,7 @@ class PassManager : public PassInfoMixin<
detail::getAnalysisResult<PassInstrumentationAnalysis>(
AM, IR, std::tuple<ExtraArgTs...>(ExtraArgs...));
- // RemoveDIs: if requested, convert debug-info to DPValue representation
+ // RemoveDIs: if requested, convert debug-info to DbgRecord representation
// for duration of these passes.
bool ShouldConvertDbgInfo = shouldConvertDbgInfo(IR);
if (ShouldConvertDbgInfo)
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp
index 93fb2a821dee74..0eb9c246f2a9b6 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp
@@ -19,7 +19,7 @@
using namespace llvm;
PreservedAnalyses BitcodeWriterPass::run(Module &M, ModuleAnalysisManager &AM) {
- // RemoveDIs: there's no bitcode representation of the DPValue debug-info,
+ // RemoveDIs: there's no bitcode representation of the DbgRecord debug-info,
// convert to dbg.values before writing out.
bool IsNewDbgInfoFormat = M.IsNewDbgInfoFormat;
if (IsNewDbgInfoFormat)
@@ -56,8 +56,8 @@ namespace {
StringRef getPassName() const override { return "Bitcode Writer"; }
bool runOnModule(Module &M) override {
- // RemoveDIs: there's no bitcode representation of the DPValue debug-info,
- // convert to dbg.values before writing out.
+ // RemoveDIs: there's no bitcode representation of the DbgRecord
+ // debug-info, convert to dbg.values before writing out.
bool IsNewDbgInfoFormat = M.IsNewDbgInfoFormat;
if (IsNewDbgInfoFormat)
M.convertFromNewDbgValues();
diff --git a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
index a4b819a735c640..746926e56f2e17 100644
--- a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
+++ b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
@@ -217,13 +217,14 @@ void FunctionVarLocs::init(FunctionVarLocsBuilder &Builder) {
// to the start and end position in the vector with VarLocsBeforeInst. This
// block includes VarLocs for any DPValues attached to that instruction.
for (auto &P : Builder.VarLocsBeforeInst) {
- // Process VarLocs attached to a DPValue alongside their marker Instruction.
+ // Process VarLocs attached to a DbgRecord alongside their marker
+ // Instruction.
if (isa<const DbgRecord *>(P.first))
continue;
const Instruction *I = cast<const Instruction *>(P.first);
unsigned BlockStart = VarLocRecords.size();
- // Any VarLocInfos attached to a DPValue should now be remapped to their
- // marker Instruction, in order of DPValue appearance and prior to any
+ // Any VarLocInfos attached to a DbgRecord should now be remapped to their
+ // marker Instruction, in order of DbgRecord appearance and prior to any
// VarLocInfos attached directly to that instruction.
for (const DPValue &DPV : DPValue::filter(I->getDbgRecordRange())) {
// Even though DPV defines a variable location, VarLocsBeforeInst can
@@ -1649,7 +1650,7 @@ void AssignmentTrackingLowering::processUntaggedInstruction(
Ops.push_back(dwarf::DW_OP_deref);
DIE...
[truncated]
|
jryans
approved these changes
Mar 13, 2024
This patch continues the ongoing rename work, replacing DPValue with DbgRecord in comments and the names of variables, both members and fn-local. This is the most labour-intensive part of the rename, as it is where the most decisions have to be made about whether a given comment or variable is referring to DPValues (equivalent to debug variable intrinsics) or DbgRecords (a catch-all for all debug intrinsics); these decisions are not individually difficult, but comprise a fairly large amount of text to review. This patch still largely performs basic string substitutions followed by clang-format; there are almost* no places where, for example, a comment has been expanded or modified to reflect the semantic difference between DPValues and DbgRecords. I don't believe such a change is necessary anywhere in LLVM, but it may be useful in the docs, and so I'll be submitting docs changes as a separate patch. *In 2-3 places, `dbg.values` was replaced with `debug intrinsics`.
SLTozer
force-pushed
the
ddd-rename-comments-vars
branch
from
1f4262a to
06fca09
Compare
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 continues the ongoing rename work, replacing DPValue with DbgRecord in comments and the names of variables, both members and fn-local. This is the most labour-intensive part of the rename, as it is where the most decisions have to be made about whether a given comment or variable is referring to DPValues (equivalent to debug variable intrinsics) or DbgRecords (a catch-all for all debug intrinsics); these decisions are not individually difficult, but comprise a fairly large amount of text to review.
This patch still largely performs basic string substitutions followed by clang-format; there are almost* no places where, for example, a comment has been expanded or modified to reflect the semantic difference between DPValues and DbgRecords. I don't believe such a change is generally necessary in LLVM, but it may be useful in the docs, and so I'll be submitting docs changes as a separate patch.
*In a few places,
dbg.valueswas replaced withdebug intrinsics.