Skip to content

[RemoveDIs] Print non-intrinsic debug info in textual IR output #79281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 14 additions & 0 deletions llvm/include/llvm/IR/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,18 @@ class LLVM_EXTERNAL_VISIBILITY Module {
/// \ref BasicBlock.
bool IsNewDbgInfoFormat;

/// Used when converting this module to the new debug info format; removes all
/// declarations of debug intrinsics that are replaced by non-intrinsic
/// records in the new format.
void removeDebugIntrinsicDeclarations();
Copy link
Member

Choose a reason for hiding this comment

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

(Brief) Docu-comment wanted


/// \see BasicBlock::convertToNewDbgValues.
void convertToNewDbgValues() {
for (auto &F : *this) {
F.convertToNewDbgValues();
}
// Remove the declarations of the old debug intrinsics, if any exist.
removeDebugIntrinsicDeclarations();
IsNewDbgInfoFormat = true;
}

Expand All @@ -234,6 +241,13 @@ class LLVM_EXTERNAL_VISIBILITY Module {
IsNewDbgInfoFormat = false;
}

void setIsNewDbgInfoFormat(bool UseNewFormat) {
if (UseNewFormat && !IsNewDbgInfoFormat)
convertToNewDbgValues();
else if (!UseNewFormat && IsNewDbgInfoFormat)
convertFromNewDbgValues();
}

/// The Module constructor. Note that there is no default constructor. You
/// must provide a name for the module upon construction.
explicit Module(StringRef ModuleID, LLVMContext& C);
Expand Down
19 changes: 19 additions & 0 deletions llvm/include/llvm/IR/PrintPasses.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ std::string doSystemDiff(StringRef Before, StringRef After,
StringRef OldLineFormat, StringRef NewLineFormat,
StringRef UnchangedLineFormat);

/// Used to temporarily set the debug info format of a function, module, or
/// basic block for the duration of this object's lifetime, after which the
/// prior state will be restored.
template <typename T> class ScopedDbgInfoFormatSetter {
Copy link
Member

Choose a reason for hiding this comment

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

This wants a docu-comment indicating what it's doing -- no need to be detailed as it's unlikely to be a long term facility.

T &Obj;
bool OldState;

public:
ScopedDbgInfoFormatSetter(T &Obj, bool NewState)
: Obj(Obj), OldState(Obj.IsNewDbgInfoFormat) {
Obj.setIsNewDbgInfoFormat(NewState);
}
~ScopedDbgInfoFormatSetter() { Obj.setIsNewDbgInfoFormat(OldState); }
};

template <typename T>
ScopedDbgInfoFormatSetter(T &Obj, bool NewState)
-> ScopedDbgInfoFormatSetter<T>;

} // namespace llvm

#endif // LLVM_IR_PRINTPASSES_H
75 changes: 32 additions & 43 deletions llvm/lib/IR/AsmWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ class SlotTracker : public AbstractSlotTrackerStorage {
/// Add all of the metadata from an instruction.
void processInstructionMetadata(const Instruction &I);

/// Add all of the metadata from an instruction.
/// Add all of the metadata from a DbgRecord.
void processDbgRecordMetadata(const DbgRecord &DPV);
};

Expand Down Expand Up @@ -1140,6 +1140,9 @@ void SlotTracker::processFunctionMetadata(const Function &F) {

void SlotTracker::processDbgRecordMetadata(const DbgRecord &DR) {
if (const DPValue *DPV = dyn_cast<const DPValue>(&DR)) {
// Process metadata used by DbgRecords; we only specifically care about the
// DILocalVariable, DILocation, and DIAssignID fields, as the Value and
// Expression fields should only be printed inline and so do not use a slot.
CreateMetadataSlot(DPV->getVariable());
if (DPV->isDbgAssign())
CreateMetadataSlot(DPV->getAssignID());
Expand Down Expand Up @@ -2703,6 +2706,7 @@ class AssemblyWriter {
void printDPValue(const DPValue &DPI);
void printDPLabel(const DPLabel &DPL);
void printDbgRecord(const DbgRecord &DPI);
void printDbgRecordLine(const DbgRecord &DPI);

void printUseListOrder(const Value *V, const std::vector<unsigned> &Shuffle);
void printUseLists(const Function *F);
Expand Down Expand Up @@ -3885,9 +3889,6 @@ void AssemblyWriter::printTypeIdentities() {

/// printFunction - Print all aspects of a function.
void AssemblyWriter::printFunction(const Function *F) {
bool ConvertBack = F->IsNewDbgInfoFormat;
if (ConvertBack)
const_cast<Function *>(F)->convertFromNewDbgValues();
if (AnnotationWriter) AnnotationWriter->emitFunctionAnnot(F, Out);

if (F->isMaterializable())
Expand Down Expand Up @@ -4030,8 +4031,6 @@ void AssemblyWriter::printFunction(const Function *F) {
Out << "}\n";
}

if (ConvertBack)
const_cast<Function *>(F)->convertToNewDbgValues();
Machine.purgeFunction();
}

Expand Down Expand Up @@ -4098,6 +4097,8 @@ void AssemblyWriter::printBasicBlock(const BasicBlock *BB) {

// Output all of the instructions in the basic block...
for (const Instruction &I : *BB) {
for (const DbgRecord &DR : I.getDbgValueRange())
printDbgRecordLine(DR);
printInstructionLine(I);
}

Expand Down Expand Up @@ -4611,12 +4612,10 @@ void AssemblyWriter::printDbgRecord(const DbgRecord &DR) {
llvm_unreachable("Unexpected DbgRecord kind");
}

void AssemblyWriter::printDPValue(const DPValue &Value) {
// There's no formal representation of a DPValue -- print purely as a
// debugging aid.
Out << " DPValue ";

switch (Value.getType()) {
void AssemblyWriter::printDPValue(const DPValue &DPV) {
auto WriterCtx = getContext();
Out << "#dbg_";
switch (DPV.getType()) {
case DPValue::LocationType::Value:
Out << "value";
break;
Expand All @@ -4629,35 +4628,39 @@ void AssemblyWriter::printDPValue(const DPValue &Value) {
default:
llvm_unreachable("Tried to print a DPValue with an invalid LocationType!");
}
Out << " { ";
auto WriterCtx = getContext();
WriteAsOperandInternal(Out, Value.getRawLocation(), WriterCtx, true);
Out << "(";
WriteAsOperandInternal(Out, DPV.getRawLocation(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, Value.getVariable(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getVariable(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, Value.getExpression(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getExpression(), WriterCtx, true);
Out << ", ";
if (Value.isDbgAssign()) {
WriteAsOperandInternal(Out, Value.getAssignID(), WriterCtx, true);
if (DPV.isDbgAssign()) {
WriteAsOperandInternal(Out, DPV.getAssignID(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, Value.getRawAddress(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getRawAddress(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, Value.getAddressExpression(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getAddressExpression(), WriterCtx, true);
Out << ", ";
}
WriteAsOperandInternal(Out, Value.getDebugLoc().get(), WriterCtx, true);
Out << " marker @" << Value.getMarker();
Out << " }";
WriteAsOperandInternal(Out, DPV.getDebugLoc().getAsMDNode(), WriterCtx, true);
Out << ")";
}

/// printDbgRecordLine - Print a DbgRecord with indentation and a newline
/// character.
void AssemblyWriter::printDbgRecordLine(const DbgRecord &DR) {
// Print lengthier indentation to bring out-of-line with instructions.
Out << " ";
printDbgRecord(DR);
Out << '\n';
}

void AssemblyWriter::printDPLabel(const DPLabel &Label) {
// There's no formal representation of a DPLabel -- print purely as
// a debugging aid.
Out << " DPLabel { ";
auto WriterCtx = getContext();
Out << "#dbg_label(";
WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
Out << " marker @" << Label.getMarker();
Out << " }";
Out << ")";
}

void AssemblyWriter::printMetadataAttachments(
Expand Down Expand Up @@ -4805,19 +4808,11 @@ void BasicBlock::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,

void Module::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,
bool ShouldPreserveUseListOrder, bool IsForDebug) const {
// RemoveDIs: always print with debug-info in intrinsic format.
bool ConvertAfter = IsNewDbgInfoFormat;
if (IsNewDbgInfoFormat)
const_cast<Module *>(this)->convertFromNewDbgValues();

SlotTracker SlotTable(this);
formatted_raw_ostream OS(ROS);
AssemblyWriter W(OS, SlotTable, this, AAW, IsForDebug,
ShouldPreserveUseListOrder);
W.printModule(this);

if (ConvertAfter)
const_cast<Module *>(this)->convertToNewDbgValues();
}

void NamedMDNode::print(raw_ostream &ROS, bool IsForDebug) const {
Expand Down Expand Up @@ -4908,8 +4903,6 @@ void DPValue::print(raw_ostream &ROS, bool IsForDebug) const {

void DPMarker::print(raw_ostream &ROS, ModuleSlotTracker &MST,
bool IsForDebug) const {
// There's no formal representation of a DPMarker -- print purely as a
// debugging aid.
formatted_raw_ostream OS(ROS);
SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
SlotTracker &SlotTable =
Expand All @@ -4931,8 +4924,6 @@ void DPLabel::print(raw_ostream &ROS, bool IsForDebug) const {

void DPValue::print(raw_ostream &ROS, ModuleSlotTracker &MST,
bool IsForDebug) const {
// There's no formal representation of a DPValue -- print purely as a
// debugging aid.
formatted_raw_ostream OS(ROS);
SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
SlotTracker &SlotTable =
Expand All @@ -4950,8 +4941,6 @@ void DPValue::print(raw_ostream &ROS, ModuleSlotTracker &MST,

void DPLabel::print(raw_ostream &ROS, ModuleSlotTracker &MST,
bool IsForDebug) const {
// There's no formal representation of a DbgLabelRecord -- print purely as
// a debugging aid.
formatted_raw_ostream OS(ROS);
SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
SlotTracker &SlotTable =
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/IR/BasicBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ DPMarker *BasicBlock::createMarker(InstListType::iterator It) {
}

void BasicBlock::convertToNewDbgValues() {
// Is the command line option set?
if (!UseNewDbgInfoFormat)
return;

IsNewDbgInfoFormat = true;

// Iterate over all instructions in the instruction list, collecting dbg.value
Expand Down
24 changes: 8 additions & 16 deletions llvm/lib/IR/IRPrintingPasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

using namespace llvm;

extern cl::opt<bool> WriteNewDbgInfoFormat;
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes llvm/lib/IR depend on llvm/lib/IRPrinter (where this is defined), but the latter already depends on the former. That is, this introduces a cyclic dependency.

The definition of this symbol should probably live in this file and be extern in IRPrinter?

Copy link
Member

@MaskRay MaskRay Feb 26, 2024

Choose a reason for hiding this comment

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

Agree with the analysis. If you link the executable with ld.lld --warn-backrefs --no-fatal-warnings, you will get something like

error: backward reference detected: WriteNewDbgInfoFormat in llvm/_objs/Core/IRPrintingPasses.pic.o refers to llvm/_objs/IRPrinter/IRPrintingPasses.pic.o

(This is from a Bazel build).


namespace {

class PrintModulePassWrapper : public ModulePass {
Expand All @@ -39,11 +41,9 @@ class PrintModulePassWrapper : public ModulePass {
ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {}

bool runOnModule(Module &M) override {
// RemoveDIs: there's no textual representation of the DPValue debug-info,
// convert to dbg.values before writing out.
bool IsNewDbgInfoFormat = M.IsNewDbgInfoFormat;
if (IsNewDbgInfoFormat)
M.convertFromNewDbgValues();
// RemoveDIs: Regardless of the format we've processed this module in, use
// `WriteNewDbgInfoFormat` to determine which format we use to write it.
ScopedDbgInfoFormatSetter FormatSetter(M, WriteNewDbgInfoFormat);

if (llvm::isFunctionInPrintList("*")) {
if (!Banner.empty())
Expand All @@ -62,9 +62,6 @@ class PrintModulePassWrapper : public ModulePass {
}
}

if (IsNewDbgInfoFormat)
M.convertToNewDbgValues();

return false;
}

Expand All @@ -87,11 +84,9 @@ class PrintFunctionPassWrapper : public FunctionPass {

// This pass just prints a banner followed by the function as it's processed.
bool runOnFunction(Function &F) override {
// RemoveDIs: there's no textual representation of the DPValue debug-info,
// convert to dbg.values before writing out.
bool IsNewDbgInfoFormat = F.IsNewDbgInfoFormat;
if (IsNewDbgInfoFormat)
F.convertFromNewDbgValues();
// RemoveDIs: Regardless of the format we've processed this function in, use
// `WriteNewDbgInfoFormat` to determine which format we use to write it.
ScopedDbgInfoFormatSetter FormatSetter(F, WriteNewDbgInfoFormat);

if (isFunctionInPrintList(F.getName())) {
if (forcePrintModuleIR())
Expand All @@ -101,9 +96,6 @@ class PrintFunctionPassWrapper : public FunctionPass {
OS << Banner << '\n' << static_cast<Value &>(F);
}

if (IsNewDbgInfoFormat)
F.convertToNewDbgValues();

return false;
}

Expand Down
22 changes: 22 additions & 0 deletions llvm/lib/IR/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,28 @@ Module::~Module() {
IFuncList.clear();
}

void Module::removeDebugIntrinsicDeclarations() {
auto *DeclareIntrinsicFn =
Intrinsic::getDeclaration(this, Intrinsic::dbg_declare);
assert((!isMaterialized() || DeclareIntrinsicFn->hasZeroLiveUses()) &&
"Debug declare intrinsic should have had uses removed.");
DeclareIntrinsicFn->eraseFromParent();
auto *ValueIntrinsicFn =
Intrinsic::getDeclaration(this, Intrinsic::dbg_value);
assert((!isMaterialized() || ValueIntrinsicFn->hasZeroLiveUses()) &&
"Debug value intrinsic should have had uses removed.");
ValueIntrinsicFn->eraseFromParent();
auto *AssignIntrinsicFn =
Intrinsic::getDeclaration(this, Intrinsic::dbg_assign);
assert((!isMaterialized() || AssignIntrinsicFn->hasZeroLiveUses()) &&
"Debug assign intrinsic should have had uses removed.");
AssignIntrinsicFn->eraseFromParent();
auto *LabelntrinsicFn = Intrinsic::getDeclaration(this, Intrinsic::dbg_label);
assert((!isMaterialized() || LabelntrinsicFn->hasZeroLiveUses()) &&
"Debug label intrinsic should have had uses removed.");
LabelntrinsicFn->eraseFromParent();
}

std::unique_ptr<RandomNumberGenerator>
Module::createRNG(const StringRef Name) const {
SmallString<32> Salt(Name);
Expand Down
27 changes: 11 additions & 16 deletions llvm/lib/IRPrinter/IRPrintingPasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@

using namespace llvm;

cl::opt<bool> WriteNewDbgInfoFormat(
"write-experimental-debuginfo",
cl::desc("Write debug info in the new non-intrinsic format"),
cl::init(false));

PrintModulePass::PrintModulePass() : OS(dbgs()) {}
PrintModulePass::PrintModulePass(raw_ostream &OS, const std::string &Banner,
bool ShouldPreserveUseListOrder,
Expand All @@ -31,11 +36,9 @@ PrintModulePass::PrintModulePass(raw_ostream &OS, const std::string &Banner,
EmitSummaryIndex(EmitSummaryIndex) {}

PreservedAnalyses PrintModulePass::run(Module &M, ModuleAnalysisManager &AM) {
// RemoveDIs: there's no textual representation of the DPValue debug-info,
// convert to dbg.values before writing out.
bool ShouldConvert = M.IsNewDbgInfoFormat;
if (ShouldConvert)
M.convertFromNewDbgValues();
// RemoveDIs: Regardless of the format we've processed this module in, use
// `WriteNewDbgInfoFormat` to determine which format we use to write it.
ScopedDbgInfoFormatSetter FormatSetter(M, WriteNewDbgInfoFormat);

if (llvm::isFunctionInPrintList("*")) {
if (!Banner.empty())
Expand Down Expand Up @@ -63,9 +66,6 @@ PreservedAnalyses PrintModulePass::run(Module &M, ModuleAnalysisManager &AM) {
Index->print(OS);
}

if (ShouldConvert)
M.convertToNewDbgValues();

return PreservedAnalyses::all();
}

Expand All @@ -75,11 +75,9 @@ PrintFunctionPass::PrintFunctionPass(raw_ostream &OS, const std::string &Banner)

PreservedAnalyses PrintFunctionPass::run(Function &F,
FunctionAnalysisManager &) {
// RemoveDIs: there's no textual representation of the DPValue debug-info,
// convert to dbg.values before writing out.
bool ShouldConvert = F.IsNewDbgInfoFormat;
if (ShouldConvert)
F.convertFromNewDbgValues();
// RemoveDIs: Regardless of the format we've processed this function in, use
// `WriteNewDbgInfoFormat` to determine which format we use to write it.
ScopedDbgInfoFormatSetter FormatSetter(F, WriteNewDbgInfoFormat);

if (isFunctionInPrintList(F.getName())) {
if (forcePrintModuleIR())
Expand All @@ -88,8 +86,5 @@ PreservedAnalyses PrintFunctionPass::run(Function &F,
OS << Banner << '\n' << static_cast<Value &>(F);
}

if (ShouldConvert)
F.convertToNewDbgValues();

return PreservedAnalyses::all();
}
Loading