maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity
llvm-project/llvm/unittests/CodeGen/MachineInstrBundleIteratorT...

198 lines
7.3 KiB
C++
Raw Normal View History

ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
//===- MachineInstrBundleIteratorTest.cpp ---------------------------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#include "llvm/ADT/ilist_node.h"
#include "llvm/CodeGen/MachineInstrBundleIterator.h"
#include "gtest/gtest.h"
using namespace llvm;
namespace {
CodeGen: Turn on sentinel tracking for MachineInstr iterators This is a prep commit before fixing MachineBasicBlock::reverse_iterator invalidation semantics, ala r281167 for ilist::reverse_iterator. This changes MachineBasicBlock::Instructions to track which node is the sentinel regardless of LLVM_ENABLE_ABI_BREAKING_CHECKS. There's almost no functionality change (aside from ABI). However, in the rare configuration: #if !defined(NDEBUG) && !defined(LLVM_ENABLE_ABI_BREAKING_CHECKS) the isKnownSentinel() assertions in ilist_iterator<>::operator* suddenly have teeth for MachineInstr. If these assertions start firing for your out-of-tree backend, have a look at the suggestions in the commit message for r279314, and at some of the commits leading up to it that avoid dereferencing the end() iterator. llvm-svn: 281168
2016-09-12 00:38:18 +08:00
struct MyBundledInstr
: public ilist_node<MyBundledInstr, ilist_sentinel_tracking<true>> {
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
bool isBundledWithPred() const { return true; }
bool isBundledWithSucc() const { return true; }
};
typedef MachineInstrBundleIterator<MyBundledInstr> bundled_iterator;
typedef MachineInstrBundleIterator<const MyBundledInstr> const_bundled_iterator;
CodeGen: Give MachineBasicBlock::reverse_iterator a handle to the current MI Now that MachineBasicBlock::reverse_instr_iterator knows when it's at the end (since r281168 and r281170), implement MachineBasicBlock::reverse_iterator directly on top of an ilist::reverse_iterator by adding an IsReverse template parameter to MachineInstrBundleIterator. This replaces another hard-to-reason-about use of std::reverse_iterator on list iterators, matching the changes for ilist::reverse_iterator from r280032 (see the "out of scope" section at the end of that commit message). MachineBasicBlock::reverse_iterator now has a handle to the current node and has obvious invalidation semantics. r280032 has a more detailed explanation of how list-style reverse iterators (invalidated when the pointed-at node is deleted) are different from vector-style reverse iterators like std::reverse_iterator (invalidated on every operation). A great motivating example is this commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp. Note: If your out-of-tree backend deletes instructions while iterating on a MachineBasicBlock::reverse_iterator or converts between MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator, you'll need to update your code in similar ways to r280032. The following table might help: [Old] ==> [New] delete &*RI, RE = end() delete &*RI++ RI->erase(), RE = end() RI++->erase() reverse_iterator(I) std::prev(I).getReverse() reverse_iterator(I) ++I.getReverse() --reverse_iterator(I) I.getReverse() reverse_iterator(std::next(I)) I.getReverse() RI.base() std::prev(RI).getReverse() RI.base() ++RI.getReverse() --RI.base() RI.getReverse() std::next(RI).base() RI.getReverse() (For more details, have a look at r280032.) llvm-svn: 281172
2016-09-12 02:51:28 +08:00
typedef MachineInstrBundleIterator<MyBundledInstr, true>
reverse_bundled_iterator;
typedef MachineInstrBundleIterator<const MyBundledInstr, true>
const_reverse_bundled_iterator;
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
#ifdef GTEST_HAS_DEATH_TEST
#ifndef NDEBUG
TEST(MachineInstrBundleIteratorTest, CheckForBundles) {
MyBundledInstr MBI;
CodeGen: Give MachineBasicBlock::reverse_iterator a handle to the current MI Now that MachineBasicBlock::reverse_instr_iterator knows when it's at the end (since r281168 and r281170), implement MachineBasicBlock::reverse_iterator directly on top of an ilist::reverse_iterator by adding an IsReverse template parameter to MachineInstrBundleIterator. This replaces another hard-to-reason-about use of std::reverse_iterator on list iterators, matching the changes for ilist::reverse_iterator from r280032 (see the "out of scope" section at the end of that commit message). MachineBasicBlock::reverse_iterator now has a handle to the current node and has obvious invalidation semantics. r280032 has a more detailed explanation of how list-style reverse iterators (invalidated when the pointed-at node is deleted) are different from vector-style reverse iterators like std::reverse_iterator (invalidated on every operation). A great motivating example is this commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp. Note: If your out-of-tree backend deletes instructions while iterating on a MachineBasicBlock::reverse_iterator or converts between MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator, you'll need to update your code in similar ways to r280032. The following table might help: [Old] ==> [New] delete &*RI, RE = end() delete &*RI++ RI->erase(), RE = end() RI++->erase() reverse_iterator(I) std::prev(I).getReverse() reverse_iterator(I) ++I.getReverse() --reverse_iterator(I) I.getReverse() reverse_iterator(std::next(I)) I.getReverse() RI.base() std::prev(RI).getReverse() RI.base() ++RI.getReverse() --RI.base() RI.getReverse() std::next(RI).base() RI.getReverse() (For more details, have a look at r280032.) llvm-svn: 281172
2016-09-12 02:51:28 +08:00
auto I = MBI.getIterator();
auto RI = I.getReverse();
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
// Confirm that MBI is always considered bundled.
EXPECT_TRUE(MBI.isBundledWithPred());
EXPECT_TRUE(MBI.isBundledWithSucc());
// Confirm that iterators check in their constructor for bundled iterators.
CodeGen: Give MachineBasicBlock::reverse_iterator a handle to the current MI Now that MachineBasicBlock::reverse_instr_iterator knows when it's at the end (since r281168 and r281170), implement MachineBasicBlock::reverse_iterator directly on top of an ilist::reverse_iterator by adding an IsReverse template parameter to MachineInstrBundleIterator. This replaces another hard-to-reason-about use of std::reverse_iterator on list iterators, matching the changes for ilist::reverse_iterator from r280032 (see the "out of scope" section at the end of that commit message). MachineBasicBlock::reverse_iterator now has a handle to the current node and has obvious invalidation semantics. r280032 has a more detailed explanation of how list-style reverse iterators (invalidated when the pointed-at node is deleted) are different from vector-style reverse iterators like std::reverse_iterator (invalidated on every operation). A great motivating example is this commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp. Note: If your out-of-tree backend deletes instructions while iterating on a MachineBasicBlock::reverse_iterator or converts between MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator, you'll need to update your code in similar ways to r280032. The following table might help: [Old] ==> [New] delete &*RI, RE = end() delete &*RI++ RI->erase(), RE = end() RI++->erase() reverse_iterator(I) std::prev(I).getReverse() reverse_iterator(I) ++I.getReverse() --reverse_iterator(I) I.getReverse() reverse_iterator(std::next(I)) I.getReverse() RI.base() std::prev(RI).getReverse() RI.base() ++RI.getReverse() --RI.base() RI.getReverse() std::next(RI).base() RI.getReverse() (For more details, have a look at r280032.) llvm-svn: 281172
2016-09-12 02:51:28 +08:00
EXPECT_DEATH((void)static_cast<bundled_iterator>(I),
"not legal to initialize");
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
EXPECT_DEATH((void)static_cast<bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<bundled_iterator>(&MBI),
"not legal to initialize");
CodeGen: Give MachineBasicBlock::reverse_iterator a handle to the current MI Now that MachineBasicBlock::reverse_instr_iterator knows when it's at the end (since r281168 and r281170), implement MachineBasicBlock::reverse_iterator directly on top of an ilist::reverse_iterator by adding an IsReverse template parameter to MachineInstrBundleIterator. This replaces another hard-to-reason-about use of std::reverse_iterator on list iterators, matching the changes for ilist::reverse_iterator from r280032 (see the "out of scope" section at the end of that commit message). MachineBasicBlock::reverse_iterator now has a handle to the current node and has obvious invalidation semantics. r280032 has a more detailed explanation of how list-style reverse iterators (invalidated when the pointed-at node is deleted) are different from vector-style reverse iterators like std::reverse_iterator (invalidated on every operation). A great motivating example is this commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp. Note: If your out-of-tree backend deletes instructions while iterating on a MachineBasicBlock::reverse_iterator or converts between MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator, you'll need to update your code in similar ways to r280032. The following table might help: [Old] ==> [New] delete &*RI, RE = end() delete &*RI++ RI->erase(), RE = end() RI++->erase() reverse_iterator(I) std::prev(I).getReverse() reverse_iterator(I) ++I.getReverse() --reverse_iterator(I) I.getReverse() reverse_iterator(std::next(I)) I.getReverse() RI.base() std::prev(RI).getReverse() RI.base() ++RI.getReverse() --RI.base() RI.getReverse() std::next(RI).base() RI.getReverse() (For more details, have a look at r280032.) llvm-svn: 281172
2016-09-12 02:51:28 +08:00
EXPECT_DEATH((void)static_cast<const_bundled_iterator>(I),
"not legal to initialize");
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
EXPECT_DEATH((void)static_cast<const_bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_bundled_iterator>(&MBI),
"not legal to initialize");
CodeGen: Give MachineBasicBlock::reverse_iterator a handle to the current MI Now that MachineBasicBlock::reverse_instr_iterator knows when it's at the end (since r281168 and r281170), implement MachineBasicBlock::reverse_iterator directly on top of an ilist::reverse_iterator by adding an IsReverse template parameter to MachineInstrBundleIterator. This replaces another hard-to-reason-about use of std::reverse_iterator on list iterators, matching the changes for ilist::reverse_iterator from r280032 (see the "out of scope" section at the end of that commit message). MachineBasicBlock::reverse_iterator now has a handle to the current node and has obvious invalidation semantics. r280032 has a more detailed explanation of how list-style reverse iterators (invalidated when the pointed-at node is deleted) are different from vector-style reverse iterators like std::reverse_iterator (invalidated on every operation). A great motivating example is this commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp. Note: If your out-of-tree backend deletes instructions while iterating on a MachineBasicBlock::reverse_iterator or converts between MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator, you'll need to update your code in similar ways to r280032. The following table might help: [Old] ==> [New] delete &*RI, RE = end() delete &*RI++ RI->erase(), RE = end() RI++->erase() reverse_iterator(I) std::prev(I).getReverse() reverse_iterator(I) ++I.getReverse() --reverse_iterator(I) I.getReverse() reverse_iterator(std::next(I)) I.getReverse() RI.base() std::prev(RI).getReverse() RI.base() ++RI.getReverse() --RI.base() RI.getReverse() std::next(RI).base() RI.getReverse() (For more details, have a look at r280032.) llvm-svn: 281172
2016-09-12 02:51:28 +08:00
EXPECT_DEATH((void)static_cast<reverse_bundled_iterator>(RI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<reverse_bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<reverse_bundled_iterator>(&MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_reverse_bundled_iterator>(RI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_reverse_bundled_iterator>(MBI),
"not legal to initialize");
EXPECT_DEATH((void)static_cast<const_reverse_bundled_iterator>(&MBI),
"not legal to initialize");
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
}
#endif
#endif
TEST(MachineInstrBundleIteratorTest, CompareToBundledMI) {
MyBundledInstr MBI;
const MyBundledInstr &CMBI = MBI;
bundled_iterator I;
const_bundled_iterator CI;
// Confirm that MBI is always considered bundled.
EXPECT_TRUE(MBI.isBundledWithPred());
EXPECT_TRUE(MBI.isBundledWithSucc());
// These invocations will crash when !NDEBUG if a conversion is taking place.
// These checks confirm that comparison operators don't use any conversion
// operators.
ASSERT_FALSE(MBI == I);
ASSERT_FALSE(&MBI == I);
ASSERT_FALSE(CMBI == I);
ASSERT_FALSE(&CMBI == I);
ASSERT_FALSE(I == MBI);
ASSERT_FALSE(I == &MBI);
ASSERT_FALSE(I == CMBI);
ASSERT_FALSE(I == &CMBI);
ASSERT_FALSE(MBI == CI);
ASSERT_FALSE(&MBI == CI);
ASSERT_FALSE(CMBI == CI);
ASSERT_FALSE(&CMBI == CI);
ASSERT_FALSE(CI == MBI);
ASSERT_FALSE(CI == &MBI);
ASSERT_FALSE(CI == CMBI);
ASSERT_FALSE(CI == &CMBI);
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.) llvm-svn: 281170
2016-09-12 01:12:28 +08:00
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());
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
ASSERT_TRUE(MBI != I);
ASSERT_TRUE(&MBI != I);
ASSERT_TRUE(CMBI != I);
ASSERT_TRUE(&CMBI != I);
ASSERT_TRUE(I != MBI);
ASSERT_TRUE(I != &MBI);
ASSERT_TRUE(I != CMBI);
ASSERT_TRUE(I != &CMBI);
ASSERT_TRUE(MBI != CI);
ASSERT_TRUE(&MBI != CI);
ASSERT_TRUE(CMBI != CI);
ASSERT_TRUE(&CMBI != CI);
ASSERT_TRUE(CI != MBI);
ASSERT_TRUE(CI != &MBI);
ASSERT_TRUE(CI != CMBI);
ASSERT_TRUE(CI != &CMBI);
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.) llvm-svn: 281170
2016-09-12 01:12:28 +08:00
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());
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
}
ADT: Add explicit conversions for reverse ilist iterators Add explicit conversions between forward and reverse ilist iterators. These follow the conversion conventions of std::reverse_iterator, which are off-by-one: the newly-constructed "reverse" iterator dereferences to the previous node of the one sent in. This has the benefit of converting reverse ranges in place: - If [I, E) is a valid range, - then [reverse(E), reverse(I)) gives the same range in reverse order. ilist_iterator::getReverse() is unchanged: it returns a reverse iterator to the *same* node. llvm-svn: 294349
2017-02-08 05:03:50 +08:00
struct MyUnbundledInstr
: ilist_node<MyUnbundledInstr, ilist_sentinel_tracking<true>> {
bool isBundledWithPred() const { return false; }
bool isBundledWithSucc() const { return false; }
};
typedef MachineInstrBundleIterator<MyUnbundledInstr> unbundled_iterator;
typedef MachineInstrBundleIterator<const MyUnbundledInstr>
const_unbundled_iterator;
typedef MachineInstrBundleIterator<MyUnbundledInstr, true>
reverse_unbundled_iterator;
typedef MachineInstrBundleIterator<const MyUnbundledInstr, true>
const_reverse_unbundled_iterator;
TEST(MachineInstrBundleIteratorTest, ReverseConstructor) {
simple_ilist<MyUnbundledInstr, ilist_sentinel_tracking<true>> L;
const auto &CL = L;
MyUnbundledInstr A, B;
L.insert(L.end(), A);
L.insert(L.end(), B);
// Save typing.
typedef MachineInstrBundleIterator<MyUnbundledInstr> iterator;
typedef MachineInstrBundleIterator<MyUnbundledInstr, true> reverse_iterator;
typedef MachineInstrBundleIterator<const MyUnbundledInstr> const_iterator;
typedef MachineInstrBundleIterator<const MyUnbundledInstr, true>
const_reverse_iterator;
// Convert to bundle iterators.
auto begin = [&]() -> iterator { return L.begin(); };
auto end = [&]() -> iterator { return L.end(); };
auto rbegin = [&]() -> reverse_iterator { return L.rbegin(); };
auto rend = [&]() -> reverse_iterator { return L.rend(); };
auto cbegin = [&]() -> const_iterator { return CL.begin(); };
auto cend = [&]() -> const_iterator { return CL.end(); };
auto crbegin = [&]() -> const_reverse_iterator { return CL.rbegin(); };
auto crend = [&]() -> const_reverse_iterator { return CL.rend(); };
// Check conversion values.
EXPECT_EQ(begin(), iterator(rend()));
EXPECT_EQ(++begin(), iterator(++rbegin()));
EXPECT_EQ(end(), iterator(rbegin()));
EXPECT_EQ(rbegin(), reverse_iterator(end()));
EXPECT_EQ(++rbegin(), reverse_iterator(++begin()));
EXPECT_EQ(rend(), reverse_iterator(begin()));
// Check const iterator constructors.
EXPECT_EQ(cbegin(), const_iterator(rend()));
EXPECT_EQ(cbegin(), const_iterator(crend()));
EXPECT_EQ(crbegin(), const_reverse_iterator(end()));
EXPECT_EQ(crbegin(), const_reverse_iterator(cend()));
// Confirm lack of implicit conversions.
static_assert(!std::is_convertible<iterator, reverse_iterator>::value,
"unexpected implicit conversion");
static_assert(!std::is_convertible<reverse_iterator, iterator>::value,
"unexpected implicit conversion");
static_assert(
!std::is_convertible<const_iterator, const_reverse_iterator>::value,
"unexpected implicit conversion");
static_assert(
!std::is_convertible<const_reverse_iterator, const_iterator>::value,
"unexpected implicit conversion");
}
ADT: Remove all ilist_iterator => pointer casts, NFC Remove all ilist_iterator to pointer casts. There were two reasons for casts: - Checking for an uninitialized (i.e., null) iterator. I added MachineInstrBundleIterator::isValid() to check for that case. - Comparing an iterator against the underlying pointer value while avoiding converting the pointer value to an iterator. This is occasionally necessary in MachineInstrBundleIterator, since there is an assertion in the constructors that the underlying MachineInstr is not bundled (but we don't care about that if we're just checking for pointer equality). To support the latter case, I rewrote the == and != operators for ilist_iterator and MachineInstrBundleIterator. - The implicit constructors now use enable_if to exclude const-iterator => non-const-iterator conversions from overload resolution (previously it was a compiler error on instantiation, now it's SFINAE). - The == and != operators are now global (friends), and are not templated. - MachineInstrBundleIterator has overloads to compare against both const_pointer and const_reference. This avoids the implicit conversions to MachineInstrBundleIterator that assert, instead just checking the address (and I added unit tests to confirm this). Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked are in ilist.h, and no code outside of ilist*.h directly relies on this UB end-iterator-to-pointer conversion anymore. It's still needed for ilist_*sentinel_traits, but I'll clean that up soon. llvm-svn: 278478
2016-08-12 13:05:36 +08:00
} // end namespace