[ADT][DebugInfo][RemoveDIs] Add extra bits to ilist_iterator for debug-info

...behind an experimental CMAKE option that's off by default.

This patch adds a new ilist-iterator-like class that can carry two extra bits
as well as the usual node pointer. This is part of the project to remove
debug-intrinsics from LLVM: see the rationale here [0], they're needed to
signal whether a "position" in a BasicBlock includes any debug-info before or
after the iterator.

This entirely duplicates ilist_iterator, attempting re-use showed it to be a
false economy. It's enable-able through the existing ilist_node options
interface, hence a few sites where the instruction-list type needs to be
updated. The actual main feature, the extra bits in the class, aren't part of
the class unless the cmake flag is given: this is because there's a
compile-time cost associated with it, and I'd like to get everything in-tree
but off-by-default so that we can do proper comparisons.

Nothing actually makes use of this yet, but will do soon, see the Phab patch
stack.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939

Differential Revision: https://reviews.llvm.org/D153777

Author: jmorse

llvm/CMakeLists.txt

@@ -643,6 +643,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS
+  "Add extra Booleans to ilist_iterators to communicate facts for debug-info" OFF)
+
 set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
   "Sign executables and dylibs with the given identity or skip if empty (Darwin Only)")
 

llvm/cmake/modules/HandleLLVMOptions.cmake

@@ -109,6 +109,10 @@ if(LLVM_ENABLE_EXPENSIVE_CHECKS)
   endif()
 endif()
 
+if(LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS)
+  add_compile_definitions(EXPERIMENTAL_DEBUGINFO_ITERATORS)
+endif()
+
 if (LLVM_ENABLE_STRICT_FIXED_SIZE_VECTORS)
   add_compile_definitions(STRICT_FIXED_SIZE_VECTORS)
 endif()

llvm/include/llvm/ADT/ilist_iterator.h

@@ -175,6 +175,185 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT> {
   bool isEnd() const { return NodePtr ? NodePtr->isSentinel() : false; }
 };
 
+/// Iterator for intrusive lists  based on ilist_node. Much like ilist_iterator,
+/// but with the addition of two bits recording whether this position (when in
+/// a range) is half or fully open.
+template <class OptionsT, bool IsReverse, bool IsConst>
+class ilist_iterator_w_bits : ilist_detail::SpecificNodeAccess<OptionsT> {
+  friend ilist_iterator_w_bits<OptionsT, IsReverse, !IsConst>;
+  friend ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst>;
+  friend ilist_iterator<OptionsT, !IsReverse, !IsConst>;
+
+  using Traits = ilist_detail::IteratorTraits<OptionsT, IsConst>;
+  using Access = ilist_detail::SpecificNodeAccess<OptionsT>;
+
+public:
+  using value_type = typename Traits::value_type;
+  using pointer = typename Traits::pointer;
+  using reference = typename Traits::reference;
+  using difference_type = ptrdiff_t;
+  using iterator_category = std::bidirectional_iterator_tag;
+  using const_pointer = typename OptionsT::const_pointer;
+  using const_reference = typename OptionsT::const_reference;
+
+private:
+  using node_pointer = typename Traits::node_pointer;
+  using node_reference = typename Traits::node_reference;
+
+  node_pointer NodePtr = nullptr;
+
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+  // (Default: Off) Allow extra position-information flags to be stored
+  // in iterators, in aid of removing debug-info intrinsics from LLVM.
+
+  /// Is this position intended to contain any debug-info immediately before
+  /// the position?
+  mutable bool HeadInclusiveBit = false;
+  /// Is this position intended to contain any debug-info immediately after
+  /// the position?
+  mutable bool TailInclusiveBit = false;
+#endif
+
+public:
+  /// Create from an ilist_node.
+  explicit ilist_iterator_w_bits(node_reference N) : NodePtr(&N) {}
+
+  explicit ilist_iterator_w_bits(pointer NP)
+      : NodePtr(Access::getNodePtr(NP)) {}
+  explicit ilist_iterator_w_bits(reference NR)
+      : NodePtr(Access::getNodePtr(&NR)) {}
+  ilist_iterator_w_bits() = default;
+
+  // This is templated so that we can allow constructing a const iterator from
+  // a nonconst iterator...
+  template <bool RHSIsConst>
+  ilist_iterator_w_bits(
+      const ilist_iterator_w_bits<OptionsT, IsReverse, RHSIsConst> &RHS,
+      std::enable_if_t<IsConst || !RHSIsConst, void *> = nullptr)
+      : NodePtr(RHS.NodePtr) {
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+    HeadInclusiveBit = RHS.HeadInclusiveBit;
+    TailInclusiveBit = RHS.TailInclusiveBit;
+#endif
+  }
+
+  // This is templated so that we can allow assigning to a const iterator from
+  // a nonconst iterator...
+  template <bool RHSIsConst>
+  std::enable_if_t<IsConst || !RHSIsConst, ilist_iterator_w_bits &>
+  operator=(const ilist_iterator_w_bits<OptionsT, IsReverse, RHSIsConst> &RHS) {
+    NodePtr = RHS.NodePtr;
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+    HeadInclusiveBit = RHS.HeadInclusiveBit;
+    TailInclusiveBit = RHS.TailInclusiveBit;
+#endif
+    return *this;
+  }
+
+  /// Explicit conversion between forward/reverse iterators.
+  ///
+  /// Translate between forward and reverse iterators without changing range
+  /// boundaries.  The resulting iterator will dereference (and have a handle)
+  /// to the previous node, which is somewhat unexpected; but converting the
+  /// two endpoints in a range will give the same range in reverse.
+  ///
+  /// This matches std::reverse_iterator conversions.
+  explicit ilist_iterator_w_bits(
+      const ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst> &RHS)
+      : ilist_iterator_w_bits(++RHS.getReverse()) {}
+
+  /// Get a reverse iterator to the same node.
+  ///
+  /// Gives a reverse iterator that will dereference (and have a handle) to the
+  /// same node.  Converting the endpoint iterators in a range will give a
+  /// different range; for range operations, use the explicit conversions.
+  ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst> getReverse() const {
+    if (NodePtr)
+      return ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst>(*NodePtr);
+    return ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst>();
+  }
+
+  /// Const-cast.
+  ilist_iterator_w_bits<OptionsT, IsReverse, false> getNonConst() const {
+    if (NodePtr) {
+      auto New = ilist_iterator_w_bits<OptionsT, IsReverse, false>(
+          const_cast<typename ilist_iterator_w_bits<OptionsT, IsReverse,
+                                                    false>::node_reference>(
+              *NodePtr));
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+      New.HeadInclusiveBit = HeadInclusiveBit;
+      New.TailInclusiveBit = TailInclusiveBit;
+#endif
+      return New;
+    }
+    return ilist_iterator_w_bits<OptionsT, IsReverse, false>();
+  }
+
+  // Accessors...
+  reference operator*() const {
+    assert(!NodePtr->isKnownSentinel());
+    return *Access::getValuePtr(NodePtr);
+  }
+  pointer operator->() const { return &operator*(); }
+
+  // Comparison operators
+  friend bool operator==(const ilist_iterator_w_bits &LHS,
+                         const ilist_iterator_w_bits &RHS) {
+    return LHS.NodePtr == RHS.NodePtr;
+  }
+  friend bool operator!=(const ilist_iterator_w_bits &LHS,
+                         const ilist_iterator_w_bits &RHS) {
+    return LHS.NodePtr != RHS.NodePtr;
+  }
+
+  // Increment and decrement operators...
+  ilist_iterator_w_bits &operator--() {
+    NodePtr = IsReverse ? NodePtr->getNext() : NodePtr->getPrev();
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+    HeadInclusiveBit = false;
+    TailInclusiveBit = false;
+#endif
+    return *this;
+  }
+  ilist_iterator_w_bits &operator++() {
+    NodePtr = IsReverse ? NodePtr->getPrev() : NodePtr->getNext();
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+    HeadInclusiveBit = false;
+    TailInclusiveBit = false;
+#endif
+    return *this;
+  }
+  ilist_iterator_w_bits operator--(int) {
+    ilist_iterator_w_bits tmp = *this;
+    --*this;
+    return tmp;
+  }
+  ilist_iterator_w_bits operator++(int) {
+    ilist_iterator_w_bits tmp = *this;
+    ++*this;
+    return tmp;
+  }
+
+  /// Get the underlying ilist_node.
+  node_pointer getNodePtr() const { return static_cast<node_pointer>(NodePtr); }
+
+  /// Check for end.  Only valid if ilist_sentinel_tracking<true>.
+  bool isEnd() const { return NodePtr ? NodePtr->isSentinel() : false; }
+
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+  bool getHeadBit() const { return HeadInclusiveBit; }
+  bool getTailBit() const { return TailInclusiveBit; }
+  void setHeadBit(bool SetBit) const { HeadInclusiveBit = SetBit; }
+  void setTailBit(bool SetBit) const { TailInclusiveBit = SetBit; }
+#else
+  // Store and return no information if we're not using this feature.
+  bool getHeadBit() const { return false; }
+  bool getTailBit() const { return false; }
+  void setHeadBit(bool SetBit) const { (void)SetBit; }
+  void setTailBit(bool SetBit) const { (void)SetBit; }
+#endif
+};
+
 template <typename From> struct simplify_type;
 
 /// Allow ilist_iterators to convert into pointers to a node automatically when
@@ -192,6 +371,18 @@ template <class OptionsT, bool IsConst>
 struct simplify_type<const ilist_iterator<OptionsT, false, IsConst>>
     : simplify_type<ilist_iterator<OptionsT, false, IsConst>> {};
 
+// ilist_iterator_w_bits should also be accessible via isa/dyn_cast.
+template <class OptionsT, bool IsConst>
+struct simplify_type<ilist_iterator_w_bits<OptionsT, false, IsConst>> {
+  using iterator = ilist_iterator_w_bits<OptionsT, false, IsConst>;
+  using SimpleType = typename iterator::pointer;
+
+  static SimpleType getSimplifiedValue(const iterator &Node) { return &*Node; }
+};
+template <class OptionsT, bool IsConst>
+struct simplify_type<const ilist_iterator_w_bits<OptionsT, false, IsConst>>
+    : simplify_type<ilist_iterator_w_bits<OptionsT, false, IsConst>> {};
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_ILIST_ITERATOR_H

llvm/include/llvm/ADT/ilist_node.h

@@ -27,8 +27,22 @@ struct NodeAccess;
 } // end namespace ilist_detail
 
 template <class OptionsT, bool IsReverse, bool IsConst> class ilist_iterator;
+template <class OptionsT, bool IsReverse, bool IsConst>
+class ilist_iterator_w_bits;
 template <class OptionsT> class ilist_sentinel;
 
+// Selector for which iterator type to pick given the iterator-bits node option.
+template <bool use_iterator_bits, typename Opts, bool arg1, bool arg2>
+class ilist_select_iterator_type {
+public:
+  using type = ilist_iterator<Opts, arg1, arg2>;
+};
+template <typename Opts, bool arg1, bool arg2>
+class ilist_select_iterator_type<true, Opts, arg1, arg2> {
+public:
+  using type = ilist_iterator_w_bits<Opts, arg1, arg2>;
+};
+
 /// Implementation for an ilist node.
 ///
 /// Templated on an appropriate \a ilist_detail::node_options, usually computed
@@ -45,16 +59,29 @@ template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
   friend typename OptionsT::list_base_type;
   friend struct ilist_detail::NodeAccess;
   friend class ilist_sentinel<OptionsT>;
+
   friend class ilist_iterator<OptionsT, false, false>;
   friend class ilist_iterator<OptionsT, false, true>;
   friend class ilist_iterator<OptionsT, true, false>;
   friend class ilist_iterator<OptionsT, true, true>;
+  friend class ilist_iterator_w_bits<OptionsT, false, false>;
+  friend class ilist_iterator_w_bits<OptionsT, false, true>;
+  friend class ilist_iterator_w_bits<OptionsT, true, false>;
+  friend class ilist_iterator_w_bits<OptionsT, true, true>;
 
 protected:
-  using self_iterator = ilist_iterator<OptionsT, false, false>;
-  using const_self_iterator = ilist_iterator<OptionsT, false, true>;
-  using reverse_self_iterator = ilist_iterator<OptionsT, true, false>;
-  using const_reverse_self_iterator = ilist_iterator<OptionsT, true, true>;
+  using self_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          false, false>::type;
+  using const_self_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          false, true>::type;
+  using reverse_self_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          true, false>::type;
+  using const_reverse_self_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          true, true>::type;
 
   ilist_node_impl() = default;
 

llvm/include/llvm/ADT/ilist_node_options.h

@@ -31,6 +31,14 @@ template <bool EnableSentinelTracking> struct ilist_sentinel_tracking {};
 /// simultaneously.  See \a ilist_node for usage examples.
 template <class Tag> struct ilist_tag {};
 
+/// Option to add extra bits to the ilist_iterator.
+///
+/// Some use-cases (debug-info) need to know whether a position is intended
+/// to be half-open or fully open, i.e. whether to include any immediately
+/// adjacent debug-info in an operation. This option adds two bits to the
+/// iterator class to store that information.
+template <bool ExtraIteratorBits> struct ilist_iterator_bits {};
+
 namespace ilist_detail {
 
 /// Helper trait for recording whether an option is specified explicitly.
@@ -91,6 +99,21 @@ template <> struct extract_tag<> {
 };
 template <class Tag> struct is_valid_option<ilist_tag<Tag>> : std::true_type {};
 
+/// Extract iterator bits option.
+///
+/// Look through \p Options for the \a ilist_iterator_bits option. Defaults
+/// to false.
+template <class... Options> struct extract_iterator_bits;
+template <bool IteratorBits, class... Options>
+struct extract_iterator_bits<ilist_iterator_bits<IteratorBits>, Options...>
+    : std::integral_constant<bool, IteratorBits> {};
+template <class Option1, class... Options>
+struct extract_iterator_bits<Option1, Options...>
+    : extract_iterator_bits<Options...> {};
+template <> struct extract_iterator_bits<> : std::false_type, is_implicit {};
+template <bool IteratorBits>
+struct is_valid_option<ilist_iterator_bits<IteratorBits>> : std::true_type {};
+
 /// Check whether options are valid.
 ///
 /// The conjunction of \a is_valid_option on each individual option.
@@ -105,7 +128,7 @@ struct check_options<Option1, Options...>
 ///
 /// This is usually computed via \a compute_node_options.
 template <class T, bool EnableSentinelTracking, bool IsSentinelTrackingExplicit,
-          class TagT>
+          class TagT, bool HasIteratorBits>
 struct node_options {
   typedef T value_type;
   typedef T *pointer;
@@ -115,6 +138,7 @@ struct node_options {
 
   static const bool enable_sentinel_tracking = EnableSentinelTracking;
   static const bool is_sentinel_tracking_explicit = IsSentinelTrackingExplicit;
+  static const bool has_iterator_bits = HasIteratorBits;
   typedef TagT tag;
   typedef ilist_node_base<enable_sentinel_tracking> node_base_type;
   typedef ilist_base<enable_sentinel_tracking> list_base_type;
@@ -123,7 +147,8 @@ struct node_options {
 template <class T, class... Options> struct compute_node_options {
   typedef node_options<T, extract_sentinel_tracking<Options...>::value,
                        extract_sentinel_tracking<Options...>::is_explicit,
-                       typename extract_tag<Options...>::type>
+                       typename extract_tag<Options...>::type,
+                       extract_iterator_bits<Options...>::value>
       type;
 };
 

llvm/include/llvm/ADT/simple_ilist.h

@@ -92,10 +92,18 @@ class simple_ilist
   using reference = typename OptionsT::reference;
   using const_pointer = typename OptionsT::const_pointer;
   using const_reference = typename OptionsT::const_reference;
-  using iterator = ilist_iterator<OptionsT, false, false>;
-  using const_iterator = ilist_iterator<OptionsT, false, true>;
-  using reverse_iterator = ilist_iterator<OptionsT, true, false>;
-  using const_reverse_iterator = ilist_iterator<OptionsT, true, true>;
+  using iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          false, false>::type;
+  using const_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          false, true>::type;
+  using reverse_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          true, false>::type;
+  using const_reverse_iterator =
+      typename ilist_select_iterator_type<OptionsT::has_iterator_bits, OptionsT,
+                                          true, true>::type;
   using size_type = size_t;
   using difference_type = ptrdiff_t;