[CodeView] Combine variable def ranges that are continuous.

[zequanwu]

Summary

It saves about 1.13% size for chrome.dll.pdb on chrome official build.

[zequanwu]

Update test.

[zequanwu]

Remove unnecessary includes.

[rnk]

Nice!

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

82

To minimize the amount of code that needs to know or assume the size of LocalVarDefRange, I recommend exposing a method, maybe called toOpaqueValue, which returns a uint64_t. Then, all of these methods don't need to do memcpy, they just call that method, and compare or operate on integer values as you have them. You may also need createFromOpaqueValue or something like that.

95

Prefer sizeof(Val) to hardcoding 8. It's worth adding static_assert(sizeof(uint64_t) == sizeof(LocalVarDefRange)); to ensure that the bitfields are packed as expected.

505

Instead of inheritance, can all those methods be defined here?

[zequanwu]

• Address comments.
• Rename LocalVarDefRange to LocalVarDef as it no longer has range info inside.

[rnk]

lgtm, thanks!

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

506

You may need to use ULL sufixes so that this compiles as you expect on LLP64 platforms, such as Windows x64.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

 class GlobalVariable;
 class MCSectionCOFF;
 class MCStreamer;
 class MCSymbol;
 class MachineFunction;
 
 /// Collects and handles line tables information in a CodeView format.
 class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
-  MCStreamer &OS;
-  BumpPtrAllocator Allocator;
-  codeview::GlobalTypeTableBuilder TypeTable;
-
-  /// Whether to emit type record hashes into .debug$H.
-  bool EmitDebugGlobalHashes = false;
-
-  /// The codeview CPU type used by the translation unit.
-  codeview::CPUType TheCPU;
-
-  /// Represents the most general definition range.
-  struct LocalVarDefRange {
+public:
+  struct LocalVarDef {
     /// Indicates that variable data is stored in memory relative to the
     /// specified register.
     int InMemory : 1;
 
     /// Offset of variable data in memory.
     int DataOffset : 31;
 
     /// Non-zero if this is a piece of an aggregate.
     uint16_t IsSubfield : 1;
 
     /// Offset into aggregate.
     uint16_t StructOffset : 15;
 
     /// Register containing the data or the register base of the memory
     /// location containing the data.
     uint16_t CVRegister;
 
-    /// Compares all location fields. This includes all fields except the label
-    /// ranges.
-    bool isDifferentLocation(LocalVarDefRange &O) {
-      return InMemory != O.InMemory || DataOffset != O.DataOffset ||
-             IsSubfield != O.IsSubfield || StructOffset != O.StructOffset ||
-             CVRegister != O.CVRegister;
+    uint64_t static toOpaqueValue(const LocalVarDef DR) {
+      uint64_t Val = 0;
+      std::memcpy(&Val, &DR, sizeof(Val));
+      return Val;
     }
 
-    SmallVector<std::pair<const MCSymbol *, const MCSymbol *>, 1> Ranges;
+    LocalVarDef static createFromOpaqueValue(uint64_t Val) {
+      LocalVarDef DR;
+      std::memcpy(&DR, &Val, sizeof(Val));
+      return DR;
+    }
   };
 
-  static LocalVarDefRange createDefRangeMem(uint16_t CVRegister, int Offset);
+  static_assert(sizeof(uint64_t) == sizeof(LocalVarDef), "");
+
+private:
+  MCStreamer &OS;
+  BumpPtrAllocator Allocator;
+  codeview::GlobalTypeTableBuilder TypeTable;
+
+  /// Whether to emit type record hashes into .debug$H.
+  bool EmitDebugGlobalHashes = false;
+
+  /// The codeview CPU type used by the translation unit.
+  codeview::CPUType TheCPU;
+
+  static LocalVarDef createDefRangeMem(uint16_t CVRegister, int Offset);
 
   /// Similar to DbgVariable in DwarfDebug, but not dwarf-specific.
   struct LocalVariable {
     const DILocalVariable *DIVar = nullptr;
-    SmallVector<LocalVarDefRange, 1> DefRanges;
+    MapVector<LocalVarDef,
+              SmallVector<std::pair<const MCSymbol *, const MCSymbol *>, 1>>
+        DefRanges;
     bool UseReferenceType = false;
   };
 
   struct CVGlobalVariable {
     const DIGlobalVariable *DIGV;
     PointerUnion<const GlobalVariable *, const DIExpression *> GVInfo;
   };
 
public:
 
   /// Emit the COFF section that holds the line table information.
   void endModule() override;
 
   /// Process beginning of an instruction.
   void beginInstruction(const MachineInstr *MI) override;
 };
 
+template <> struct DenseMapInfo<CodeViewDebug::LocalVarDef> {
+
+  static inline CodeViewDebug::LocalVarDef getEmptyKey() {
+    return CodeViewDebug::LocalVarDef::createFromOpaqueValue(~0UL);
+  }
+
+  static inline CodeViewDebug::LocalVarDef getTombstoneKey() {
+    return CodeViewDebug::LocalVarDef::createFromOpaqueValue(~0UL - 1L);
+  }
+
+  static unsigned getHashValue(const CodeViewDebug::LocalVarDef &DR) {
+    return CodeViewDebug::LocalVarDef::toOpaqueValue(DR) * 37UL;
+  }
+
+  static bool isEqual(const CodeViewDebug::LocalVarDef &LHS,
+                      const CodeViewDebug::LocalVarDef &RHS) {
+    return CodeViewDebug::LocalVarDef::toOpaqueValue(LHS) ==
+           CodeViewDebug::LocalVarDef::toOpaqueValue(RHS);
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_ASMPRINTER_CODEVIEWDEBUG_H

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

MCSymbol *SymbolsEnd = beginCVSubsection(DebugSubsectionKind::Symbols);
     emitEndSymbolRecord(SymbolKind::S_PROC_ID_END);
   }
   endCVSubsection(SymbolsEnd);
 
   // We have an assembler directive that takes care of the whole line table.
   OS.emitCVLinetableDirective(FI.FuncId, Fn, FI.End);
 }
 
-CodeViewDebug::LocalVarDefRange
+CodeViewDebug::LocalVarDef
 CodeViewDebug::createDefRangeMem(uint16_t CVRegister, int Offset) {
-  LocalVarDefRange DR;
+  LocalVarDef DR;
   DR.InMemory = -1;
   DR.DataOffset = Offset;
   assert(DR.DataOffset == Offset && "truncation");
   DR.IsSubfield = 0;
   DR.StructOffset = 0;
   DR.CVRegister = CVRegister;
   return DR;
 }
for (const MachineFunction::VariableDbgInfo &VI : MF.getVariableDbgInfo()) {
     Register FrameReg;
     StackOffset FrameOffset = TFI->getFrameIndexReference(*Asm->MF, VI.Slot, FrameReg);
     uint16_t CVReg = TRI->getCodeViewRegNum(FrameReg);
 
     assert(!FrameOffset.getScalable() &&
            "Frame offsets with a scalable component are not supported");
 
     // Calculate the label ranges.
-    LocalVarDefRange DefRange =
+    LocalVarDef DefRange =
         createDefRangeMem(CVReg, FrameOffset.getFixed() + ExprOffset);
 
+    LocalVariable Var;
+    Var.DIVar = VI.Var;
+
     for (const InsnRange &Range : Scope->getRanges()) {
       const MCSymbol *Begin = getLabelBeforeInsn(Range.first);
       const MCSymbol *End = getLabelAfterInsn(Range.second);
       End = End ? End : Asm->getFunctionEnd();
-      DefRange.Ranges.emplace_back(Begin, End);
+      Var.DefRanges[DefRange].emplace_back(Begin, End);
     }
 
-    LocalVariable Var;
-    Var.DIVar = VI.Var;
-    Var.DefRanges.emplace_back(std::move(DefRange));
     if (Deref)
       Var.UseReferenceType = true;
 
     recordLocalVariable(std::move(Var), Scope);
   }
 }
 
 static bool canUseReferenceType(const DbgVariableLocation &Loc) {
if (Var.UseReferenceType) {
       Var.DefRanges.clear();
       calculateRanges(Var, Entries);
       return;
     }
 
     // We can only handle a register or an offseted load of a register.
     if (Location->Register == 0 || Location->LoadChain.size() > 1)
       continue;
-    {
-      LocalVarDefRange DR;
-      DR.CVRegister = TRI->getCodeViewRegNum(Location->Register);
-      DR.InMemory = !Location->LoadChain.empty();
-      DR.DataOffset =
-          !Location->LoadChain.empty() ? Location->LoadChain.back() : 0;
-      if (Location->FragmentInfo) {
-        DR.IsSubfield = true;
-        DR.StructOffset = Location->FragmentInfo->OffsetInBits / 8;
-      } else {
-        DR.IsSubfield = false;
-        DR.StructOffset = 0;
-      }
+
+    LocalVarDef DR;
 
-      if (Var.DefRanges.empty() ||
-          Var.DefRanges.back().isDifferentLocation(DR)) {
-        Var.DefRanges.emplace_back(std::move(DR));
-      }
-    }
-
     // Compute the label range.
     const MCSymbol *Begin = getLabelBeforeInsn(Entry.getInstr());
     const MCSymbol *End;
     if (Entry.getEndIndex() != DbgValueHistoryMap::NoEntry) {
       auto &EndingEntry = Entries[Entry.getEndIndex()];
       End = EndingEntry.isDbgValue()
                 ? getLabelBeforeInsn(EndingEntry.getInstr())
                 : getLabelAfterInsn(EndingEntry.getInstr());
     } else
       End = Asm->getFunctionEnd();
 
     // If the last range end is our begin, just extend the last range.
     // Otherwise make a new range.
     SmallVectorImpl<std::pair<const MCSymbol *, const MCSymbol *>> &R =
-        Var.DefRanges.back().Ranges;
+        Var.DefRanges[DR];
     if (!R.empty() && R.back().second == Begin)
       R.back().second = End;
     else
       R.emplace_back(Begin, End);
 
     // FIXME: Do more range combining.
   }
 }
void CodeViewDebug::emitLocalVariable(const FunctionInfo &FI,
   // Truncate the name so we won't overflow the record length field.
   emitNullTerminatedSymbolName(OS, Var.DIVar->getName());
   endSymbolRecord(LocalEnd);
 
   // Calculate the on disk prefix of the appropriate def range record. The
   // records and on disk formats are described in SymbolRecords.h. BytePrefix
   // should be big enough to hold all forms without memory allocation.
   SmallString<20> BytePrefix;
-  for (const LocalVarDefRange &DefRange : Var.DefRanges) {
+  for (const auto &Pair : Var.DefRanges) {
+    LocalVarDef DefRange = Pair.first;
+    const auto &Ranges = Pair.second;
     BytePrefix.clear();
     if (DefRange.InMemory) {
       int Offset = DefRange.DataOffset;
       unsigned Reg = DefRange.CVRegister;
 
       // 32-bit x86 call sequences often use PUSH instructions, which disrupt
       // ESP-relative offsets. Use the virtual frame pointer, VFRAME or $T0,
       // instead. In frames without stack realignment, $T0 will be the CFA.
       if (RegisterId(Reg) == RegisterId::ESP) {
         Reg = unsigned(RegisterId::VFRAME);
         Offset += FI.OffsetAdjustment;
       }
 
       // If we can use the chosen frame pointer for the frame and this isn't a
       // sliced aggregate, use the smaller S_DEFRANGE_FRAMEPOINTER_REL record.
       // Otherwise, use S_DEFRANGE_REGISTER_REL.
       EncodedFramePtrReg EncFP = encodeFramePtrReg(RegisterId(Reg), TheCPU);
       if (!DefRange.IsSubfield && EncFP != EncodedFramePtrReg::None &&
           (bool(Flags & LocalSymFlags::IsParameter)
                ? (EncFP == FI.EncodedParamFramePtrReg)
                : (EncFP == FI.EncodedLocalFramePtrReg))) {
         DefRangeFramePointerRelHeader DRHdr;
         DRHdr.Offset = Offset;
-        OS.emitCVDefRangeDirective(DefRange.Ranges, DRHdr);
+        OS.emitCVDefRangeDirective(Ranges, DRHdr);
       } else {
         uint16_t RegRelFlags = 0;
         if (DefRange.IsSubfield) {
           RegRelFlags = DefRangeRegisterRelSym::IsSubfieldFlag |
                         (DefRange.StructOffset
                          << DefRangeRegisterRelSym::OffsetInParentShift);
         }
         DefRangeRegisterRelHeader DRHdr;
         DRHdr.Register = Reg;
         DRHdr.Flags = RegRelFlags;
         DRHdr.BasePointerOffset = Offset;
-        OS.emitCVDefRangeDirective(DefRange.Ranges, DRHdr);
+        OS.emitCVDefRangeDirective(Ranges, DRHdr);
       }
     } else {
       assert(DefRange.DataOffset == 0 && "unexpected offset into register");
       if (DefRange.IsSubfield) {
         DefRangeSubfieldRegisterHeader DRHdr;
         DRHdr.Register = DefRange.CVRegister;
         DRHdr.MayHaveNoName = 0;
         DRHdr.OffsetInParent = DefRange.StructOffset;
-        OS.emitCVDefRangeDirective(DefRange.Ranges, DRHdr);
+        OS.emitCVDefRangeDirective(Ranges, DRHdr);
       } else {
         DefRangeRegisterHeader DRHdr;
         DRHdr.Register = DefRange.CVRegister;
         DRHdr.MayHaveNoName = 0;
-        OS.emitCVDefRangeDirective(DefRange.Ranges, DRHdr);
+        OS.emitCVDefRangeDirective(Ranges, DRHdr);
       }
     }
   }
 }
 
 void CodeViewDebug::emitLexicalBlockList(ArrayRef<LexicalBlock *> Blocks,
                                          const FunctionInfo& FI) {
   for (LexicalBlock *Block : Blocks)

llvm/test/DebugInfo/COFF/pieces.ll

 ; ASM: [[spill_o_x_end_func:\.Ltmp[0-9]+]]:
 ; ASM-NEXT: .Lfunc_end4:
 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "loop_csr"              # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    [[oy_ox_start]] [[ox_start]], subfield_reg, 24, 0
-; ASM:        .cv_def_range    [[oy_ox_start]] [[oy_start]], subfield_reg, 23, 4
-; ASM:        .cv_def_range    [[ox_start]] [[loopskip_start]], subfield_reg, 24, 0
-; ASM:        .cv_def_range    [[oy_start]] [[loopskip_start]], subfield_reg, 23, 4
+; ASM:        .cv_def_range    [[oy_ox_start]] [[loopskip_start]], subfield_reg, 24, 0
+; ASM:        .cv_def_range    [[oy_ox_start]] [[loopskip_start]], subfield_reg, 23, 4
 
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: loop_csr
 ; OBJ:       }
 ; OBJ:       LocalSym {
 ; OBJ:         VarName: o