From 088d272e83259a5d8e577a3d2e62012c42a9f9db Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Tue, 17 Oct 2023 15:08:58 +0100 Subject: [PATCH] [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 --- llvm/CMakeLists.txt | 3 + llvm/cmake/modules/HandleLLVMOptions.cmake | 4 + llvm/include/llvm/ADT/ilist_iterator.h | 191 +++++++++++++++++++ llvm/include/llvm/ADT/ilist_node.h | 35 +++- llvm/include/llvm/ADT/ilist_node_options.h | 29 ++- llvm/include/llvm/ADT/simple_ilist.h | 16 +- llvm/include/llvm/IR/BasicBlock.h | 28 ++- llvm/include/llvm/IR/GlobalAlias.h | 2 +- llvm/include/llvm/IR/GlobalIFunc.h | 2 +- llvm/include/llvm/IR/GlobalVariable.h | 2 +- llvm/include/llvm/IR/Instruction.h | 23 ++- llvm/include/llvm/IR/Instructions.h | 2 +- llvm/include/llvm/IR/SymbolTableListTraits.h | 16 +- llvm/include/llvm/IR/ValueSymbolTable.h | 5 +- llvm/lib/IR/BasicBlock.cpp | 15 +- llvm/lib/IR/Instruction.cpp | 5 +- llvm/lib/IR/Instructions.cpp | 2 +- llvm/lib/IR/SymbolTableListTraitsImpl.h | 20 +- llvm/unittests/ADT/CMakeLists.txt | 1 + llvm/unittests/ADT/IListIteratorBitsTest.cpp | 138 ++++++++++++++ 20 files changed, 483 insertions(+), 56 deletions(-) create mode 100644 llvm/unittests/ADT/IListIteratorBitsTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 103c08ffbe83..ef2f2146a036 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 19cb881adc3f..56b63cb5acb8 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 be876347907b..9047b9b73959 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 { 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 ilist_iterator_w_bits : ilist_detail::SpecificNodeAccess { + friend ilist_iterator_w_bits; + friend ilist_iterator_w_bits; + friend ilist_iterator; + + using Traits = ilist_detail::IteratorTraits; + using Access = ilist_detail::SpecificNodeAccess; + +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 + ilist_iterator_w_bits( + const ilist_iterator_w_bits &RHS, + std::enable_if_t = 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 + std::enable_if_t + operator=(const ilist_iterator_w_bits &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 &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 getReverse() const { + if (NodePtr) + return ilist_iterator_w_bits(*NodePtr); + return ilist_iterator_w_bits(); + } + + /// Const-cast. + ilist_iterator_w_bits getNonConst() const { + if (NodePtr) { + auto New = ilist_iterator_w_bits( + const_cast::node_reference>( + *NodePtr)); +#ifdef EXPERIMENTAL_DEBUGINFO_ITERATORS + New.HeadInclusiveBit = HeadInclusiveBit; + New.TailInclusiveBit = TailInclusiveBit; +#endif + return New; + } + return ilist_iterator_w_bits(); + } + + // 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(NodePtr); } + + /// Check for end. Only valid if ilist_sentinel_tracking. + 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 struct simplify_type; /// Allow ilist_iterators to convert into pointers to a node automatically when @@ -192,6 +371,18 @@ template struct simplify_type> : simplify_type> {}; +// ilist_iterator_w_bits should also be accessible via isa/dyn_cast. +template +struct simplify_type> { + using iterator = ilist_iterator_w_bits; + using SimpleType = typename iterator::pointer; + + static SimpleType getSimplifiedValue(const iterator &Node) { return &*Node; } +}; +template +struct simplify_type> + : simplify_type> {}; + } // 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 7856b1c0d410..3b6f0dcc7b5e 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 ilist_iterator; +template +class ilist_iterator_w_bits; template class ilist_sentinel; +// Selector for which iterator type to pick given the iterator-bits node option. +template +class ilist_select_iterator_type { +public: + using type = ilist_iterator; +}; +template +class ilist_select_iterator_type { +public: + using type = ilist_iterator_w_bits; +}; + /// Implementation for an ilist node. /// /// Templated on an appropriate \a ilist_detail::node_options, usually computed @@ -45,16 +59,29 @@ template class ilist_node_impl : OptionsT::node_base_type { friend typename OptionsT::list_base_type; friend struct ilist_detail::NodeAccess; friend class ilist_sentinel; + friend class ilist_iterator; friend class ilist_iterator; friend class ilist_iterator; friend class ilist_iterator; + friend class ilist_iterator_w_bits; + friend class ilist_iterator_w_bits; + friend class ilist_iterator_w_bits; + friend class ilist_iterator_w_bits; protected: - using self_iterator = ilist_iterator; - using const_self_iterator = ilist_iterator; - using reverse_self_iterator = ilist_iterator; - using const_reverse_self_iterator = ilist_iterator; + using self_iterator = + typename ilist_select_iterator_type::type; + using const_self_iterator = + typename ilist_select_iterator_type::type; + using reverse_self_iterator = + typename ilist_select_iterator_type::type; + using const_reverse_self_iterator = + typename ilist_select_iterator_type::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 05340d344e39..e6e1068953e3 100644 --- a/llvm/include/llvm/ADT/ilist_node_options.h +++ b/llvm/include/llvm/ADT/ilist_node_options.h @@ -31,6 +31,14 @@ template struct ilist_sentinel_tracking {}; /// simultaneously. See \a ilist_node for usage examples. template 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 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 struct is_valid_option> : std::true_type {}; +/// Extract iterator bits option. +/// +/// Look through \p Options for the \a ilist_iterator_bits option. Defaults +/// to false. +template struct extract_iterator_bits; +template +struct extract_iterator_bits, Options...> + : std::integral_constant {}; +template +struct extract_iterator_bits + : extract_iterator_bits {}; +template <> struct extract_iterator_bits<> : std::false_type, is_implicit {}; +template +struct is_valid_option> : 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 /// /// This is usually computed via \a compute_node_options. template + 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 node_base_type; typedef ilist_base list_base_type; @@ -123,7 +147,8 @@ struct node_options { template struct compute_node_options { typedef node_options::value, extract_sentinel_tracking::is_explicit, - typename extract_tag::type> + typename extract_tag::type, + extract_iterator_bits::value> type; }; diff --git a/llvm/include/llvm/ADT/simple_ilist.h b/llvm/include/llvm/ADT/simple_ilist.h index 3a96e1ba5657..7236b3fa5a7d 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; - using const_iterator = ilist_iterator; - using reverse_iterator = ilist_iterator; - using const_reverse_iterator = ilist_iterator; + using iterator = + typename ilist_select_iterator_type::type; + using const_iterator = + typename ilist_select_iterator_type::type; + using reverse_iterator = + typename ilist_select_iterator_type::type; + using const_reverse_iterator = + typename ilist_select_iterator_type::type; using size_type = size_t; using difference_type = ptrdiff_t; diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h index b031f72493e1..ab291c24e5b6 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 { public: - using InstListType = SymbolTableList; + using InstListType = SymbolTableList>; 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::iterator Instruction::eraseFromParent(); + friend BasicBlock::iterator Instruction::eraseFromParent(); friend BasicBlock::iterator Instruction::insertInto(BasicBlock *BB, BasicBlock::iterator It); - friend class llvm::SymbolTableListTraits; - friend class llvm::ilist_node_with_parent; + friend class llvm::SymbolTableListTraits>; + friend class llvm::ilist_node_with_parent>; /// 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(this)->getFirstNonPHIIt().getNonConst(); + static_cast(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 de405da5ca23..583d66e28155 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 class SymbolTableListTraits; +template class SymbolTableListTraits; class GlobalAlias : public GlobalValue, public ilist_node { friend class SymbolTableListTraits; diff --git a/llvm/include/llvm/IR/GlobalIFunc.h b/llvm/include/llvm/IR/GlobalIFunc.h index c148ee790778..4d1982da0baf 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 class SymbolTableListTraits; +template class SymbolTableListTraits; class GlobalIFunc final : public GlobalObject, public ilist_node { friend class SymbolTableListTraits; diff --git a/llvm/include/llvm/IR/GlobalVariable.h b/llvm/include/llvm/IR/GlobalVariable.h index 03c680e4f955..f915dba5c659 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 class SymbolTableListTraits; +template class SymbolTableListTraits; class DIGlobalVariableExpression; class GlobalVariable : public GlobalObject, public ilist_node { diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h index 69c3af5b7610..af7aa791cb6d 100644 --- a/llvm/include/llvm/IR/Instruction.h +++ b/llvm/include/llvm/IR/Instruction.h @@ -39,7 +39,11 @@ template <> struct ilist_alloc_traits { }; class Instruction : public User, - public ilist_node_with_parent { + public ilist_node_with_parent> { +public: + using InstListType = SymbolTableList>; +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::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::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::iterator - insertInto(BasicBlock *ParentBB, SymbolTableList::iterator It); + InstListType::iterator insertInto(BasicBlock *ParentBB, + InstListType::iterator It); - void insertBefore(BasicBlock &BB, - SymbolTableList::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::iterator I); + void moveBefore(BasicBlock &BB, InstListType::iterator I); /// (See other overload for moveBeforePreserving). - void moveBeforePreserving(BasicBlock &BB, SymbolTableList::iterator I) { + void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I) { moveBefore(BB, I); } @@ -902,7 +905,7 @@ class Instruction : public User, }; private: - friend class SymbolTableListTraits; + friend class SymbolTableListTraits>; 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 c85727ce30a9..af6ac566a019 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::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 8af712374bfa..bd31fca5e525 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 class SymbolTableList; +template 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 +template class SymbolTableListTraits : public ilist_alloc_traits { - using ListTy = SymbolTableList; - using iterator = typename simple_ilist::iterator; + using ListTy = SymbolTableList; + using iterator = typename simple_ilist::iterator; using ItemParentClass = typename SymbolTableListParentType::type; @@ -110,9 +111,10 @@ class SymbolTableListTraits : public ilist_alloc_traits { /// 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 SymbolTableList - : public iplist_impl, SymbolTableListTraits> {}; +template +class SymbolTableList : public iplist_impl, + SymbolTableListTraits> { +}; } // end namespace llvm diff --git a/llvm/include/llvm/IR/ValueSymbolTable.h b/llvm/include/llvm/IR/ValueSymbolTable.h index 43d00268f4b2..6350f6a2435e 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 struct ilist_iterator_bits; template class SmallString; -template class SymbolTableListTraits; +template class SymbolTableListTraits; /// This class provides a symbol table of name/value pairs. It is essentially /// a std::map but has a controlled interface provided by @@ -41,7 +42,7 @@ class ValueSymbolTable { friend class SymbolTableListTraits; friend class SymbolTableListTraits; friend class SymbolTableListTraits; - friend class SymbolTableListTraits; + friend class SymbolTableListTraits>; friend class Value; /// @name Types diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp index d6677aa721bb..46b1a3b37132 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; +template class llvm::SymbolTableListTraits>; 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 b497951a598c..9b176eb78888 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::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::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 ece3b58792dd..2ea9c05de6be 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::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 4283744bd058..990552f9b65a 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 +template template -void SymbolTableListTraits::setSymTabObject(TPtr *Dest, - TPtr Src) { +void SymbolTableListTraits::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::setSymTabObject(TPtr *Dest, if (I->hasName()) NewST->reinsertValue(&*I); } - } -template -void SymbolTableListTraits::addNodeToList(ValueSubClass *V) { +template +void SymbolTableListTraits::addNodeToList( + ValueSubClass *V) { assert(!V->getParent() && "Value already in a container!!"); ItemParentClass *Owner = getListOwner(); V->setParent(Owner); @@ -75,8 +75,8 @@ void SymbolTableListTraits::addNodeToList(ValueSubClass *V) { ST->reinsertValue(V); } -template -void SymbolTableListTraits::removeNodeFromList( +template +void SymbolTableListTraits::removeNodeFromList( ValueSubClass *V) { V->setParent(nullptr); if (V->hasName()) @@ -84,8 +84,8 @@ void SymbolTableListTraits::removeNodeFromList( ST->removeValueName(V->getValueName()); } -template -void SymbolTableListTraits::transferNodesFromList( +template +void SymbolTableListTraits::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 42634cef6d30..12d7325036bf 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 000000000000..167b30a5e308 --- /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> { + friend class dummy; +}; + +struct PlainNode : ilist_node { + friend class dummy; +}; + +TEST(IListIteratorBitsTest, DefaultConstructor) { + simple_ilist>::iterator I; + simple_ilist>::reverse_iterator RI; + simple_ilist>::const_iterator CI; + simple_ilist>::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> L; + Node A; + L.insert(L.end(), A); + + simple_ilist>::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>::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>::type; + static_assert(std::is_same>::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::type; + static_assert(std::is_same< + PlainNode::self_iterator, + llvm::ilist_iterator>::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> L; + Node A; + L.insert(L.end(), A); + + for (Node &N : make_range(L.begin(), L.end())) + (void)N; + + simple_ilist>::iterator It = + L.begin()->getIterator(); + auto RevIt = It.getReverse(); + + for (Node &N : make_range(RevIt, L.rend())) + (void)N; +} + +} // end namespace