[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

78

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.

91

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.

520

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

521

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

 #define LLVM_LIB_CODEGEN_ASMPRINTER_CODEVIEWDEBUG_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/DbgEntityHistoryCalculator.h"
 #include "llvm/CodeGen/DebugHandlerBase.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/MC/MCSymbol.h"
 #include <cstdint>
 #include <map>
 #include <string>
 #include <tuple>
 #include <unordered_map>
 #include <utility>
 #include <vector>
 
 namespace llvm {
 
 struct ClassInfo;
 class StringRef;
 class AsmPrinter;
 class Function;
 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.
+public:
   struct LocalVarDefRange {
     /// 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;
+    struct DenseMapInfo {
+      static inline LocalVarDefRange getEmptyKey() {
+        uint64_t Empty = ~0UL;
+        LocalVarDefRange Range;
+        std::memcpy(&Range, &Empty, 8);
+        return Range;
+      }
+
+      static inline LocalVarDefRange getTombstoneKey() {
+        uint64_t Tombstone = ~0UL - 1L;
+        LocalVarDefRange Range;
+        std::memcpy(&Range, &Tombstone, 8);
+        return Range;
+      }
+
+      static unsigned getHashValue(const LocalVarDefRange &DR) {
+        uint64_t Val;
+        std::memcpy(&Val, &DR, 8);
+        return Val * 37UL;
       }
 
-    SmallVector<std::pair<const MCSymbol *, const MCSymbol *>, 1> Ranges;
+      static bool isEqual(const LocalVarDefRange &LHS,
+                          const LocalVarDefRange &RHS) {
+        return std::memcmp(&LHS, &RHS, 8) == 0;
+      }
     };
+  };
+
+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 LocalVarDefRange 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<LocalVarDefRange,
+              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::LocalVarDefRange>
+    : CodeViewDebug::LocalVarDefRange::DenseMapInfo {};
+
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_ASMPRINTER_CODEVIEWDEBUG_H

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

for (const MachineFunction::VariableDbgInfo &VI : MF.getVariableDbgInfo()) {
 
     assert(!FrameOffset.getScalable() &&
            "Frame offsets with a scalable component are not supported");
 
     // Calculate the label ranges.
     LocalVarDefRange 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;
-      }
+
 
-      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) {
+    LocalVarDefRange 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