[llvm] r281170 - CodeGen: Assert that bundle iterators are valid
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 11 10:12:28 PDT 2016
Author: dexonsmith
Date: Sun Sep 11 12:12:28 2016
New Revision: 281170
URL: http://llvm.org/viewvc/llvm-project?rev=281170&view=rev
Log:
CodeGen: Assert that bundle iterators are valid
Add an assertion to the MachineInstrBundleIterator from instr_iterator
that the underlying iterator is valid. This is possible know that we
can check ilist_node::isSentinel (since r281168), and is consistent with
the constructors from MachineInstr* and MachineInstr&.
Avoiding the new assertion in operator== and operator!= requires four
(!!!!) new overloads each.
(As an aside, I'm strongly in favour of:
- making the conversion from instr_iterator explicit;
- making the conversion from pointer explicit;
- making the conversion from reference explicit; and
- removing all the extra overloads of operator== and operator!= except
const_instr_iterator.
I'm not signing up for that at this point, but being clear about when
something is an MachineInstr-iterator (possibly instr_end()) vs
MachineInstr-bundle-iterator (possibly end()) vs MachineInstr* (possibly
nullptr) vs MachineInstr& (known valid) would surely make code
cleaner... and it would remove a ton of boilerplate from
MachineInstrBundleIterator operators.)
Modified:
llvm/trunk/include/llvm/ADT/ilist_iterator.h
llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h
llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp
Modified: llvm/trunk/include/llvm/ADT/ilist_iterator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist_iterator.h?rev=281170&r1=281169&r2=281170&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist_iterator.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist_iterator.h Sun Sep 11 12:12:28 2016
@@ -160,6 +160,9 @@ public:
/// 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; }
};
template <typename From> struct simplify_type;
Modified: llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h?rev=281170&r1=281169&r2=281170&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h Sun Sep 11 12:12:28 2016
@@ -23,18 +23,20 @@ template <class T> struct MachineInstrBu
typedef simple_ilist<T, ilist_sentinel_tracking<true>> list_type;
typedef typename list_type::iterator instr_iterator;
typedef typename list_type::iterator nonconst_instr_iterator;
+ typedef typename list_type::const_iterator const_instr_iterator;
};
template <class T> struct MachineInstrBundleIteratorTraits<const T> {
typedef simple_ilist<T, ilist_sentinel_tracking<true>> list_type;
typedef typename list_type::const_iterator instr_iterator;
typedef typename list_type::iterator nonconst_instr_iterator;
+ typedef typename list_type::const_iterator const_instr_iterator;
};
/// MachineBasicBlock iterator that automatically skips over MIs that are
/// inside bundles (i.e. walk top level MIs only).
template <typename Ty> class MachineInstrBundleIterator {
- typedef typename MachineInstrBundleIteratorTraits<Ty>::instr_iterator
- instr_iterator;
+ typedef MachineInstrBundleIteratorTraits<Ty> Traits;
+ typedef typename Traits::instr_iterator instr_iterator;
instr_iterator MII;
public:
@@ -48,13 +50,18 @@ public:
typedef typename instr_iterator::const_reference const_reference;
private:
- typedef typename std::remove_const<value_type>::type nonconst_value_type;
- typedef typename MachineInstrBundleIteratorTraits<Ty>::nonconst_instr_iterator
- nonconst_instr_iterator;
- typedef MachineInstrBundleIterator<nonconst_value_type> nonconst_iterator;
+ typedef typename Traits::nonconst_instr_iterator nonconst_instr_iterator;
+ typedef typename Traits::const_instr_iterator const_instr_iterator;
+ typedef MachineInstrBundleIterator<
+ typename nonconst_instr_iterator::value_type>
+ nonconst_iterator;
public:
- MachineInstrBundleIterator(instr_iterator MI) : MII(MI) {}
+ MachineInstrBundleIterator(instr_iterator MI) : MII(MI) {
+ assert((!MI.getNodePtr() || MI.isEnd() || !MI->isBundledWithPred()) &&
+ "It's not legal to initialize MachineInstrBundleIterator with a "
+ "bundled MI");
+ }
MachineInstrBundleIterator(reference MI) : MII(MI) {
assert(!MI.isBundledWithPred() && "It's not legal to initialize "
@@ -86,13 +93,27 @@ public:
const MachineInstrBundleIterator &R) {
return L.MII == R.MII;
}
+ friend bool operator==(const MachineInstrBundleIterator &L,
+ const const_instr_iterator &R) {
+ return L.MII == R; // Avoid assertion about validity of R.
+ }
+ friend bool operator==(const const_instr_iterator &L,
+ const MachineInstrBundleIterator &R) {
+ return L == R.MII; // Avoid assertion about validity of L.
+ }
+ friend bool operator==(const MachineInstrBundleIterator &L,
+ const nonconst_instr_iterator &R) {
+ return L.MII == R; // Avoid assertion about validity of R.
+ }
+ friend bool operator==(const nonconst_instr_iterator &L,
+ const MachineInstrBundleIterator &R) {
+ return L == R.MII; // Avoid assertion about validity of L.
+ }
friend bool operator==(const MachineInstrBundleIterator &L, const_pointer R) {
- // Avoid assertion about validity of R.
- return L.MII == instr_iterator(const_cast<pointer>(R));
+ return L == const_instr_iterator(R); // Avoid assertion about validity of R.
}
friend bool operator==(const_pointer L, const MachineInstrBundleIterator &R) {
- // Avoid assertion about validity of L.
- return instr_iterator(const_cast<pointer>(L)) == R.MII;
+ return const_instr_iterator(L) == R; // Avoid assertion about validity of L.
}
friend bool operator==(const MachineInstrBundleIterator &L,
const_reference R) {
@@ -107,6 +128,22 @@ public:
const MachineInstrBundleIterator &R) {
return !(L == R);
}
+ friend bool operator!=(const MachineInstrBundleIterator &L,
+ const const_instr_iterator &R) {
+ return !(L == R);
+ }
+ friend bool operator!=(const const_instr_iterator &L,
+ const MachineInstrBundleIterator &R) {
+ return !(L == R);
+ }
+ friend bool operator!=(const MachineInstrBundleIterator &L,
+ const nonconst_instr_iterator &R) {
+ return !(L == R);
+ }
+ friend bool operator!=(const nonconst_instr_iterator &L,
+ const MachineInstrBundleIterator &R) {
+ return !(L == R);
+ }
friend bool operator!=(const MachineInstrBundleIterator &L, const_pointer R) {
return !(L == R);
}
Modified: llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp?rev=281170&r1=281169&r2=281170&view=diff
==============================================================================
--- llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp (original)
+++ llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp Sun Sep 11 12:12:28 2016
@@ -74,6 +74,14 @@ TEST(MachineInstrBundleIteratorTest, Com
ASSERT_FALSE(CI == &MBI);
ASSERT_FALSE(CI == CMBI);
ASSERT_FALSE(CI == &CMBI);
+ ASSERT_FALSE(MBI.getIterator() == I);
+ ASSERT_FALSE(CMBI.getIterator() == I);
+ ASSERT_FALSE(I == MBI.getIterator());
+ ASSERT_FALSE(I == CMBI.getIterator());
+ ASSERT_FALSE(MBI.getIterator() == CI);
+ ASSERT_FALSE(CMBI.getIterator() == CI);
+ ASSERT_FALSE(CI == MBI.getIterator());
+ ASSERT_FALSE(CI == CMBI.getIterator());
ASSERT_TRUE(MBI != I);
ASSERT_TRUE(&MBI != I);
ASSERT_TRUE(CMBI != I);
@@ -90,6 +98,14 @@ TEST(MachineInstrBundleIteratorTest, Com
ASSERT_TRUE(CI != &MBI);
ASSERT_TRUE(CI != CMBI);
ASSERT_TRUE(CI != &CMBI);
+ ASSERT_TRUE(MBI.getIterator() != I);
+ ASSERT_TRUE(CMBI.getIterator() != I);
+ ASSERT_TRUE(I != MBI.getIterator());
+ ASSERT_TRUE(I != CMBI.getIterator());
+ ASSERT_TRUE(MBI.getIterator() != CI);
+ ASSERT_TRUE(CMBI.getIterator() != CI);
+ ASSERT_TRUE(CI != MBI.getIterator());
+ ASSERT_TRUE(CI != CMBI.getIterator());
}
} // end namespace
More information about the llvm-commits
mailing list