Skip to content

Commit

Permalink
Reapply 7d77bbe, adding new debug-info classes
Browse files Browse the repository at this point in the history
This reverts commit 957efa4.

Original commit message below -- in this follow up, I've shifted
un-necessary inclusions of DebugProgramInstruction.h into being forward
declarations (fixes clang-compile time I hope), and a memory leak in the
DebugInfoTest.cpp IR unittests.

I also tracked a compile-time regression in D154080, more explanation
there, but the result of which is hiding some of the changes behind the
EXPERIMENTAL_DEBUGINFO_ITERATORS compile-time flag. This is tested by the
"new-debug-iterators" buildbot.

[DebugInfo][RemoveDIs] Add prototype storage classes for "new" debug-info

This patch adds a variety of classes needed to record variable location
debug-info without using the existing intrinsic approach, see the rationale
at [0].

The two added files and corresponding unit tests are the majority of the
plumbing required for this, but at this point isn't accessible from the
rest of LLVM as we need to stage it into the repo gently. An overview is
that classes are added for recording variable information attached to Real
(TM) instructions, in the form of DPValues and DPMarker objects. The
metadata-uses of DPValues is plumbed into the metadata hierachy, and a
field added to class Instruction, which are all stimulated in the unit
tests. The next few patches in this series add utilities to convert to/from
this new debug-info format and add instruction/block utilities to have
debug-info automatically updated in the background when various operations
occur.

This patch was reviewed in Phab in D153990 and D154080, I've squashed them
together into this commit as there are dependencies between the two
patches, and there's little profit in landing them separately.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939
  • Loading branch information
jmorse committed Nov 8, 2023
1 parent 671d10a commit f1b0a54
Show file tree
Hide file tree
Showing 25 changed files with 1,658 additions and 9 deletions.
54 changes: 54 additions & 0 deletions llvm/include/llvm/IR/BasicBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class LLVMContext;
class Module;
class PHINode;
class ValueSymbolTable;
class DPValue;
class DPMarker;

/// LLVM Basic Block Representation
///
Expand All @@ -56,6 +58,9 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
public ilist_node_with_parent<BasicBlock, Function> {
public:
using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>>;
/// Flag recording whether or not this block stores debug-info in the form
/// of intrinsic instructions (false) or non-instruction records (true).
bool IsNewDbgInfoFormat;

private:
friend class BlockAddress;
Expand All @@ -64,6 +69,55 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
InstListType InstList;
Function *Parent;

public:
/// Attach a DPMarker to the given instruction. Enables the storage of any
/// debug-info at this position in the program.
DPMarker *createMarker(Instruction *I);
DPMarker *createMarker(InstListType::iterator It);

/// Convert variable location debugging information stored in dbg.value
/// intrinsics into DPMarker / DPValue records. 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
/// IsNewDbgInfoFormat = false.
void convertFromNewDbgValues();

/// Ensure the block is in "old" dbg.value format (\p NewFlag == false) or
/// in the new format (\p NewFlag == true), converting to the desired format
/// if necessary.
void setIsNewDbgInfoFormat(bool NewFlag);

/// Validate any DPMarkers / DPValues attached to instructions in this block,
/// and block-level stored data too (TrailingDPValues).
/// \p Assert Should this method fire an assertion if a problem is found?
/// \p Msg Should this method print a message to errs() if a problem is found?
/// \p OS Output stream to write errors to.
/// \returns True if a problem is found.
bool validateDbgValues(bool Assert = true, bool Msg = false,
raw_ostream *OS = nullptr);

/// Record that the collection of DPValues 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 setTrailingDPValues(DPMarker *M);

/// Fetch the collection of DPValues that "trail" after the last instruction
/// of this block, see \ref setTrailingDPValues. If there are none, returns
/// nullptr.
DPMarker *getTrailingDPValues();

/// Delete any trailing DPValues at the end of this block, see
/// \ref setTrailingDPValues.
void deleteTrailingDPValues();

void dumpDbgValues() const;

private:
void setParent(Function *parent);

/// Constructor.
Expand Down
4 changes: 4 additions & 0 deletions llvm/include/llvm/IR/DebugInfoMetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -3765,6 +3765,10 @@ class DIArgList : public MDNode {
iterator args_begin() { return Args.begin(); }
iterator args_end() { return Args.end(); }

ReplaceableMetadataImpl *getReplaceableUses() {
return Context.getReplaceableUses();
}

static bool classof(const Metadata *MD) {
return MD->getMetadataID() == DIArgListKind;
}
Expand Down

4 comments on commit f1b0a54

@ZequanWu
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this introduced a non-determinism bug. I'm still reducing the IR for repro.

@ZequanWu
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmorse
Copy link
Member Author

@jmorse jmorse commented on f1b0a54 Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note I've replicated this locally between tcmalloc and non-tcmalloc preloaded opt's. Would you be able to open an LLVM issue for more visibility -- I don't believe commit comments (i.e. here) are well monitored by anyone.

@jmorse
Copy link
Member Author

@jmorse jmorse commented on f1b0a54 Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #77774 as a fix -- it turns out to not be debug-info related, just a latent problem (from 2016!) that this commit exposed. (Insert here -- griping about destroying our tools with our tools)

Please sign in to comment.