Skip to content
Draft
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/include/clang/Basic/CodeGenOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// The file to use for dumping bug report by `Debugify` for original
/// debug info.
std::string DIBugsReportFilePath;
std::string DIBugsReportArgString;

/// The floating-point denormal mode to use.
llvm::DenormalMode FPDenormalMode = llvm::DenormalMode::getIEEE();
Expand Down
32 changes: 31 additions & 1 deletion clang/lib/CodeGen/BackendUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -904,13 +904,43 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
DebugifyEachInstrumentation Debugify;
DebugInfoPerPass DebugInfoBeforePass;
if (CodeGenOpts.EnableDIPreservationVerify) {

Debugify.setDebugifyMode(DebugifyMode::OriginalDebugInfo);
Debugify.setDebugInfoBeforePass(DebugInfoBeforePass);

if (!CodeGenOpts.DIBugsReportFilePath.empty())
if (!CodeGenOpts.DIBugsReportFilePath.empty()) {
Debugify.setOrigDIVerifyBugsReportFilePath(
CodeGenOpts.DIBugsReportFilePath);
std::error_code EC;
raw_fd_ostream OS_FILE{CodeGenOpts.DIBugsReportFilePath, EC,
sys::fs::OF_Append | sys::fs::OF_TextWithCRLF};
if (EC) {
errs() << "Could not open file: " << EC.message() << ", "
<< CodeGenOpts.DIBugsReportFilePath << '\n';
} else {
if (auto L = OS_FILE.lock()) {
OS_FILE << CodeGenOpts.DIBugsReportArgString;
}
OS_FILE.close();
}
}
Debugify.registerCallbacks(PIC, MAM);

#ifdef LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
// If we're using debug location coverage tracking, mark all the
// instructions coming out of the frontend without a DebugLoc as being
// intentional line-zero locations, to prevent both those instructions and
// new instructions that inherit their location from being treated as
// incorrectly empty locations.
for (Function &F : *TheModule) {
if (!F.getSubprogram())
continue;
for (BasicBlock &BB : F)
for (Instruction &I : BB)
if (!I.getDebugLoc())
I.setDebugLoc(DebugLoc::getLineZero());
}
#endif
}
// Attempt to load pass plugins and register their callbacks with PB.
for (auto &PluginFN : CodeGenOpts.PassPlugins) {
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1876,6 +1876,16 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
<< Opts.DIBugsReportFilePath;
Opts.DIBugsReportFilePath = "";
}
if (Opts.EnableDIPreservationVerify && Opts.DIBugsReportFilePath.size()) {
std::string ArgString;
llvm::raw_string_ostream OS(ArgString);
OS << "{\"file\":\"" << OutputFile << "\", \"args\":\"";
for (Arg *A : Args) {
OS << A->getAsString(Args) << " ";
}
OS << "\"}\n";
Opts.DIBugsReportArgString = ArgString;
}

Opts.NewStructPathTBAA = !Args.hasArg(OPT_no_struct_path_tbaa) &&
Args.hasArg(OPT_new_struct_path_tbaa);
Expand Down
2 changes: 2 additions & 0 deletions llvm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ endif()

option(LLVM_ENABLE_CRASH_DUMPS "Turn on memory dumps on crashes. Currently only implemented on Windows." OFF)

option(LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING "Enhance debugify's line number tracking at the cost of performance; abi-breaking." ON)

set(WINDOWS_PREFER_FORWARD_SLASH_DEFAULT OFF)
if (MINGW)
# Cygwin doesn't identify itself as Windows, and thus gets path::Style::posix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ class LegalizationArtifactCombiner {
const LLT DstTy = MRI.getType(DstReg);
if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
auto &CstVal = SrcMI->getOperand(1);
auto *MergedLocation = DILocation::getMergedLocation(
MI.getDebugLoc().get(), SrcMI->getDebugLoc().get());
auto MergedLocation = DebugLoc::getMergedLocation(
MI.getDebugLoc(), SrcMI->getDebugLoc());
// Set the debug location to the merged location of the SrcMI and the MI
// if the aext fold is successful.
Builder.setDebugLoc(MergedLocation);
Expand Down
4 changes: 4 additions & 0 deletions llvm/include/llvm/Config/config.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
/* Define to 1 to enable crash memory dumps, and to 0 otherwise. */
#cmakedefine01 LLVM_ENABLE_CRASH_DUMPS

/* Define to 1 to enable expensive checks for debug location coverage checking,
and to 0 otherwise. */
#cmakedefine01 LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING

/* Define to 1 to prefer forward slashes on Windows, and to 0 prefer
backslashes. */
#cmakedefine01 LLVM_WINDOWS_PREFER_FORWARD_SLASH
Expand Down
94 changes: 93 additions & 1 deletion llvm/include/llvm/IR/DebugLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#ifndef LLVM_IR_DEBUGLOC_H
#define LLVM_IR_DEBUGLOC_H

#include "llvm/Config/config.h"
#include "llvm/IR/TrackingMDRef.h"
#include "llvm/Support/DataTypes.h"

Expand All @@ -22,6 +23,74 @@ namespace llvm {
class LLVMContext;
class raw_ostream;
class DILocation;
class Function;

#ifdef LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
struct DbgLocOriginBacktrace {
static constexpr unsigned long MaxDepth = 16;
SmallVector<std::pair<int, std::array<void *, MaxDepth>>, 0> Stacktraces;
DbgLocOriginBacktrace(bool ShouldCollectTrace);
void addTrace();
};

// Used to represent different "kinds" of DebugLoc, expressing that a DebugLoc
// is either ordinary, containing a valid DILocation, or otherwise describing
// the reason why the DebugLoc does not contain a valid DILocation.
enum class DebugLocKind : uint8_t {
// DebugLoc should contain a valid DILocation.
Normal,
// DebugLoc intentionally does not have a valid DILocation; may be for a
// compiler-generated instruction, or an explicitly dropped location.
LineZero,
// DebugLoc does not have a known or currently knowable source location.
Unknown,
// Used for instructions that we don't expect to be emitted, and so can omit
// a valid DILocation. It is an error to try and emit a Temporary DebugLoc
// into the line table.
Temporary
};

// Extends TrackingMDNodeRef to also store a DebugLocKind and Backtrace,
// allowing Debugify to ignore intentionally-empty DebugLocs and display the
// code responsible for generating unintentionally-empty DebugLocs.
class DILocAndCoverageTracking : public TrackingMDNodeRef {
public:
DebugLocKind Kind;
// Currently we only need to track the Origin of this DILoc when using a
// normal empty DebugLoc, so only collect the stack trace in those cases.
DbgLocOriginBacktrace Origin;
DILocAndCoverageTracking(bool NeedsStacktrace = true)
: TrackingMDNodeRef(nullptr), Kind(DebugLocKind::Normal), Origin(NeedsStacktrace) {
}
// Valid or nullptr MDNode*, normal DebugLocKind
DILocAndCoverageTracking(const MDNode *Loc)
: TrackingMDNodeRef(const_cast<MDNode *>(Loc)),
Kind(DebugLocKind::Normal), Origin(!Loc) {}
DILocAndCoverageTracking(const DILocation *Loc);
// Always nullptr MDNode*, any DebugLocKind
DILocAndCoverageTracking(DebugLocKind Kind)
: TrackingMDNodeRef(nullptr), Kind(Kind),
Origin(Kind == DebugLocKind::Normal) {}
};
template <> struct simplify_type<DILocAndCoverageTracking> {
using SimpleType = MDNode *;

static MDNode *getSimplifiedValue(DILocAndCoverageTracking &MD) {
return MD.get();
}
};
template <> struct simplify_type<const DILocAndCoverageTracking> {
using SimpleType = MDNode *;

static MDNode *getSimplifiedValue(const DILocAndCoverageTracking &MD) {
return MD.get();
}
};

using DebugLocTrackingRef = DILocAndCoverageTracking;
#else
using DebugLocTrackingRef = TrackingMDNodeRef;
#endif // LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING

/// A debug info location.
///
Expand All @@ -31,7 +100,8 @@ namespace llvm {
/// To avoid extra includes, \a DebugLoc doubles the \a DILocation API with a
/// one based on relatively opaque \a MDNode pointers.
class DebugLoc {
TrackingMDNodeRef Loc;

DebugLocTrackingRef Loc;

public:
DebugLoc() = default;
Expand All @@ -47,6 +117,28 @@ namespace llvm {
/// IR.
explicit DebugLoc(const MDNode *N);

#ifdef LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
DebugLoc(DebugLocKind Kind) : Loc(Kind) {}
DebugLocKind getKind() const { return Loc.Kind; }
DbgLocOriginBacktrace getOrigin() const { return Loc.Origin; }
DebugLoc getCopied() const {
DebugLoc NewDL = *this;
NewDL.Loc.Origin.addTrace();
return NewDL;
}
#else
DebugLoc getCopied() const {
return *this;
}
#endif

static DebugLoc getTemporary();
static DebugLoc getUnknown();
static DebugLoc getLineZero();

static DebugLoc getMergedLocations(ArrayRef<DebugLoc> Locs);
static DebugLoc getMergedLocation(DebugLoc LocA, DebugLoc LocB);

/// Get the underlying \a DILocation.
///
/// \pre !*this or \c isa<DILocation>(getAsMDNode()).
Expand Down
30 changes: 28 additions & 2 deletions llvm/include/llvm/IR/IRBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,15 @@ class IRBuilderBase {
/// created instructions, like !dbg metadata.
SmallVector<std::pair<unsigned, MDNode *>, 2> MetadataToCopy;

DebugLoc StoredDL;

/// Add or update the an entry (Kind, MD) to MetadataToCopy, if \p MD is not
/// null. If \p MD is null, remove the entry with \p Kind.
void AddOrRemoveMetadataToCopy(unsigned Kind, MDNode *MD) {
if (Kind == LLVMContext::MD_dbg) {
StoredDL = DebugLoc(MD);
}

if (!MD) {
erase_if(MetadataToCopy, [Kind](const std::pair<unsigned, MDNode *> &KV) {
return KV.first == Kind;
Expand Down Expand Up @@ -216,6 +222,10 @@ class IRBuilderBase {
/// Set location information used by debugging information.
void SetCurrentDebugLocation(DebugLoc L) {
AddOrRemoveMetadataToCopy(LLVMContext::MD_dbg, L.getAsMDNode());
// Although we set StoredDL in the above call, we prefer to use the exact
// DebugLoc we were given, so overwrite it here; the call is only needed to
// update the entry in MetadataToCopy.
StoredDL = std::move(L);
}

/// Set nosanitize metadata.
Expand All @@ -229,8 +239,11 @@ class IRBuilderBase {
/// not on \p Src will be dropped from MetadataToCopy.
void CollectMetadataToCopy(Instruction *Src,
ArrayRef<unsigned> MetadataKinds) {
for (unsigned K : MetadataKinds)
for (unsigned K : MetadataKinds) {
AddOrRemoveMetadataToCopy(K, Src->getMetadata(K));
if (K == LLVMContext::MD_dbg)
SetCurrentDebugLocation(Src->getDebugLoc());
}
Copy link

Choose a reason for hiding this comment

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

Why this?

Copy link
Owner Author

@SLTozer SLTozer Aug 15, 2024

Choose a reason for hiding this comment

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

AddOrRemoveMetadataToCopy only takes the MDNode* as an argument, so we can't capture the metadata from the source DebugLoc. We still call AddOrRemoveMetadataToCopy because this whole bit in IRBuilder is a bit messy; we want MD_dbg to still appear in the MetadataToCopy map because otherwise we won't know that we need to copy the stored debugloc to any inserted instruction. I've sprinkled comments around the place to explain parts of this piecemeal, but before submitting this to a real review it probably needs to be rewritten more intuitively.

}

/// Get location information used by debugging information.
Expand All @@ -242,8 +255,21 @@ class IRBuilderBase {

/// Add all entries in MetadataToCopy to \p I.
void AddMetadataToInst(Instruction *I) const {
for (const auto &KV : MetadataToCopy)
for (const auto &KV : MetadataToCopy) {
if (KV.first == LLVMContext::MD_dbg) {
// If `!I->getDebugLoc()` then we will call this below anyway, so skip
// a duplicate call here.
if (I->getDebugLoc())
I->setDebugLoc(StoredDL.getCopied());
continue;
}
I->setMetadata(KV.first, KV.second);
}
// If I does not have an existing DebugLoc and no DebugLoc has been set
// here, we copy our DebugLoc to I anyway, because more likely than not I
// is a new instruction whose DL should originate from this builder.
if (!I->getDebugLoc())
I->setDebugLoc(StoredDL.getCopied());
Copy link

Choose a reason for hiding this comment

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

Would these things not more naturally be placed in setDebugLoc?

Copy link
Owner Author

Choose a reason for hiding this comment

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

"these things" meaning the getCopied() call? Probably; the only reason I held back is that it would mean collecting a lot more backtraces (probably nearly twice as many), but that's not necessarily a good reason to not do it considering we've already killed performance by collecting them in the first place.

}

/// Get the return type of the current function that we're emitting
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/IR/Instruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ class Instruction : public User,
/// The DebugLoc attached to this instruction will be overwritten by the
/// merged DebugLoc.
void applyMergedLocation(DILocation *LocA, DILocation *LocB);
void applyMergedLocation(DebugLoc LocA, DebugLoc LocB);

/// Updates the debug location given that the instruction has been hoisted
/// from a block to a predecessor of that block.
Expand Down
25 changes: 25 additions & 0 deletions llvm/include/llvm/Support/Signals.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,29 @@
#ifndef LLVM_SUPPORT_SIGNALS_H
#define LLVM_SUPPORT_SIGNALS_H

#include "llvm/Config/config.h"
#include <array>
#include <cstdint>
#include <string>

namespace llvm {
class StringRef;
class raw_ostream;

#ifdef LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
template <typename T, typename Enable> struct DenseMapInfo;
template <typename ValueT, typename ValueInfoT> class DenseSet;
namespace detail {
template <typename KeyT, typename ValueT> struct DenseMapPair;
}
template <typename KeyT, typename ValueT, typename KeyInfoT, typename BucketT>
class DenseMap;
using AddressSet = DenseSet<void *, DenseMapInfo<void *, void>>;
using SymbolizedAddressMap =
DenseMap<void *, std::string, DenseMapInfo<void *, void>,
detail::DenseMapPair<void *, std::string>>;
#endif

namespace sys {

/// This function runs all the registered interrupt handlers, including the
Expand Down Expand Up @@ -55,6 +71,15 @@ namespace sys {
/// specified, the entire frame is printed.
void PrintStackTrace(raw_ostream &OS, int Depth = 0);

#ifdef LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
template <unsigned long MaxDepth> int getStackTrace(std::array<void *, MaxDepth> &StackTrace);

/// Takes a set of \p Addresses, symbolizes them and stores the result in the
/// provided \p SymbolizedAddresses map.
void symbolizeAddresses(AddressSet &Addresses,
SymbolizedAddressMap &SymbolizedAddresses);
#endif

// Run all registered signal handlers.
void RunSignalHandlers();

Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "llvm/CodeGen/TargetLowering.h"
#include "llvm/CodeGen/TargetRegisterInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/Config/config.h"
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include "llvm/IR/Constants.h"
Expand Down Expand Up @@ -2080,6 +2081,10 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
}

if (!DL) {
#ifdef LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
assert(DL.getKind() != DebugLocKind::Temporary &&
"Temporary DebugLocs should never be considered for emission!");
#endif
// We have an unspecified location, which might want to be line 0.
// If we have already emitted a line-0 record, don't repeat it.
if (LastAsmLine == 0)
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/BranchFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ void BranchFolder::mergeCommonTails(unsigned commonTailIndex) {
"Reached BB end within common tail");
}
assert(MI.isIdenticalTo(*Pos) && "Expected matching MIIs!");
DL = DILocation::getMergedLocation(DL, Pos->getDebugLoc());
DL = DebugLoc::getMergedLocation(DL, Pos->getDebugLoc());
NextCommonInsts[i] = ++Pos;
}
MI.setDebugLoc(DL);
Expand Down Expand Up @@ -915,7 +915,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
// Walk through equivalence sets looking for actual exact matches.
while (MergePotentials.size() > 1) {
unsigned CurHash = MergePotentials.back().getHash();
const DebugLoc &BranchDL = MergePotentials.back().getBranchDebugLoc();
const DebugLoc BranchDL = MergePotentials.back().getBranchDebugLoc();

// Build SameTails, identifying the set of blocks with this hash code
// and with the maximum number of instructions in common.
Expand Down
Loading