[PATCH] D153777: [ADT][DebugInfo][RemoveDIs] Permit extra flags in ilist_iterator's for communicating debug-info facts

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando accepted this revision.
Orlando added a comment.
This revision is now accepted and ready to land.

LGTM (pending clang-formatting if not already, plus a few nits).

> The exact cost of these bits being present is something like 0.1% according to the LLVM compile-time-tracker (more than offset by future savings), but given that this overall (removing debug intrinsics) feature is going to be in development and evaluation for a while, it doesn't make sense for the compile-time cost to be paid up front.

This makes sense to me.



================
Comment at: llvm/CMakeLists.txt:627
+option(LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS
+  "Add extra Booleans to ilist_iterator's to communicate facts for debug-info" OFF)
+
----------------
nit: no ilist_iterator's -> ilist_iterators, but the singular instead works too imo.



================
Comment at: llvm/include/llvm/IR/SymbolTableListTraits.h:114-117
+template <class T, typename... Args>
+class SymbolTableList : public iplist_impl<simple_ilist<T, Args...>,
+                                           SymbolTableListTraits<T, Args...>> {
+};
----------------
has this been clang-formatted?


================
Comment at: llvm/lib/IR/BasicBlock.cpp:229
   BasicBlock::const_iterator It = I->getIterator();
+  It.setHeadBit(true);
   return It;
----------------
Worth copying the comment you added to `getFirstInsertionPt` here too?


================
Comment at: llvm/unittests/ADT/IListIteratorBitsTest.cpp:99-100
+
+  // Test that a node that did not have the iterator bits option given to it
+  // does not get the "with bits" iterator type.
+}
----------------
Comment floated here from elsewhere?


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

https://reviews.llvm.org/D153777