[PATCH] D40631: Split TypeTableBuilder into two classes

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 29 17:03:39 PST 2017


zturner created this revision.
Herald added subscribers: JDevlieghere, hiraditya, mgorny.

I struggled with this for quite a while, but the motivation for this is the desire to add a different hashing strategy that can be selected at runtime while maintaining backwards compatibility with the existing hashing strategy until the new method is robust and we can be certain it does not cause any regressions (correctness or performance).

The problem is that we pass `TypeTableBuilder` references around in many scenarios, but a lot of the places we use them are ignorant of whether or not the type table is in de-duplication mode.  On the surface this seems neat, but we need to be able to change the interface in ways that only make sense when the builder is hashing.  If we try to cram it all into a single interface again, I think we run into the problem we had before with `TypeSerializer` where it was just doing too much, and as a result there were a lot of strange states you get the class into.

It turns out that all of the places that were agnostic about the de-duplication mode was just by chance.  In all of those cases, there was actually only one single mode that made sense, so splitting the class into a de-duplicating type table and an appending type table did not cause any serious problems.

After this patch, the two interfaces will be able to diverge so that we can provide a better interface for the hashing version of `TypeTableBuilder`.


https://reviews.llvm.org/D40631

Files:
  lld/COFF/PDB.cpp
  llvm/include/llvm/DebugInfo/CodeView/AppendingTypeTableBuilder.h
  llvm/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h
  llvm/include/llvm/DebugInfo/CodeView/TypeStreamMerger.h
  llvm/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
  llvm/include/llvm/ObjectYAML/CodeViewYAMLTypes.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/CMakeLists.txt
  llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
  llvm/lib/DebugInfo/CodeView/TypeTableBuilder.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
  llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp
  llvm/tools/llvm-readobj/COFFDumper.cpp
  llvm/tools/llvm-readobj/ObjDumper.h
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
  llvm/unittests/DebugInfo/CodeView/TypeIndexDiscoveryTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40631.124856.patch
Type: text/x-patch
Size: 35124 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171130/add4329f/attachment.bin>


More information about the llvm-commits mailing list