Revert "[RemoveDIs] Read/write DbgRecords directly from/to bitcode" (#85382)

Reverts #83251

Buildbot: https://lab.llvm.org/buildbot/#/builders/139/builds/61485

Author: OCHyams

llvm/include/llvm/Bitcode/LLVMBitCodes.h

@@ -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 {

llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp

@@ -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) {

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

@@ -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 {
@@ -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))
@@ -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)
@@ -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);
 

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -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
@@ -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
@@ -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)) {
@@ -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();
 }
 

llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp

@@ -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();
@@ -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;
     }