[PATCH] D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added inline comments.


================
Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:1645
   Record.push_back(VE.getMetadataOrNullID(N->getType()));
-  Record.push_back(N->isLocalToUnit());
-  Record.push_back(N->isDefinition());
+  Record.push_back(0); // unused
+  Record.push_back(0); // unused
----------------
dblaikie wrote:
> probinson wrote:
> > aprantl wrote:
> > > Am I right in assuming that these dead fields take up virtually no extra space in the bitcode? Otherwise we might want to add an extra bit distinguish old vs. a newer, more compact record layout.
> > Each field in Record is 64-bit, so as written this is wasting 24 bytes per DISubprogram in the bitcode. Compressing those out would mean the reader would pretty much have to handle the old/new formats separately, as the indexes of all fields after this one would vary too much to handle cleanly any other way.
> > I'm happy to make that change if you think the size is worthwhile.
> I don't /think/ that's how the bitcode format works, though, right? I believe it's a more compact encoding than using all 64 bits regardless of the value encoded... but I don't know the specifics/not very sure about it.
@dblaikie should be right.  IIRC the default (in absence of an explicit abbreviation) is vbr-6, so that's 6 bits times three for 18 bits.

That said, I'm not convinced this would be hard to deal with in the reader.  There's probably a way to reorganize the logic to avoid forking.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54755/new/

https://reviews.llvm.org/D54755