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

Jeremy Morse via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 17 07:26:51 PDT 2023


Author: Jeremy Morse
Date: 2023-10-17T15:24:44+01:00
New Revision: 088d272e83259a5d8e577a3d2e62012c42a9f9db

URL: https://github.com/llvm/llvm-project/commit/088d272e83259a5d8e577a3d2e62012c42a9f9db
DIFF: https://github.com/llvm/llvm-project/commit/088d272e83259a5d8e577a3d2e62012c42a9f9db.diff

LOG: [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

Added: 
    llvm/unittests/ADT/IListIteratorBitsTest.cpp

Modified: 
    llvm/CMakeLists.txt
    llvm/cmake/modules/HandleLLVMOptions.cmake
    llvm/include/llvm/ADT/ilist_iterator.h
    llvm/include/llvm/ADT/ilist_node.h
    llvm/include/llvm/ADT/ilist_node_options.h
    llvm/include/llvm/ADT/simple_ilist.h
    llvm/include/llvm/IR/BasicBlock.h
    llvm/include/llvm/IR/GlobalAlias.h
    llvm/include/llvm/IR/GlobalIFunc.h
    llvm/include/llvm/IR/GlobalVariable.h
    llvm/include/llvm/IR/Instruction.h
    llvm/include/llvm/IR/Instructions.h
    llvm/include/llvm/IR/SymbolTableListTraits.h
    llvm/include/llvm/IR/ValueSymbolTable.h
    llvm/lib/IR/BasicBlock.cpp
    llvm/lib/IR/Instruction.cpp
    llvm/lib/IR/Instructions.cpp
    llvm/lib/IR/SymbolTableListTraitsImpl.h
    llvm/unittests/ADT/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 103c08ffbe83b38..ef2f2146a036448 100644
--- a/llvm/CMakeLists.txt
+++ b/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)")
 

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 19cb881adc3fa95..56b63cb5acb81e6 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/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()

diff  --git a/llvm/include/llvm/ADT/ilist_iterator.h b/llvm/include/llvm/ADT/ilist_iterator.h
index be876347907bb2b..9047b9b73959eea 100644
--- a/llvm/include/llvm/ADT/ilist_iterator.h
+++ b/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 
diff erence_type = ptr
diff _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
+  /// 
diff erent 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

diff  --git a/llvm/include/llvm/ADT/ilist_node.h b/llvm/include/llvm/ADT/ilist_node.h
index 7856b1c0d410e2c..3b6f0dcc7b5e932 100644
--- a/llvm/include/llvm/ADT/ilist_node.h
+++ b/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;
 

diff  --git a/llvm/include/llvm/ADT/ilist_node_options.h b/llvm/include/llvm/ADT/ilist_node_options.h
index 05340d344e39988..e6e1068953e36ad 100644
--- a/llvm/include/llvm/ADT/ilist_node_options.h
+++ b/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;
 };
 

diff  --git a/llvm/include/llvm/ADT/simple_ilist.h b/llvm/include/llvm/ADT/simple_ilist.h
index 3a96e1ba56575d6..7236b3fa5a7d2d0 100644
--- a/llvm/include/llvm/ADT/simple_ilist.h
+++ b/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 
diff erence_type = ptr
diff _t;
 

diff  --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index b031f72493e13d0..ab291c24e5b6c75 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -55,7 +55,7 @@ class ValueSymbolTable;
 class BasicBlock final : public Value, // Basic blocks are data objects also
                          public ilist_node_with_parent<BasicBlock, Function> {
 public:
-  using InstListType = SymbolTableList<Instruction>;
+  using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>>;
 
 private:
   friend class BlockAddress;
@@ -91,11 +91,13 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
 
   // These functions and classes need access to the instruction list.
   friend void Instruction::removeFromParent();
-  friend iplist<Instruction>::iterator Instruction::eraseFromParent();
+  friend BasicBlock::iterator Instruction::eraseFromParent();
   friend BasicBlock::iterator Instruction::insertInto(BasicBlock *BB,
                                                       BasicBlock::iterator It);
-  friend class llvm::SymbolTableListTraits<llvm::Instruction>;
-  friend class llvm::ilist_node_with_parent<llvm::Instruction, llvm::BasicBlock>;
+  friend class llvm::SymbolTableListTraits<llvm::Instruction,
+                                           ilist_iterator_bits<true>>;
+  friend class llvm::ilist_node_with_parent<llvm::Instruction, llvm::BasicBlock,
+                                            ilist_iterator_bits<true>>;
 
   /// Creates a new BasicBlock.
   ///
@@ -178,7 +180,8 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   InstListType::const_iterator getFirstNonPHIIt() const;
   InstListType::iterator getFirstNonPHIIt() {
     BasicBlock::iterator It =
-      static_cast<const BasicBlock *>(this)->getFirstNonPHIIt().getNonConst();
+        static_cast<const BasicBlock *>(this)->getFirstNonPHIIt().getNonConst();
+    It.setHeadBit(true);
     return It;
   }
 
@@ -332,8 +335,19 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   //===--------------------------------------------------------------------===//
   /// Instruction iterator methods
   ///
-  inline iterator                begin()       { return InstList.begin(); }
-  inline const_iterator          begin() const { return InstList.begin(); }
+  inline iterator begin() {
+    iterator It = InstList.begin();
+    // Set the head-inclusive bit to indicate that this iterator includes
+    // any debug-info at the start of the block. This is a no-op unless the
+    // appropriate CMake flag is set.
+    It.setHeadBit(true);
+    return It;
+  }
+  inline const_iterator begin() const {
+    const_iterator It = InstList.begin();
+    It.setHeadBit(true);
+    return It;
+  }
   inline iterator                end  ()       { return InstList.end();   }
   inline const_iterator          end  () const { return InstList.end();   }
 

diff  --git a/llvm/include/llvm/IR/GlobalAlias.h b/llvm/include/llvm/IR/GlobalAlias.h
index de405da5ca23173..583d66e28155d76 100644
--- a/llvm/include/llvm/IR/GlobalAlias.h
+++ b/llvm/include/llvm/IR/GlobalAlias.h
@@ -23,7 +23,7 @@ namespace llvm {
 
 class Twine;
 class Module;
-template <typename ValueSubClass> class SymbolTableListTraits;
+template <typename ValueSubClass, typename... Args> class SymbolTableListTraits;
 
 class GlobalAlias : public GlobalValue, public ilist_node<GlobalAlias> {
   friend class SymbolTableListTraits<GlobalAlias>;

diff  --git a/llvm/include/llvm/IR/GlobalIFunc.h b/llvm/include/llvm/IR/GlobalIFunc.h
index c148ee7907789bc..4d1982da0baff38 100644
--- a/llvm/include/llvm/IR/GlobalIFunc.h
+++ b/llvm/include/llvm/IR/GlobalIFunc.h
@@ -29,7 +29,7 @@ class Twine;
 class Module;
 
 // Traits class for using GlobalIFunc in symbol table in Module.
-template <typename ValueSubClass> class SymbolTableListTraits;
+template <typename ValueSubClass, typename... Args> class SymbolTableListTraits;
 
 class GlobalIFunc final : public GlobalObject, public ilist_node<GlobalIFunc> {
   friend class SymbolTableListTraits<GlobalIFunc>;

diff  --git a/llvm/include/llvm/IR/GlobalVariable.h b/llvm/include/llvm/IR/GlobalVariable.h
index 03c680e4f95585b..f915dba5c6595df 100644
--- a/llvm/include/llvm/IR/GlobalVariable.h
+++ b/llvm/include/llvm/IR/GlobalVariable.h
@@ -33,7 +33,7 @@ namespace llvm {
 class Constant;
 class Module;
 
-template <typename ValueSubClass> class SymbolTableListTraits;
+template <typename ValueSubClass, typename... Args> class SymbolTableListTraits;
 class DIGlobalVariableExpression;
 
 class GlobalVariable : public GlobalObject, public ilist_node<GlobalVariable> {

diff  --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 69c3af5b76103f6..af7aa791cb6da60 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -39,7 +39,11 @@ template <> struct ilist_alloc_traits<Instruction> {
 };
 
 class Instruction : public User,
-                    public ilist_node_with_parent<Instruction, BasicBlock> {
+                    public ilist_node_with_parent<Instruction, BasicBlock,
+                                                  ilist_iterator_bits<true>> {
+public:
+  using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>>;
+private:
   BasicBlock *Parent;
   DebugLoc DbgLoc;                         // 'dbg' Metadata cache.
 
@@ -118,12 +122,12 @@ class Instruction : public User,
   /// This method unlinks 'this' from the containing basic block and deletes it.
   ///
   /// \returns an iterator pointing to the element after the erased one
-  SymbolTableList<Instruction>::iterator eraseFromParent();
+  InstListType::iterator eraseFromParent();
 
   /// Insert an unlinked instruction into a basic block immediately before
   /// the specified instruction.
   void insertBefore(Instruction *InsertPos);
-  void insertBefore(SymbolTableList<Instruction>::iterator InsertPos) {
+  void insertBefore(InstListType::iterator InsertPos) {
     insertBefore(&*InsertPos);
   }
 
@@ -133,11 +137,10 @@ class Instruction : public User,
 
   /// Inserts an unlinked instruction into \p ParentBB at position \p It and
   /// returns the iterator of the inserted instruction.
-  SymbolTableList<Instruction>::iterator
-  insertInto(BasicBlock *ParentBB, SymbolTableList<Instruction>::iterator It);
+  InstListType::iterator insertInto(BasicBlock *ParentBB,
+                                    InstListType::iterator It);
 
-  void insertBefore(BasicBlock &BB,
-                    SymbolTableList<Instruction>::iterator InsertPos) {
+  void insertBefore(BasicBlock &BB, InstListType::iterator InsertPos) {
     insertInto(&BB, InsertPos);
   }
 
@@ -157,10 +160,10 @@ class Instruction : public User,
   /// Unlink this instruction and insert into BB before I.
   ///
   /// \pre I is a valid iterator into BB.
-  void moveBefore(BasicBlock &BB, SymbolTableList<Instruction>::iterator I);
+  void moveBefore(BasicBlock &BB, InstListType::iterator I);
 
   /// (See other overload for moveBeforePreserving).
-  void moveBeforePreserving(BasicBlock &BB, SymbolTableList<Instruction>::iterator I) {
+  void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I) {
     moveBefore(BB, I);
   }
 
@@ -902,7 +905,7 @@ class Instruction : public User,
   };
 
 private:
-  friend class SymbolTableListTraits<Instruction>;
+  friend class SymbolTableListTraits<Instruction, ilist_iterator_bits<true>>;
   friend class BasicBlock; // For renumbering.
 
   // Shadow Value::setValueSubclassData with a private forwarding method so that

diff  --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index c85727ce30a94a4..af6ac566a0192bb 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -3661,7 +3661,7 @@ class SwitchInstProfUpdateWrapper {
 
   /// Delegate the call to the underlying SwitchInst::eraseFromParent() and mark
   /// this object to not touch the underlying SwitchInst in destructor.
-  SymbolTableList<Instruction>::iterator eraseFromParent();
+  Instruction::InstListType::iterator eraseFromParent();
 
   void setSuccessorWeight(unsigned idx, CaseWeightOpt W);
   CaseWeightOpt getSuccessorWeight(unsigned idx);

diff  --git a/llvm/include/llvm/IR/SymbolTableListTraits.h b/llvm/include/llvm/IR/SymbolTableListTraits.h
index 8af712374bfafd0..bd31fca5e525b60 100644
--- a/llvm/include/llvm/IR/SymbolTableListTraits.h
+++ b/llvm/include/llvm/IR/SymbolTableListTraits.h
@@ -57,15 +57,16 @@ DEFINE_SYMBOL_TABLE_PARENT_TYPE(GlobalAlias, Module)
 DEFINE_SYMBOL_TABLE_PARENT_TYPE(GlobalIFunc, Module)
 #undef DEFINE_SYMBOL_TABLE_PARENT_TYPE
 
-template <typename NodeTy> class SymbolTableList;
+template <typename NodeTy, typename... Args> class SymbolTableList;
 
 // ValueSubClass   - The type of objects that I hold, e.g. Instruction.
 // ItemParentClass - The type of object that owns the list, e.g. BasicBlock.
+// OptionsT        - Extra options to ilist nodes.
 //
-template <typename ValueSubClass>
+template <typename ValueSubClass, typename... Args>
 class SymbolTableListTraits : public ilist_alloc_traits<ValueSubClass> {
-  using ListTy = SymbolTableList<ValueSubClass>;
-  using iterator = typename simple_ilist<ValueSubClass>::iterator;
+  using ListTy = SymbolTableList<ValueSubClass, Args...>;
+  using iterator = typename simple_ilist<ValueSubClass, Args...>::iterator;
   using ItemParentClass =
       typename SymbolTableListParentType<ValueSubClass>::type;
 
@@ -110,9 +111,10 @@ class SymbolTableListTraits : public ilist_alloc_traits<ValueSubClass> {
 /// When nodes are inserted into and removed from this list, the associated
 /// symbol table will be automatically updated.  Similarly, parent links get
 /// updated automatically.
-template <class T>
-class SymbolTableList
-    : public iplist_impl<simple_ilist<T>, SymbolTableListTraits<T>> {};
+template <class T, typename... Args>
+class SymbolTableList : public iplist_impl<simple_ilist<T, Args...>,
+                                           SymbolTableListTraits<T, Args...>> {
+};
 
 } // end namespace llvm
 

diff  --git a/llvm/include/llvm/IR/ValueSymbolTable.h b/llvm/include/llvm/IR/ValueSymbolTable.h
index 43d00268f4b2218..6350f6a2435e472 100644
--- a/llvm/include/llvm/IR/ValueSymbolTable.h
+++ b/llvm/include/llvm/IR/ValueSymbolTable.h
@@ -27,8 +27,9 @@ class GlobalAlias;
 class GlobalIFunc;
 class GlobalVariable;
 class Instruction;
+template <bool ExtraIteratorBits> struct ilist_iterator_bits;
 template <unsigned InternalLen> class SmallString;
-template <typename ValueSubClass> class SymbolTableListTraits;
+template <typename ValueSubClass, typename ... Args> class SymbolTableListTraits;
 
 /// This class provides a symbol table of name/value pairs. It is essentially
 /// a std::map<std::string,Value*> but has a controlled interface provided by
@@ -41,7 +42,7 @@ class ValueSymbolTable {
   friend class SymbolTableListTraits<GlobalAlias>;
   friend class SymbolTableListTraits<GlobalIFunc>;
   friend class SymbolTableListTraits<GlobalVariable>;
-  friend class SymbolTableListTraits<Instruction>;
+  friend class SymbolTableListTraits<Instruction, ilist_iterator_bits<true>>;
   friend class Value;
 
 /// @name Types

diff  --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index d6677aa721bb050..46b1a3b37132bee 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -42,7 +42,8 @@ template <> void llvm::invalidateParentIListOrdering(BasicBlock *BB) {
 
 // Explicit instantiation of SymbolTableListTraits since some of the methods
 // are not in the public header file...
-template class llvm::SymbolTableListTraits<Instruction>;
+template class llvm::SymbolTableListTraits<Instruction,
+                                           ilist_iterator_bits<true>>;
 
 BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent,
                        BasicBlock *InsertBefore)
@@ -221,7 +222,13 @@ const Instruction* BasicBlock::getFirstNonPHI() const {
 }
 
 BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() const {
-  return getFirstNonPHI()->getIterator();
+  const Instruction *I = getFirstNonPHI();
+  BasicBlock::const_iterator It = I->getIterator();
+  // Set the head-inclusive bit to indicate that this iterator includes
+  // any debug-info at the start of the block. This is a no-op unless the
+  // appropriate CMake flag is set.
+  It.setHeadBit(true);
+  return It;
 }
 
 const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
@@ -261,6 +268,10 @@ BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
 
   const_iterator InsertPt = FirstNonPHI->getIterator();
   if (InsertPt->isEHPad()) ++InsertPt;
+  // Set the head-inclusive bit to indicate that this iterator includes
+  // any debug-info at the start of the block. This is a no-op unless the
+  // appropriate CMake flag is set.
+  InsertPt.setHeadBit(true);
   return InsertPt;
 }
 

diff  --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index b497951a598cc50..9b176eb78888e7c 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -80,7 +80,7 @@ void Instruction::removeFromParent() {
   getParent()->getInstList().remove(getIterator());
 }
 
-iplist<Instruction>::iterator Instruction::eraseFromParent() {
+BasicBlock::iterator Instruction::eraseFromParent() {
   return getParent()->getInstList().erase(getIterator());
 }
 
@@ -114,8 +114,7 @@ void Instruction::moveAfter(Instruction *MovePos) {
   moveBefore(*MovePos->getParent(), ++MovePos->getIterator());
 }
 
-void Instruction::moveBefore(BasicBlock &BB,
-                             SymbolTableList<Instruction>::iterator I) {
+void Instruction::moveBefore(BasicBlock &BB, InstListType::iterator I) {
   assert(I == BB.end() || I->getParent() == &BB);
   BB.splice(I, getParent(), getIterator());
 }

diff  --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index ece3b58792dd1f8..2ea9c05de6be298 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -4606,7 +4606,7 @@ void SwitchInstProfUpdateWrapper::addCase(
            "num of prof branch_weights must accord with num of successors");
 }
 
-SymbolTableList<Instruction>::iterator
+Instruction::InstListType::iterator
 SwitchInstProfUpdateWrapper::eraseFromParent() {
   // Instruction is erased. Mark as unchanged to not touch it in the destructor.
   Changed = false;

diff  --git a/llvm/lib/IR/SymbolTableListTraitsImpl.h b/llvm/lib/IR/SymbolTableListTraitsImpl.h
index 4283744bd058d44..990552f9b65a1e4 100644
--- a/llvm/lib/IR/SymbolTableListTraitsImpl.h
+++ b/llvm/lib/IR/SymbolTableListTraitsImpl.h
@@ -28,10 +28,10 @@ template <> void invalidateParentIListOrdering(BasicBlock *BB);
 /// setSymTabObject - This is called when (f.e.) the parent of a basic block
 /// changes.  This requires us to remove all the instruction symtab entries from
 /// the current function and reinsert them into the new function.
-template <typename ValueSubClass>
+template <typename ValueSubClass, typename... Args>
 template <typename TPtr>
-void SymbolTableListTraits<ValueSubClass>::setSymTabObject(TPtr *Dest,
-                                                           TPtr Src) {
+void SymbolTableListTraits<ValueSubClass, Args...>::setSymTabObject(TPtr *Dest,
+                                                                    TPtr Src) {
   // Get the old symtab and value list before doing the assignment.
   ValueSymbolTable *OldST = getSymTab(getListOwner());
 
@@ -61,11 +61,11 @@ void SymbolTableListTraits<ValueSubClass>::setSymTabObject(TPtr *Dest,
       if (I->hasName())
         NewST->reinsertValue(&*I);
   }
-
 }
 
-template <typename ValueSubClass>
-void SymbolTableListTraits<ValueSubClass>::addNodeToList(ValueSubClass *V) {
+template <typename ValueSubClass, typename... Args>
+void SymbolTableListTraits<ValueSubClass, Args...>::addNodeToList(
+    ValueSubClass *V) {
   assert(!V->getParent() && "Value already in a container!!");
   ItemParentClass *Owner = getListOwner();
   V->setParent(Owner);
@@ -75,8 +75,8 @@ void SymbolTableListTraits<ValueSubClass>::addNodeToList(ValueSubClass *V) {
       ST->reinsertValue(V);
 }
 
-template <typename ValueSubClass>
-void SymbolTableListTraits<ValueSubClass>::removeNodeFromList(
+template <typename ValueSubClass, typename... Args>
+void SymbolTableListTraits<ValueSubClass, Args...>::removeNodeFromList(
     ValueSubClass *V) {
   V->setParent(nullptr);
   if (V->hasName())
@@ -84,8 +84,8 @@ void SymbolTableListTraits<ValueSubClass>::removeNodeFromList(
       ST->removeValueName(V->getValueName());
 }
 
-template <typename ValueSubClass>
-void SymbolTableListTraits<ValueSubClass>::transferNodesFromList(
+template <typename ValueSubClass, typename... Args>
+void SymbolTableListTraits<ValueSubClass, Args...>::transferNodesFromList(
     SymbolTableListTraits &L2, iterator first, iterator last) {
   // Transfering nodes, even within the same BB, invalidates the ordering. The
   // list that we removed the nodes from still has a valid ordering.

diff  --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt
index 42634cef6d30160..12d7325036bf051 100644
--- a/llvm/unittests/ADT/CMakeLists.txt
+++ b/llvm/unittests/ADT/CMakeLists.txt
@@ -35,6 +35,7 @@ add_llvm_unittest(ADTTests
   HashingTest.cpp
   IListBaseTest.cpp
   IListIteratorTest.cpp
+  IListIteratorBitsTest.cpp
   IListNodeBaseTest.cpp
   IListNodeTest.cpp
   IListSentinelTest.cpp

diff  --git a/llvm/unittests/ADT/IListIteratorBitsTest.cpp b/llvm/unittests/ADT/IListIteratorBitsTest.cpp
new file mode 100644
index 000000000000000..167b30a5e308512
--- /dev/null
+++ b/llvm/unittests/ADT/IListIteratorBitsTest.cpp
@@ -0,0 +1,138 @@
+//==- unittests/ADT/IListIteratorBitsTest.cpp - ilist_iterator_w_bits tests -=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/simple_ilist.h"
+#include "gtest/gtest.h"
+
+// Test that ilist_iterator_w_bits can be used to store extra information about
+// what we're iterating over, that it's only enabled when given the relevant
+// option, and it can be fed into various iteration utilities.
+
+using namespace llvm;
+
+namespace {
+
+class dummy;
+
+struct Node : ilist_node<Node, ilist_iterator_bits<true>> {
+  friend class dummy;
+};
+
+struct PlainNode : ilist_node<PlainNode> {
+  friend class dummy;
+};
+
+TEST(IListIteratorBitsTest, DefaultConstructor) {
+  simple_ilist<Node, ilist_iterator_bits<true>>::iterator I;
+  simple_ilist<Node, ilist_iterator_bits<true>>::reverse_iterator RI;
+  simple_ilist<Node, ilist_iterator_bits<true>>::const_iterator CI;
+  simple_ilist<Node, ilist_iterator_bits<true>>::const_reverse_iterator CRI;
+  EXPECT_EQ(nullptr, I.getNodePtr());
+  EXPECT_EQ(nullptr, CI.getNodePtr());
+  EXPECT_EQ(nullptr, RI.getNodePtr());
+  EXPECT_EQ(nullptr, CRI.getNodePtr());
+  EXPECT_EQ(I, I);
+  EXPECT_EQ(I, CI);
+  EXPECT_EQ(CI, I);
+  EXPECT_EQ(CI, CI);
+  EXPECT_EQ(RI, RI);
+  EXPECT_EQ(RI, CRI);
+  EXPECT_EQ(CRI, RI);
+  EXPECT_EQ(CRI, CRI);
+  EXPECT_EQ(I, RI.getReverse());
+  EXPECT_EQ(RI, I.getReverse());
+}
+
+TEST(IListIteratorBitsTest, ConsAndAssignment) {
+  simple_ilist<Node, ilist_iterator_bits<true>> L;
+  Node A;
+  L.insert(L.end(), A);
+
+  simple_ilist<Node, ilist_iterator_bits<true>>::iterator I, I2;
+
+// Two sets of tests: if we've compiled in the iterator bits, then check that
+// HeadInclusiveBit and TailInclusiveBit are preserved on assignment and copy
+// construction, but not on other operations.
+#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS
+  I = L.begin();
+  EXPECT_FALSE(I.getHeadBit());
+  EXPECT_FALSE(I.getTailBit());
+  I.setHeadBit(true);
+  I.setTailBit(true);
+  EXPECT_TRUE(I.getHeadBit());
+  EXPECT_TRUE(I.getTailBit());
+
+  ++I;
+
+  EXPECT_FALSE(I.getHeadBit());
+  EXPECT_FALSE(I.getTailBit());
+
+  I = L.begin();
+  I.setHeadBit(true);
+  I.setTailBit(true);
+  I2 = I;
+  EXPECT_TRUE(I2.getHeadBit());
+  EXPECT_TRUE(I2.getTailBit());
+
+  I = L.begin();
+  I.setHeadBit(true);
+  I.setTailBit(true);
+  simple_ilist<Node, ilist_iterator_bits<true>>::iterator I3(I);
+  EXPECT_TRUE(I3.getHeadBit());
+  EXPECT_TRUE(I3.getTailBit());
+#else
+  // The calls should be available, but shouldn't actually store information.
+  I = L.begin();
+  EXPECT_FALSE(I.getHeadBit());
+  EXPECT_FALSE(I.getTailBit());
+  I.setHeadBit(true);
+  I.setTailBit(true);
+  EXPECT_FALSE(I.getHeadBit());
+  EXPECT_FALSE(I.getTailBit());
+  // Suppress warnings as we don't test with this variable.
+  (void)I2;
+#endif
+}
+
+class dummy {
+  // Test that we get an ilist_iterator_w_bits out of the node given that the
+  // options are enabled.
+  using node_options = typename ilist_detail::compute_node_options<
+      Node, ilist_iterator_bits<true>>::type;
+  static_assert(std::is_same<Node::self_iterator,
+                             llvm::ilist_iterator_w_bits<node_options, false,
+                                                         false>>::value);
+
+  // Now test that a plain node, without the option, gets a plain
+  // ilist_iterator.
+  using plain_node_options =
+      typename ilist_detail::compute_node_options<PlainNode>::type;
+  static_assert(std::is_same<
+                PlainNode::self_iterator,
+                llvm::ilist_iterator<plain_node_options, false, false>>::value);
+};
+
+TEST(IListIteratorBitsTest, RangeIteration) {
+  // Check that we can feed ilist_iterator_w_bits into make_range and similar.
+  // Plus, we should be able to convert it to a reverse iterator and use that.
+  simple_ilist<Node, ilist_iterator_bits<true>> L;
+  Node A;
+  L.insert(L.end(), A);
+
+  for (Node &N : make_range(L.begin(), L.end()))
+    (void)N;
+
+  simple_ilist<Node, ilist_iterator_bits<true>>::iterator It =
+      L.begin()->getIterator();
+  auto RevIt = It.getReverse();
+
+  for (Node &N : make_range(RevIt, L.rend()))
+    (void)N;
+}
+
+} // end namespace


        


More information about the llvm-commits mailing list