Skip to content

Commit

Permalink
Revert "[RemoveDIs] Read/write DbgRecords directly from/to bitcode (#…
Browse files Browse the repository at this point in the history
…83251)"

This reverts commit d6d3d96.
  • Loading branch information
OCHyams committed Mar 15, 2024
1 parent 3789870 commit eba928f
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 500 deletions.
11 changes: 0 additions & 11 deletions llvm/include/llvm/Bitcode/LLVMBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -624,17 +624,6 @@ enum FunctionCodes {
// operation, align, vol,
// ordering, synchscope]
FUNC_CODE_BLOCKADDR_USERS = 60, // BLOCKADDR_USERS: [value...]

FUNC_CODE_DEBUG_RECORD_VALUE =
61, // [DILocation, DILocalVariable, DIExpression, ValueAsMetadata]
FUNC_CODE_DEBUG_RECORD_DECLARE =
62, // [DILocation, DILocalVariable, DIExpression, ValueAsMetadata]
FUNC_CODE_DEBUG_RECORD_ASSIGN =
63, // [DILocation, DILocalVariable, DIExpression, ValueAsMetadata,
// DIAssignID, DIExpression (addr), ValueAsMetadata (addr)]
FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE =
64, // [DILocation, DILocalVariable, DIExpression, Value]
FUNC_CODE_DEBUG_RECORD_LABEL = 65, // [DILocation, DILabel]
};

enum UseListCodes {
Expand Down
5 changes: 0 additions & 5 deletions llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,6 @@ GetCodeName(unsigned CodeID, unsigned BlockID,
STRINGIFY_CODE(FUNC_CODE, INST_CMPXCHG)
STRINGIFY_CODE(FUNC_CODE, INST_CALLBR)
STRINGIFY_CODE(FUNC_CODE, BLOCKADDR_USERS)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_DECLARE)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_VALUE)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_ASSIGN)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_VALUE_SIMPLE)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_LABEL)
}
case bitc::VALUE_SYMTAB_BLOCK_ID:
switch (CodeID) {
Expand Down
101 changes: 3 additions & 98 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ static cl::opt<bool> ExpandConstantExprs(
cl::desc(
"Expand constant expressions to instructions for testing purposes"));

// Declare external flag for whether we're using the new debug-info format.
extern llvm::cl::opt<bool> UseNewDbgInfoFormat;

namespace {

enum {
Expand Down Expand Up @@ -4276,10 +4279,6 @@ Error BitcodeReader::parseGlobalIndirectSymbolRecord(
Error BitcodeReader::parseModule(uint64_t ResumeBit,
bool ShouldLazyLoadMetadata,
ParserCallbacks Callbacks) {
// Force the debug-info mode into the old format for now.
// FIXME: Remove this once all tools support RemoveDIs.
TheModule->IsNewDbgInfoFormat = false;

this->ValueTypeCallback = std::move(Callbacks.ValueType);
if (ResumeBit) {
if (Error JumpFailed = Stream.JumpToBit(ResumeBit))
Expand Down Expand Up @@ -6399,89 +6398,6 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
InstructionList.push_back(I);
break;
}
case bitc::FUNC_CODE_DEBUG_RECORD_LABEL: {
// DPLabels are placed after the Instructions that they are attached to.
Instruction *Inst = getLastInstruction();
if (!Inst)
return error("Invalid dbg record: missing instruction");
DILocation *DIL = cast<DILocation>(getFnMetadataByID(Record[0]));
DILabel *Label = cast<DILabel>(getFnMetadataByID(Record[1]));
Inst->getParent()->insertDbgRecordBefore(
new DPLabel(Label, DebugLoc(DIL)), Inst->getIterator());
continue; // This isn't an instruction.
}
case bitc::FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE:
case bitc::FUNC_CODE_DEBUG_RECORD_VALUE:
case bitc::FUNC_CODE_DEBUG_RECORD_DECLARE:
case bitc::FUNC_CODE_DEBUG_RECORD_ASSIGN: {
// DPValues are placed after the Instructions that they are attached to.
Instruction *Inst = getLastInstruction();
if (!Inst)
return error("Invalid dbg record: missing instruction");

// First 3 fields are common to all kinds:
// DILocation, DILocalVariable, DIExpression
// dbg_value (FUNC_CODE_DEBUG_RECORD_VALUE)
// ..., LocationMetadata
// dbg_value (FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE - abbrev'd)
// ..., Value
// dbg_declare (FUNC_CODE_DEBUG_RECORD_DECLARE)
// ..., LocationMetadata
// dbg_assign (FUNC_CODE_DEBUG_RECORD_ASSIGN)
// ..., LocationMetadata, DIAssignID, DIExpression, LocationMetadata
unsigned Slot = 0;
// Common fields (0-2).
DILocation *DIL = cast<DILocation>(getFnMetadataByID(Record[Slot++]));
DILocalVariable *Var =
cast<DILocalVariable>(getFnMetadataByID(Record[Slot++]));
DIExpression *Expr =
cast<DIExpression>(getFnMetadataByID(Record[Slot++]));

// Union field (3: LocationMetadata | Value).
Metadata *RawLocation = nullptr;
if (BitCode == bitc::FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE) {
Value *V = nullptr;
unsigned TyID = 0;
// We never expect to see a fwd reference value here because
// use-before-defs are encoded with the standard non-abbrev record
// type (they'd require encoding the type too, and they're rare). As a
// result, getValueTypePair only ever increments Slot by one here (once
// for the value, never twice for value and type).
unsigned SlotBefore = Slot;
if (getValueTypePair(Record, Slot, NextValueNo, V, TyID, CurBB))
return error("Invalid dbg record: invalid value");
(void)SlotBefore;
assert((SlotBefore == Slot - 1) && "unexpected fwd ref");
RawLocation = ValueAsMetadata::get(V);
} else {
RawLocation = getFnMetadataByID(Record[Slot++]);
}

DPValue *DPV = nullptr;
switch (BitCode) {
case bitc::FUNC_CODE_DEBUG_RECORD_VALUE:
case bitc::FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE:
DPV = new DPValue(RawLocation, Var, Expr, DIL,
DPValue::LocationType::Value);
break;
case bitc::FUNC_CODE_DEBUG_RECORD_DECLARE:
DPV = new DPValue(RawLocation, Var, Expr, DIL,
DPValue::LocationType::Declare);
break;
case bitc::FUNC_CODE_DEBUG_RECORD_ASSIGN: {
DIAssignID *ID = cast<DIAssignID>(getFnMetadataByID(Record[Slot++]));
DIExpression *AddrExpr =
cast<DIExpression>(getFnMetadataByID(Record[Slot++]));
Metadata *Addr = getFnMetadataByID(Record[Slot++]);
DPV = new DPValue(RawLocation, Var, Expr, ID, Addr, AddrExpr, DIL);
break;
}
default:
llvm_unreachable("Unknown DPValue bitcode");
}
Inst->getParent()->insertDbgRecordBefore(DPV, Inst->getIterator());
continue; // This isn't an instruction.
}
case bitc::FUNC_CODE_INST_CALL: {
// CALL: [paramattrs, cc, fmf, fnty, fnid, arg0, arg1...]
if (Record.size() < 3)
Expand Down Expand Up @@ -6761,21 +6677,10 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
// Move the bit stream to the saved position of the deferred function body.
if (Error JumpFailed = Stream.JumpToBit(DFII->second))
return JumpFailed;

// Set the debug info mode to "new", forcing a mismatch between
// module and function debug modes. This is okay because we'll convert
// everything back to the old mode after parsing.
// FIXME: Remove this once all tools support RemoveDIs.
F->IsNewDbgInfoFormat = true;

if (Error Err = parseFunctionBody(F))
return Err;
F->setIsMaterializable(false);

// Convert new debug info records into intrinsics.
// FIXME: Remove this once all tools support RemoveDIs.
F->convertFromNewDbgValues();

if (StripDebugInfo)
stripDebugInfo(*F);

Expand Down
120 changes: 18 additions & 102 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ namespace llvm {
extern FunctionSummary::ForceSummaryHotnessType ForceSummaryEdgesCold;
}

extern bool WriteNewDbgInfoFormatToBitcode;
extern llvm::cl::opt<bool> UseNewDbgInfoFormat;

namespace {

/// These are manifest constants used by the bitcode writer. They do not need to
Expand Down Expand Up @@ -131,7 +128,6 @@ enum {
FUNCTION_INST_RET_VAL_ABBREV,
FUNCTION_INST_UNREACHABLE_ABBREV,
FUNCTION_INST_GEP_ABBREV,
FUNCTION_DEBUG_RECORD_VALUE_ABBREV,
};

/// Abstract class to manage the bitcode writing, subclassed for each bitcode
Expand Down Expand Up @@ -3516,95 +3512,25 @@ void ModuleBitcodeWriter::writeFunction(
NeedsMetadataAttachment |= I.hasMetadataOtherThanDebugLoc();

// If the instruction has a debug location, emit it.
if (DILocation *DL = I.getDebugLoc()) {
if (DL == LastDL) {
// Just repeat the same debug loc as last time.
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC_AGAIN, Vals);
} else {
Vals.push_back(DL->getLine());
Vals.push_back(DL->getColumn());
Vals.push_back(VE.getMetadataOrNullID(DL->getScope()));
Vals.push_back(VE.getMetadataOrNullID(DL->getInlinedAt()));
Vals.push_back(DL->isImplicitCode());
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC, Vals);
Vals.clear();
LastDL = DL;
}
}
DILocation *DL = I.getDebugLoc();
if (!DL)
continue;

// If the instruction has DbgRecords attached to it, emit them. Note that
// they come after the instruction so that it's easy to attach them again
// when reading the bitcode, even though conceptually the debug locations
// start "before" the instruction.
if (I.hasDbgRecords() && WriteNewDbgInfoFormatToBitcode) {
/// Try to push the value only (unwrapped), otherwise push the
/// metadata wrapped value. Returns true if the value was pushed
/// without the ValueAsMetadata wrapper.
auto PushValueOrMetadata = [&Vals, InstID,
this](Metadata *RawLocation) {
assert(RawLocation && "RawLocation unexpectedly null in DPValue");
if (ValueAsMetadata *VAM = dyn_cast<ValueAsMetadata>(RawLocation)) {
SmallVector<unsigned, 2> ValAndType;
// If the value is a fwd-ref the type is also pushed. We don't
// want the type, so fwd-refs are kept wrapped (pushValueAndType
// returns false if the value is pushed without type).
if (!pushValueAndType(VAM->getValue(), InstID, ValAndType)) {
Vals.push_back(ValAndType[0]);
return true;
}
}
// The metadata is a DIArgList, or ValueAsMetadata wrapping a
// fwd-ref. Push the metadata ID.
Vals.push_back(VE.getMetadataID(RawLocation));
return false;
};

// Write out non-instruction debug information attached to this
// instruction. Write it after the instruction so that it's easy to
// re-attach to the instruction reading the records in.
for (DbgRecord &DR : I.DbgMarker->getDbgRecordRange()) {
if (DPLabel *DPL = dyn_cast<DPLabel>(&DR)) {
Vals.push_back(VE.getMetadataID(&*DPL->getDebugLoc()));
Vals.push_back(VE.getMetadataID(DPL->getLabel()));
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_LABEL, Vals);
Vals.clear();
continue;
}

// First 3 fields are common to all kinds:
// DILocation, DILocalVariable, DIExpression
// dbg_value (FUNC_CODE_DEBUG_RECORD_VALUE)
// ..., LocationMetadata
// dbg_value (FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE - abbrev'd)
// ..., Value
// dbg_declare (FUNC_CODE_DEBUG_RECORD_DECLARE)
// ..., LocationMetadata
// dbg_assign (FUNC_CODE_DEBUG_RECORD_ASSIGN)
// ..., LocationMetadata, DIAssignID, DIExpression, LocationMetadata
DPValue &DPV = cast<DPValue>(DR);
Vals.push_back(VE.getMetadataID(&*DPV.getDebugLoc()));
Vals.push_back(VE.getMetadataID(DPV.getVariable()));
Vals.push_back(VE.getMetadataID(DPV.getExpression()));
if (DPV.isDbgValue()) {
if (PushValueOrMetadata(DPV.getRawLocation()))
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE, Vals,
FUNCTION_DEBUG_RECORD_VALUE_ABBREV);
else
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_VALUE, Vals);
} else if (DPV.isDbgDeclare()) {
Vals.push_back(VE.getMetadataID(DPV.getRawLocation()));
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_DECLARE, Vals);
} else {
assert(DPV.isDbgAssign() && "Unexpected DbgRecord kind");
Vals.push_back(VE.getMetadataID(DPV.getRawLocation()));
Vals.push_back(VE.getMetadataID(DPV.getAssignID()));
Vals.push_back(VE.getMetadataID(DPV.getAddressExpression()));
Vals.push_back(VE.getMetadataID(DPV.getRawAddress()));
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_ASSIGN, Vals);
}
Vals.clear();
}
if (DL == LastDL) {
// Just repeat the same debug loc as last time.
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC_AGAIN, Vals);
continue;
}

Vals.push_back(DL->getLine());
Vals.push_back(DL->getColumn());
Vals.push_back(VE.getMetadataOrNullID(DL->getScope()));
Vals.push_back(VE.getMetadataOrNullID(DL->getInlinedAt()));
Vals.push_back(DL->isImplicitCode());
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC, Vals);
Vals.clear();

LastDL = DL;
}

if (BlockAddress *BA = BlockAddress::lookup(&BB)) {
Expand Down Expand Up @@ -3845,17 +3771,7 @@ void ModuleBitcodeWriter::writeBlockInfo() {
FUNCTION_INST_GEP_ABBREV)
llvm_unreachable("Unexpected abbrev ordering!");
}
{
auto Abbv = std::make_shared<BitCodeAbbrev>();
Abbv->Add(BitCodeAbbrevOp(bitc::FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 7)); // dbgloc
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 7)); // var
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 7)); // expr
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // val
if (Stream.EmitBlockInfoAbbrev(bitc::FUNCTION_BLOCK_ID, Abbv) !=
FUNCTION_DEBUG_RECORD_VALUE_ABBREV)
llvm_unreachable("Unexpected abbrev ordering! 1");
}

Stream.ExitBlock();
}

Expand Down
20 changes: 10 additions & 10 deletions llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,19 @@
#include "llvm/Pass.h"
using namespace llvm;

extern bool WriteNewDbgInfoFormatToBitcode;

PreservedAnalyses BitcodeWriterPass::run(Module &M, ModuleAnalysisManager &AM) {
bool ConvertToOldDbgFormatForWrite =
M.IsNewDbgInfoFormat && !WriteNewDbgInfoFormatToBitcode;
if (ConvertToOldDbgFormatForWrite)
// 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();

const ModuleSummaryIndex *Index =
EmitSummaryIndex ? &(AM.getResult<ModuleSummaryIndexAnalysis>(M))
: nullptr;
WriteBitcodeToFile(M, OS, ShouldPreserveUseListOrder, Index, EmitModuleHash);

if (ConvertToOldDbgFormatForWrite)
if (IsNewDbgInfoFormat)
M.convertToNewDbgValues();

return PreservedAnalyses::all();
Expand All @@ -57,15 +56,16 @@ namespace {
StringRef getPassName() const override { return "Bitcode Writer"; }

bool runOnModule(Module &M) override {
bool ConvertToOldDbgFormatForWrite =
M.IsNewDbgInfoFormat && !WriteNewDbgInfoFormatToBitcode;
if (ConvertToOldDbgFormatForWrite)
// 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();

WriteBitcodeToFile(M, OS, ShouldPreserveUseListOrder, /*Index=*/nullptr,
/*EmitModuleHash=*/false);

if (ConvertToOldDbgFormatForWrite)
if (IsNewDbgInfoFormat)
M.convertToNewDbgValues();
return false;
}
Expand Down

0 comments on commit eba928f

Please sign in to comment.