[llvm] [LLVM] Add option to store Parent-pointer in ilist_node_base (PR #94224)
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 4 05:24:40 PDT 2024
https://github.com/SLTozer updated https://github.com/llvm/llvm-project/pull/94224
>From 537c8f80bfed595ca3f061a2d53b5a8f5ce7802e Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Fri, 31 May 2024 18:14:06 +0100
Subject: [PATCH 1/3] Add option to store Parent-pointer in ilist_node_base
---
llvm/include/llvm/ADT/ilist_base.h | 4 +-
llvm/include/llvm/ADT/ilist_iterator.h | 10 +++++
llvm/include/llvm/ADT/ilist_node.h | 11 +++++
llvm/include/llvm/ADT/ilist_node_base.h | 51 ++++++++++++++++++++--
llvm/include/llvm/ADT/ilist_node_options.h | 43 +++++++++++++++---
llvm/include/llvm/IR/BasicBlock.h | 10 +++--
llvm/include/llvm/IR/Instruction.h | 15 ++++---
llvm/include/llvm/IR/ValueSymbolTable.h | 4 +-
llvm/lib/IR/BasicBlock.cpp | 5 ++-
llvm/lib/IR/Instruction.cpp | 25 +++++------
10 files changed, 140 insertions(+), 38 deletions(-)
diff --git a/llvm/include/llvm/ADT/ilist_base.h b/llvm/include/llvm/ADT/ilist_base.h
index b8c098b951ade..000253a26012b 100644
--- a/llvm/include/llvm/ADT/ilist_base.h
+++ b/llvm/include/llvm/ADT/ilist_base.h
@@ -15,9 +15,9 @@
namespace llvm {
/// Implementations of list algorithms using ilist_node_base.
-template <bool EnableSentinelTracking> class ilist_base {
+template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_base {
public:
- using node_base_type = ilist_node_base<EnableSentinelTracking>;
+ using node_base_type = ilist_node_base<EnableSentinelTracking, ParentPtrTy>;
static void insertBeforeImpl(node_base_type &Next, node_base_type &N) {
node_base_type &Prev = *Next.getPrev();
diff --git a/llvm/include/llvm/ADT/ilist_iterator.h b/llvm/include/llvm/ADT/ilist_iterator.h
index 2393c4d2c403c..635e050e0d09a 100644
--- a/llvm/include/llvm/ADT/ilist_iterator.h
+++ b/llvm/include/llvm/ADT/ilist_iterator.h
@@ -70,6 +70,7 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT> {
using iterator_category = std::bidirectional_iterator_tag;
using const_pointer = typename OptionsT::const_pointer;
using const_reference = typename OptionsT::const_reference;
+ using parent_ptr_ty = typename OptionsT::parent_ptr_ty;
private:
using node_pointer = typename Traits::node_pointer;
@@ -101,6 +102,8 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT> {
return *this;
}
+ parent_ptr_ty getNodeParent() { return NodePtr->getNodeBaseParent(); }
+
/// Explicit conversion between forward/reverse iterators.
///
/// Translate between forward and reverse iterators without changing range
@@ -168,6 +171,8 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT> {
return tmp;
}
+ bool isValid() const { return NodePtr; }
+
/// Get the underlying ilist_node.
node_pointer getNodePtr() const { return static_cast<node_pointer>(NodePtr); }
@@ -195,6 +200,7 @@ class ilist_iterator_w_bits : ilist_detail::SpecificNodeAccess<OptionsT> {
using iterator_category = std::bidirectional_iterator_tag;
using const_pointer = typename OptionsT::const_pointer;
using const_reference = typename OptionsT::const_reference;
+ using parent_ptr_ty = typename OptionsT::parent_ptr_ty;
private:
using node_pointer = typename Traits::node_pointer;
@@ -319,6 +325,10 @@ class ilist_iterator_w_bits : ilist_detail::SpecificNodeAccess<OptionsT> {
return tmp;
}
+ parent_ptr_ty getNodeParent() { return NodePtr->getNodeBaseParent(); }
+
+ bool isValid() const { return NodePtr; }
+
/// Get the underlying ilist_node.
node_pointer getNodePtr() const { return static_cast<node_pointer>(NodePtr); }
diff --git a/llvm/include/llvm/ADT/ilist_node.h b/llvm/include/llvm/ADT/ilist_node.h
index 3b6f0dcc7b5e9..ec615497abee1 100644
--- a/llvm/include/llvm/ADT/ilist_node.h
+++ b/llvm/include/llvm/ADT/ilist_node.h
@@ -118,7 +118,9 @@ template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
}
// Under-approximation, but always available for assertions.
+ using node_base_type::getNodeBaseParent;
using node_base_type::isKnownSentinel;
+ using node_base_type::setNodeBaseParent;
/// Check whether this is the sentinel node.
///
@@ -171,6 +173,15 @@ template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
/// }
/// \endexample
///
+/// When the \a ilist_parent<ParentTy> option is passed to an ilist_node and the
+/// owning ilist, each node contains a pointer to the ilist's owner. This
+/// pointer does not have any automatic behaviour; set it manually, including
+/// for the sentinel node when the list is created. The primary benefit of this
+/// over declaring and using this pointer in the final node class is that the
+/// pointer will be added in the sentinel, meaning that you can safely use \a
+/// ilist_iterator::getNodeParent() to get the node parent from any valid (i.e.
+/// non-null) iterator, even a sentinel value.
+///
/// See \a is_valid_option for steps on adding a new option.
template <class T, class... Options>
class ilist_node
diff --git a/llvm/include/llvm/ADT/ilist_node_base.h b/llvm/include/llvm/ADT/ilist_node_base.h
index f6c518e6eed7a..e396bb22fca02 100644
--- a/llvm/include/llvm/ADT/ilist_node_base.h
+++ b/llvm/include/llvm/ADT/ilist_node_base.h
@@ -16,9 +16,9 @@ namespace llvm {
/// Base class for ilist nodes.
///
/// Optionally tracks whether this node is the sentinel.
-template <bool EnableSentinelTracking> class ilist_node_base;
+template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_node_base;
-template <> class ilist_node_base<false> {
+template <> class ilist_node_base<false, void> {
ilist_node_base *Prev = nullptr;
ilist_node_base *Next = nullptr;
@@ -28,11 +28,14 @@ template <> class ilist_node_base<false> {
ilist_node_base *getPrev() const { return Prev; }
ilist_node_base *getNext() const { return Next; }
+ void setNodeBaseParent(void) {}
+ inline void getNodeBaseParent() const {}
+
bool isKnownSentinel() const { return false; }
void initializeSentinel() {}
};
-template <> class ilist_node_base<true> {
+template <> class ilist_node_base<true, void> {
PointerIntPair<ilist_node_base *, 1> PrevAndSentinel;
ilist_node_base *Next = nullptr;
@@ -42,6 +45,48 @@ template <> class ilist_node_base<true> {
ilist_node_base *getPrev() const { return PrevAndSentinel.getPointer(); }
ilist_node_base *getNext() const { return Next; }
+ void setNodeBaseParent(void) {}
+ inline void getNodeBaseParent() const {}
+
+ bool isSentinel() const { return PrevAndSentinel.getInt(); }
+ bool isKnownSentinel() const { return isSentinel(); }
+ void initializeSentinel() { PrevAndSentinel.setInt(true); }
+};
+
+template <class ParentPtrTy> class ilist_node_base<false, ParentPtrTy> {
+ ilist_node_base *Prev = nullptr;
+ ilist_node_base *Next = nullptr;
+ ParentPtrTy Parent = nullptr;
+
+public:
+ void setPrev(ilist_node_base *Prev) { this->Prev = Prev; }
+ void setNext(ilist_node_base *Next) { this->Next = Next; }
+ ilist_node_base *getPrev() const { return Prev; }
+ ilist_node_base *getNext() const { return Next; }
+
+ void setNodeBaseParent(ParentPtrTy Parent) { this->Parent = Parent; }
+ inline const ParentPtrTy getNodeBaseParent() const { return Parent; }
+ inline ParentPtrTy getNodeBaseParent() { return Parent; }
+
+ bool isKnownSentinel() const { return false; }
+ void initializeSentinel() {}
+};
+
+template <class ParentPtrTy> class ilist_node_base<true, ParentPtrTy> {
+ PointerIntPair<ilist_node_base *, 1> PrevAndSentinel;
+ ilist_node_base *Next = nullptr;
+ ParentPtrTy Parent = nullptr;
+
+public:
+ void setPrev(ilist_node_base *Prev) { PrevAndSentinel.setPointer(Prev); }
+ void setNext(ilist_node_base *Next) { this->Next = Next; }
+ ilist_node_base *getPrev() const { return PrevAndSentinel.getPointer(); }
+ ilist_node_base *getNext() const { return Next; }
+
+ void setNodeBaseParent(ParentPtrTy Parent) { this->Parent = Parent; }
+ inline const ParentPtrTy getNodeBaseParent() const { return Parent; }
+ inline ParentPtrTy getNodeBaseParent() { return Parent; }
+
bool isSentinel() const { return PrevAndSentinel.getInt(); }
bool isKnownSentinel() const { return isSentinel(); }
void initializeSentinel() { PrevAndSentinel.setInt(true); }
diff --git a/llvm/include/llvm/ADT/ilist_node_options.h b/llvm/include/llvm/ADT/ilist_node_options.h
index e6e1068953e36..aa32162cd51e4 100644
--- a/llvm/include/llvm/ADT/ilist_node_options.h
+++ b/llvm/include/llvm/ADT/ilist_node_options.h
@@ -15,8 +15,8 @@
namespace llvm {
-template <bool EnableSentinelTracking> class ilist_node_base;
-template <bool EnableSentinelTracking> class ilist_base;
+template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_node_base;
+template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_base;
/// Option to choose whether to track sentinels.
///
@@ -39,6 +39,19 @@ template <class Tag> struct ilist_tag {};
/// iterator class to store that information.
template <bool ExtraIteratorBits> struct ilist_iterator_bits {};
+/// Option to add a pointer to this list's owner in every node.
+///
+/// This option causes the \a ilist_base_node for this list to contain a pointer
+/// ParentTy *Parent, returned by \a ilist_base_node::getNodeBaseParent() and
+/// set by \a ilist_base_node::setNodeBaseParent(ParentTy *Parent). The parent
+/// value is not set automatically; the ilist owner should set itself as the
+/// parent of the list sentinel, and the parent should be set on each node
+/// inserted into the list. This value is also not used by
+/// \a ilist_node_with_parent::getNodeParent(), but is used by \a
+/// ilist_iterator::getNodeParent(), which allows the parent to be fetched from
+/// any valid (non-null) iterator to this list, including the sentinel.
+template <class ParentTy> struct ilist_parent {};
+
namespace ilist_detail {
/// Helper trait for recording whether an option is specified explicitly.
@@ -114,6 +127,21 @@ template <> struct extract_iterator_bits<> : std::false_type, is_implicit {};
template <bool IteratorBits>
struct is_valid_option<ilist_iterator_bits<IteratorBits>> : std::true_type {};
+/// Extract node parent option.
+///
+/// Look through \p Options for the \a ilist_parent option, pulling out the
+/// custom parent type, using void as a default.
+template <class... Options> struct extract_parent;
+template <class ParentTy, class... Options>
+struct extract_parent<ilist_parent<ParentTy>, Options...> {
+ typedef ParentTy *type;
+};
+template <class Option1, class... Options>
+struct extract_parent<Option1, Options...> : extract_parent<Options...> {};
+template <> struct extract_parent<> { typedef void type; };
+template <class ParentTy>
+struct is_valid_option<ilist_parent<ParentTy>> : std::true_type {};
+
/// Check whether options are valid.
///
/// The conjunction of \a is_valid_option on each individual option.
@@ -128,7 +156,7 @@ struct check_options<Option1, Options...>
///
/// This is usually computed via \a compute_node_options.
template <class T, bool EnableSentinelTracking, bool IsSentinelTrackingExplicit,
- class TagT, bool HasIteratorBits>
+ class TagT, bool HasIteratorBits, class ParentPtrTy>
struct node_options {
typedef T value_type;
typedef T *pointer;
@@ -140,15 +168,18 @@ struct node_options {
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;
+ typedef ParentPtrTy parent_ptr_ty;
+ typedef ilist_node_base<enable_sentinel_tracking, parent_ptr_ty>
+ node_base_type;
+ typedef ilist_base<enable_sentinel_tracking, parent_ptr_ty> list_base_type;
};
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,
- extract_iterator_bits<Options...>::value>
+ extract_iterator_bits<Options...>::value,
+ typename extract_parent<Options...>::type>
type;
};
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index e1220966e7e6e..80067f2652a2b 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -59,7 +59,8 @@ class DbgMarker;
class BasicBlock final : public Value, // Basic blocks are data objects also
public ilist_node_with_parent<BasicBlock, Function> {
public:
- using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>>;
+ using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>,
+ ilist_parent<BasicBlock>>;
/// Flag recording whether or not this block stores debug-info in the form
/// of intrinsic instructions (false) or non-instruction records (true).
bool IsNewDbgInfoFormat;
@@ -172,10 +173,11 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
friend BasicBlock::iterator Instruction::eraseFromParent();
friend BasicBlock::iterator Instruction::insertInto(BasicBlock *BB,
BasicBlock::iterator It);
- friend class llvm::SymbolTableListTraits<llvm::Instruction,
- ilist_iterator_bits<true>>;
+ friend class llvm::SymbolTableListTraits<
+ llvm::Instruction, ilist_iterator_bits<true>, ilist_parent<BasicBlock>>;
friend class llvm::ilist_node_with_parent<llvm::Instruction, llvm::BasicBlock,
- ilist_iterator_bits<true>>;
+ ilist_iterator_bits<true>,
+ ilist_parent<BasicBlock>>;
// Friendly methods that need to access us for the maintenence of
// debug-info attachments.
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 249be2799fa93..e1cb362ad20d0 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -46,11 +46,13 @@ getDbgRecordRange(DbgMarker *);
class Instruction : public User,
public ilist_node_with_parent<Instruction, BasicBlock,
- ilist_iterator_bits<true>> {
+ ilist_iterator_bits<true>,
+ ilist_parent<BasicBlock>> {
public:
- using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>>;
+ using InstListType = SymbolTableList<Instruction, ilist_iterator_bits<true>,
+ ilist_parent<BasicBlock>>;
+
private:
- BasicBlock *Parent;
DebugLoc DbgLoc; // 'dbg' Metadata cache.
/// Relative order of this instruction in its parent basic block. Used for
@@ -149,8 +151,8 @@ class Instruction : public User,
Instruction *user_back() { return cast<Instruction>(*user_begin());}
const Instruction *user_back() const { return cast<Instruction>(*user_begin());}
- inline const BasicBlock *getParent() const { return Parent; }
- inline BasicBlock *getParent() { return Parent; }
+ inline const BasicBlock *getParent() const { return getNodeBaseParent(); }
+ inline BasicBlock *getParent() { return getNodeBaseParent(); }
/// Return the module owning the function this instruction belongs to
/// or nullptr it the function does not have a module.
@@ -980,7 +982,8 @@ class Instruction : public User,
};
private:
- friend class SymbolTableListTraits<Instruction, ilist_iterator_bits<true>>;
+ friend class SymbolTableListTraits<Instruction, ilist_iterator_bits<true>,
+ ilist_parent<BasicBlock>>;
friend class BasicBlock; // For renumbering.
// Shadow Value::setValueSubclassData with a private forwarding method so that
diff --git a/llvm/include/llvm/IR/ValueSymbolTable.h b/llvm/include/llvm/IR/ValueSymbolTable.h
index 6350f6a2435e4..cd1dbbe1688a1 100644
--- a/llvm/include/llvm/IR/ValueSymbolTable.h
+++ b/llvm/include/llvm/IR/ValueSymbolTable.h
@@ -28,6 +28,7 @@ class GlobalIFunc;
class GlobalVariable;
class Instruction;
template <bool ExtraIteratorBits> struct ilist_iterator_bits;
+template <class ParentTy> struct ilist_parent;
template <unsigned InternalLen> class SmallString;
template <typename ValueSubClass, typename ... Args> class SymbolTableListTraits;
@@ -42,7 +43,8 @@ class ValueSymbolTable {
friend class SymbolTableListTraits<GlobalAlias>;
friend class SymbolTableListTraits<GlobalIFunc>;
friend class SymbolTableListTraits<GlobalVariable>;
- friend class SymbolTableListTraits<Instruction, ilist_iterator_bits<true>>;
+ friend class SymbolTableListTraits<Instruction, ilist_iterator_bits<true>,
+ ilist_parent<BasicBlock>>;
friend class Value;
/// @name Types
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 29f2cbf611fa3..a5199f36c1cb5 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -175,8 +175,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,
- ilist_iterator_bits<true>>;
+template class llvm::SymbolTableListTraits<
+ Instruction, ilist_iterator_bits<true>, ilist_parent<BasicBlock>>;
BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent,
BasicBlock *InsertBefore)
@@ -189,6 +189,7 @@ BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent,
assert(!InsertBefore &&
"Cannot insert block before another block with no function!");
+ end().getNodePtr()->setNodeBaseParent(this);
setName(Name);
if (NewParent)
setIsNewDbgInfoFormat(NewParent->IsNewDbgInfoFormat);
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 29272e627a1d1..8e8131113a230 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -27,8 +27,7 @@ using namespace llvm;
Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
InstListType::iterator InsertBefore)
- : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(nullptr) {
-
+ : User(ty, Value::InstructionVal + it, Ops, NumOps) {
// When called with an iterator, there must be a block to insert into.
BasicBlock *BB = InsertBefore->getParent();
assert(BB && "Instruction to insert before is not in a basic block!");
@@ -37,7 +36,7 @@ Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
Instruction *InsertBefore)
- : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(nullptr) {
+ : User(ty, Value::InstructionVal + it, Ops, NumOps) {
// If requested, insert this instruction into a basic block...
if (InsertBefore) {
@@ -49,15 +48,14 @@ Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
BasicBlock *InsertAtEnd)
- : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(nullptr) {
-
+ : User(ty, Value::InstructionVal + it, Ops, NumOps) {
// If requested, append this instruction into the basic block.
if (InsertAtEnd)
insertInto(InsertAtEnd, InsertAtEnd->end());
}
Instruction::~Instruction() {
- assert(!Parent && "Instruction still linked in the program!");
+ assert(!getParent() && "Instruction still linked in the program!");
// Replace any extant metadata uses of this instruction with undef to
// preserve debug info accuracy. Some alternatives include:
@@ -76,9 +74,7 @@ Instruction::~Instruction() {
setMetadata(LLVMContext::MD_DIAssignID, nullptr);
}
-void Instruction::setParent(BasicBlock *P) {
- Parent = P;
-}
+void Instruction::setParent(BasicBlock *P) { setNodeBaseParent(P); }
const Module *Instruction::getModule() const {
return getParent()->getModule();
@@ -96,7 +92,7 @@ void Instruction::removeFromParent() {
}
void Instruction::handleMarkerRemoval() {
- if (!Parent->IsNewDbgInfoFormat || !DebugMarker)
+ if (!getParent()->IsNewDbgInfoFormat || !DebugMarker)
return;
DebugMarker->removeMarker();
@@ -329,11 +325,12 @@ void Instruction::dropOneDbgRecord(DbgRecord *DVR) {
}
bool Instruction::comesBefore(const Instruction *Other) const {
- assert(Parent && Other->Parent &&
+ assert(getParent() && Other->getParent() &&
"instructions without BB parents have no order");
- assert(Parent == Other->Parent && "cross-BB instruction order comparison");
- if (!Parent->isInstrOrderValid())
- Parent->renumberInstructions();
+ assert(getParent() == Other->getParent() &&
+ "cross-BB instruction order comparison");
+ if (!getParent()->isInstrOrderValid())
+ const_cast<BasicBlock *>(getParent())->renumberInstructions();
return Order < Other->Order;
}
>From d89ff98bd1be09c4eecce2c330e168bf2007f814 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Mon, 3 Jun 2024 18:24:14 +0100
Subject: [PATCH 2/3] Add mixins to detail namespaces, add comments
---
llvm/include/llvm/ADT/ilist_iterator.h | 31 +++++++++-
llvm/include/llvm/ADT/ilist_node.h | 44 ++++++++++---
llvm/include/llvm/ADT/ilist_node_base.h | 82 +++++++++----------------
llvm/include/llvm/IR/Instruction.h | 5 --
llvm/lib/IR/BasicBlock.cpp | 2 +-
llvm/lib/IR/Instruction.cpp | 2 -
6 files changed, 93 insertions(+), 73 deletions(-)
diff --git a/llvm/include/llvm/ADT/ilist_iterator.h b/llvm/include/llvm/ADT/ilist_iterator.h
index 635e050e0d09a..5f27dd2f3f447 100644
--- a/llvm/include/llvm/ADT/ilist_iterator.h
+++ b/llvm/include/llvm/ADT/ilist_iterator.h
@@ -50,11 +50,38 @@ template <> struct IteratorHelper<true> : ilist_detail::NodeAccess {
template <class T> static void decrement(T *&I) { I = Access::getNext(*I); }
};
+/// Mixin class used to add a \a getNodeParent() function to iterators iff the
+/// list uses \a ilist_parent, calling through to the node's \a getParent(). For
+/// more details see \a ilist_node.
+template <class IteratorTy, class ParentPtrTy, bool IsConst>
+class iterator_parent_access;
+template <class IteratorTy, class ParentPtrTy>
+class iterator_parent_access<IteratorTy, ParentPtrTy, true> {
+public:
+ inline const ParentPtrTy getNodeParent() const {
+ return static_cast<IteratorTy *>(this)->NodePtr->getParent();
+ }
+};
+template <class IteratorTy, class ParentPtrTy>
+class iterator_parent_access<IteratorTy, ParentPtrTy, false> {
+public:
+ inline ParentPtrTy getNodeParent() {
+ return static_cast<IteratorTy *>(this)->NodePtr->getParent();
+ }
+};
+template <class IteratorTy>
+class iterator_parent_access<IteratorTy, void, true> {};
+template <class IteratorTy>
+class iterator_parent_access<IteratorTy, void, false> {};
+
} // end namespace ilist_detail
/// Iterator for intrusive lists based on ilist_node.
template <class OptionsT, bool IsReverse, bool IsConst>
-class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT> {
+class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT>,
+ public ilist_detail::iterator_parent_access<
+ ilist_iterator<OptionsT, IsReverse, IsConst>,
+ typename OptionsT::parent_ptr_ty, IsConst> {
friend ilist_iterator<OptionsT, IsReverse, !IsConst>;
friend ilist_iterator<OptionsT, !IsReverse, IsConst>;
friend ilist_iterator<OptionsT, !IsReverse, !IsConst>;
@@ -102,8 +129,6 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT> {
return *this;
}
- parent_ptr_ty getNodeParent() { return NodePtr->getNodeBaseParent(); }
-
/// Explicit conversion between forward/reverse iterators.
///
/// Translate between forward and reverse iterators without changing range
diff --git a/llvm/include/llvm/ADT/ilist_node.h b/llvm/include/llvm/ADT/ilist_node.h
index ec615497abee1..209fd209167e8 100644
--- a/llvm/include/llvm/ADT/ilist_node.h
+++ b/llvm/include/llvm/ADT/ilist_node.h
@@ -24,6 +24,23 @@ namespace ilist_detail {
struct NodeAccess;
+/// Mixin base class that is used to add \a getParent() and
+/// \a setParent(ParentPtrTy) methods to \a ilist_node_impl iff \a ilist_parent
+/// has been set in the list options.
+template <class NodeTy, class ParentPtrTy> class node_parent_access {
+public:
+ inline const ParentPtrTy getParent() const {
+ return static_cast<const NodeTy *>(this)->getNodeBaseParent();
+ }
+ inline ParentPtrTy getParent() {
+ return static_cast<NodeTy *>(this)->getNodeBaseParent();
+ }
+ void setParent(ParentPtrTy Parent) {
+ return static_cast<NodeTy *>(this)->setNodeBaseParent(Parent);
+ }
+};
+template <class NodeTy> class node_parent_access<NodeTy, void> {};
+
} // end namespace ilist_detail
template <class OptionsT, bool IsReverse, bool IsConst> class ilist_iterator;
@@ -51,7 +68,11 @@ class ilist_select_iterator_type<true, Opts, arg1, arg2> {
/// This is a wrapper around \a ilist_node_base whose main purpose is to
/// provide type safety: you can't insert nodes of \a ilist_node_impl into the
/// wrong \a simple_ilist or \a iplist.
-template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
+template <class OptionsT>
+class ilist_node_impl
+ : OptionsT::node_base_type,
+ public ilist_detail::node_parent_access<
+ ilist_node_impl<OptionsT>, typename OptionsT::parent_ptr_ty> {
using value_type = typename OptionsT::value_type;
using node_base_type = typename OptionsT::node_base_type;
using list_base_type = typename OptionsT::list_base_type;
@@ -60,6 +81,8 @@ template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
friend struct ilist_detail::NodeAccess;
friend class ilist_sentinel<OptionsT>;
+ friend class ilist_detail::node_parent_access<
+ ilist_node_impl<OptionsT>, typename OptionsT::parent_ptr_ty>;
friend class ilist_iterator<OptionsT, false, false>;
friend class ilist_iterator<OptionsT, false, true>;
friend class ilist_iterator<OptionsT, true, false>;
@@ -118,9 +141,7 @@ template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
}
// Under-approximation, but always available for assertions.
- using node_base_type::getNodeBaseParent;
using node_base_type::isKnownSentinel;
- using node_base_type::setNodeBaseParent;
/// Check whether this is the sentinel node.
///
@@ -174,13 +195,18 @@ template <class OptionsT> class ilist_node_impl : OptionsT::node_base_type {
/// \endexample
///
/// When the \a ilist_parent<ParentTy> option is passed to an ilist_node and the
-/// owning ilist, each node contains a pointer to the ilist's owner. This
-/// pointer does not have any automatic behaviour; set it manually, including
-/// for the sentinel node when the list is created. The primary benefit of this
-/// over declaring and using this pointer in the final node class is that the
-/// pointer will be added in the sentinel, meaning that you can safely use \a
+/// owning ilist, each node contains a pointer to the ilist's owner. This adds
+/// \a getParent() and \a setParent(ParentTy*) methods to the ilist_node, which
+/// will be used for node access by the ilist if the node class publicly
+/// inherits from \a ilist_node_with_parent. By default, setParent() is not
+/// automatically called by the ilist; a SymbolTableList will call setParent()
+/// on inserted nodes, but the sentinel must still be manually set after the
+/// list is created (e.g. SymTabList.end()->setParent(Parent)).
+///
+/// The primary benefit of using ilist_parent is that a parent
+/// pointer will be stored in the sentinel, meaning that you can safely use \a
/// ilist_iterator::getNodeParent() to get the node parent from any valid (i.e.
-/// non-null) iterator, even a sentinel value.
+/// non-null) iterator, even one that points to a sentinel value.
///
/// See \a is_valid_option for steps on adding a new option.
template <class T, class... Options>
diff --git a/llvm/include/llvm/ADT/ilist_node_base.h b/llvm/include/llvm/ADT/ilist_node_base.h
index e396bb22fca02..caad87cdd4d71 100644
--- a/llvm/include/llvm/ADT/ilist_node_base.h
+++ b/llvm/include/llvm/ADT/ilist_node_base.h
@@ -13,84 +13,60 @@
namespace llvm {
-/// Base class for ilist nodes.
-///
-/// Optionally tracks whether this node is the sentinel.
-template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_node_base;
+namespace ilist_detail {
-template <> class ilist_node_base<false, void> {
- ilist_node_base *Prev = nullptr;
- ilist_node_base *Next = nullptr;
+template <class NodeBase, bool EnableSentinelTracking> class node_base_prevnext;
-public:
- void setPrev(ilist_node_base *Prev) { this->Prev = Prev; }
- void setNext(ilist_node_base *Next) { this->Next = Next; }
- ilist_node_base *getPrev() const { return Prev; }
- ilist_node_base *getNext() const { return Next; }
+template <class NodeBase> class node_base_prevnext<NodeBase, false> {
+ NodeBase *Prev = nullptr;
+ NodeBase *Next = nullptr;
- void setNodeBaseParent(void) {}
- inline void getNodeBaseParent() const {}
+public:
+ void setPrev(NodeBase *Prev) { this->Prev = Prev; }
+ void setNext(NodeBase *Next) { this->Next = Next; }
+ NodeBase *getPrev() const { return Prev; }
+ NodeBase *getNext() const { return Next; }
bool isKnownSentinel() const { return false; }
void initializeSentinel() {}
};
-template <> class ilist_node_base<true, void> {
- PointerIntPair<ilist_node_base *, 1> PrevAndSentinel;
- ilist_node_base *Next = nullptr;
+template <class NodeBase> class node_base_prevnext<NodeBase, true> {
+ PointerIntPair<NodeBase *, 1> PrevAndSentinel;
+ NodeBase *Next = nullptr;
public:
- void setPrev(ilist_node_base *Prev) { PrevAndSentinel.setPointer(Prev); }
- void setNext(ilist_node_base *Next) { this->Next = Next; }
- ilist_node_base *getPrev() const { return PrevAndSentinel.getPointer(); }
- ilist_node_base *getNext() const { return Next; }
-
- void setNodeBaseParent(void) {}
- inline void getNodeBaseParent() const {}
+ void setPrev(NodeBase *Prev) { PrevAndSentinel.setPointer(Prev); }
+ void setNext(NodeBase *Next) { this->Next = Next; }
+ NodeBase *getPrev() const { return PrevAndSentinel.getPointer(); }
+ NodeBase *getNext() const { return Next; }
bool isSentinel() const { return PrevAndSentinel.getInt(); }
bool isKnownSentinel() const { return isSentinel(); }
void initializeSentinel() { PrevAndSentinel.setInt(true); }
};
-template <class ParentPtrTy> class ilist_node_base<false, ParentPtrTy> {
- ilist_node_base *Prev = nullptr;
- ilist_node_base *Next = nullptr;
+template <class ParentPtrTy> class node_base_parent {
ParentPtrTy Parent = nullptr;
public:
- void setPrev(ilist_node_base *Prev) { this->Prev = Prev; }
- void setNext(ilist_node_base *Next) { this->Next = Next; }
- ilist_node_base *getPrev() const { return Prev; }
- ilist_node_base *getNext() const { return Next; }
-
void setNodeBaseParent(ParentPtrTy Parent) { this->Parent = Parent; }
inline const ParentPtrTy getNodeBaseParent() const { return Parent; }
inline ParentPtrTy getNodeBaseParent() { return Parent; }
-
- bool isKnownSentinel() const { return false; }
- void initializeSentinel() {}
};
+template <> class node_base_parent<void> {};
-template <class ParentPtrTy> class ilist_node_base<true, ParentPtrTy> {
- PointerIntPair<ilist_node_base *, 1> PrevAndSentinel;
- ilist_node_base *Next = nullptr;
- ParentPtrTy Parent = nullptr;
-
-public:
- void setPrev(ilist_node_base *Prev) { PrevAndSentinel.setPointer(Prev); }
- void setNext(ilist_node_base *Next) { this->Next = Next; }
- ilist_node_base *getPrev() const { return PrevAndSentinel.getPointer(); }
- ilist_node_base *getNext() const { return Next; }
-
- void setNodeBaseParent(ParentPtrTy Parent) { this->Parent = Parent; }
- inline const ParentPtrTy getNodeBaseParent() const { return Parent; }
- inline ParentPtrTy getNodeBaseParent() { return Parent; }
+} // end namespace ilist_detail
- bool isSentinel() const { return PrevAndSentinel.getInt(); }
- bool isKnownSentinel() const { return isSentinel(); }
- void initializeSentinel() { PrevAndSentinel.setInt(true); }
-};
+/// Base class for ilist nodes.
+///
+/// Optionally tracks whether this node is the sentinel.
+template <bool EnableSentinelTracking, class ParentPtrTy>
+class ilist_node_base
+ : public ilist_detail::node_base_prevnext<
+ ilist_node_base<EnableSentinelTracking, ParentPtrTy>,
+ EnableSentinelTracking>,
+ public ilist_detail::node_base_parent<ParentPtrTy> {};
} // end namespace llvm
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index e1cb362ad20d0..0b636fc7ceaa3 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -151,9 +151,6 @@ class Instruction : public User,
Instruction *user_back() { return cast<Instruction>(*user_begin());}
const Instruction *user_back() const { return cast<Instruction>(*user_begin());}
- inline const BasicBlock *getParent() const { return getNodeBaseParent(); }
- inline BasicBlock *getParent() { return getNodeBaseParent(); }
-
/// Return the module owning the function this instruction belongs to
/// or nullptr it the function does not have a module.
///
@@ -996,8 +993,6 @@ class Instruction : public User,
return Value::getSubclassDataFromValue();
}
- void setParent(BasicBlock *P);
-
protected:
// Instruction subclasses can stick up to 15 bits of stuff into the
// SubclassData field of instruction with these members.
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index a5199f36c1cb5..b0caa97ed3918 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -189,7 +189,7 @@ BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent,
assert(!InsertBefore &&
"Cannot insert block before another block with no function!");
- end().getNodePtr()->setNodeBaseParent(this);
+ end().getNodePtr()->setParent(this);
setName(Name);
if (NewParent)
setIsNewDbgInfoFormat(NewParent->IsNewDbgInfoFormat);
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 8e8131113a230..a5d95de033418 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -74,8 +74,6 @@ Instruction::~Instruction() {
setMetadata(LLVMContext::MD_DIAssignID, nullptr);
}
-void Instruction::setParent(BasicBlock *P) { setNodeBaseParent(P); }
-
const Module *Instruction::getModule() const {
return getParent()->getModule();
}
>From 7150d6125c222b03fb04848cf00bab959c59d8ba Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Tue, 4 Jun 2024 13:24:30 +0100
Subject: [PATCH 3/3] Update ilist_iterator_w_bits, missed it last commit
---
llvm/include/llvm/ADT/ilist_iterator.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/ADT/ilist_iterator.h b/llvm/include/llvm/ADT/ilist_iterator.h
index 5f27dd2f3f447..6e591d306eac6 100644
--- a/llvm/include/llvm/ADT/ilist_iterator.h
+++ b/llvm/include/llvm/ADT/ilist_iterator.h
@@ -97,7 +97,6 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT>,
using iterator_category = std::bidirectional_iterator_tag;
using const_pointer = typename OptionsT::const_pointer;
using const_reference = typename OptionsT::const_reference;
- using parent_ptr_ty = typename OptionsT::parent_ptr_ty;
private:
using node_pointer = typename Traits::node_pointer;
@@ -209,7 +208,11 @@ class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT>,
/// 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> {
+class ilist_iterator_w_bits
+ : ilist_detail::SpecificNodeAccess<OptionsT>,
+ public ilist_detail::iterator_parent_access<
+ ilist_iterator_w_bits<OptionsT, IsReverse, IsConst>,
+ typename OptionsT::parent_ptr_ty, IsConst> {
friend ilist_iterator_w_bits<OptionsT, IsReverse, !IsConst>;
friend ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst>;
friend ilist_iterator<OptionsT, !IsReverse, !IsConst>;
@@ -225,7 +228,6 @@ class ilist_iterator_w_bits : ilist_detail::SpecificNodeAccess<OptionsT> {
using iterator_category = std::bidirectional_iterator_tag;
using const_pointer = typename OptionsT::const_pointer;
using const_reference = typename OptionsT::const_reference;
- using parent_ptr_ty = typename OptionsT::parent_ptr_ty;
private:
using node_pointer = typename Traits::node_pointer;
@@ -350,8 +352,6 @@ class ilist_iterator_w_bits : ilist_detail::SpecificNodeAccess<OptionsT> {
return tmp;
}
- parent_ptr_ty getNodeParent() { return NodePtr->getNodeBaseParent(); }
-
bool isValid() const { return NodePtr; }
/// Get the underlying ilist_node.
More information about the llvm-commits
mailing list