labath accepted this revision.
labath added a comment.
Yeah, looks good. Some small suggestions inline, though some of them may be
better off as separate patches...
================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:207-215
+static unsigned getOffsetSize(const DWARFYAML::Unit &Unit) {
+ return Unit.Format == dwarf::DWARF64 ? 8 : 4;
+}
- void onValue(const uint16_t U) override {
- writeInteger(U, OS, DebugInfo.IsLittleEndian);
- }
+static unsigned getRefSize(const DWARFYAML::Unit &Unit) {
+ if (Unit.Version == 2)
+ return Unit.AddrSize;
----------------
Maybe this is for a separate patch, since you're just moving these, but this
code already exists in BinaryFormat/Dwarf.h. It would be nice if
DWARFYaml::Unit had a dwarf::FormParams field or a getter, and then these
things could be retrieved by querying that object...
================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:349-353
+ // sizeof(version) 2 + sizeof(address_size) 1 = 3
+ // sizeof(unit_type) = 1 (DWARFv5) or 0
+ // sizeof(debug_abbrev_offset) = 4 (DWARF32) or 8 (DWARF64)
+ uint64_t Length = 3 + (Unit.Version >= 5 ? 1 : 0) +
+ (Unit.Format == dwarf::DWARF64 ? 8 : 4);
----------------
Since the comments repeat the entire expression, it would be less repetitive to
just structure the code in a way that comments can be attached to the actual
expressions. Maybe something like:
```
Lenght = 3; // version + address_size
Length += (Unit.Version >= 5 ? 1 : 0); // unit_type
Length += Unit.getFormParams().getDwarfOffsetByteSize(); // abbrev_offset
```
================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:380-382
+ cantFail(writeVariableSizedInteger(Unit.AbbrOffset,
+ Unit.Format == dwarf::DWARF64 ? 8 : 4,
+ OS, DI.IsLittleEndian));
----------------
a writeDwarfOffset function might be handy.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84008/new/
https://reviews.llvm.org/D84008
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
