[PATCH] D42501: [dsymutil] Generate Apple accelerator tables

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 25 14:44:13 PST 2018


aprantl added inline comments.


================
Comment at: tools/dsymutil/DwarfLinker.cpp:104
+/// a DIE offset but no actual DIE pointer.
+class AppleAccelTableStaticOffsetData : public AppleAccelTableData {
+public:
----------------
It seems odd to have this implementation live in DwarfLinker. I would have expected this to be part of AsmPrinter. Out of curiosity: Why is it not needed there? Is there something else that is only on opensource.apple.com and should really be on llvm.org?


================
Comment at: tools/dsymutil/DwarfLinker.cpp:465
+
+    /// Hash of the fully qualified name
+    uint32_t QualifiedNameHash;
----------------
`.`


================
Comment at: tools/dsymutil/DwarfLinker.cpp:642
+
+/// Add a name accelerator entry.
+void CompileUnit::addNameAccelerator(const DIE *Die,
----------------
These comments are not strictly necessary and basically just repeat the function name :-)


================
Comment at: tools/dsymutil/DwarfLinker.cpp:1597
+      /// Offset to apply to PC addresses inside a function.
+      int64_t PCOffset = 0;
+
----------------
Does moving this field improve the layout somehow?


================
Comment at: tools/dsymutil/dsymutil.h:43
+  /// Minimize
+  bool Minimize = false;
+
----------------
It looks like this option is entirely untested?


https://reviews.llvm.org/D42501





More information about the llvm-commits mailing list