Commit Graph

13 Commits

Author SHA1 Message Date
Eugene Zelenko
1d081e6497 [CodeGen] Fix some Clang-tidy modernize-use-using and Include What You Use warnings; other minor fixes (NFC).
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@304265 91177308-0d34-0410-b5e6-96231b3b80d8
2017-05-31 01:10:10 +00:00
Duncan P. N. Exon Smith
98516248d0 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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294349 91177308-0d34-0410-b5e6-96231b3b80d8
2017-02-07 21:03:50 +00:00
Duncan P. N. Exon Smith
d546df8daf 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.)

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@281172 91177308-0d34-0410-b5e6-96231b3b80d8
2016-09-11 18:51:28 +00:00
Duncan P. N. Exon Smith
00530c8441 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.)

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@281170 91177308-0d34-0410-b5e6-96231b3b80d8
2016-09-11 17:12:28 +00:00
Duncan P. N. Exon Smith
6037e199c8 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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@281168 91177308-0d34-0410-b5e6-96231b3b80d8
2016-09-11 16:38:18 +00:00
Duncan P. N. Exon Smith
b4bd34e805 ADT: Add sentinel tracking and custom tags to ilists
This adds two declarative configuration options for intrusive lists
(available for simple_ilist, iplist, and ilist).  Both of these options
affect ilist_node interoperability and need to be passed both to the
node and the list.  Instead of adding a new traits class, they're
specified as optional template parameters (in any order).

The two options:

 1. Pass ilist_sentinel_tracking<true> or ilist_sentinel_tracking<false>
    to control whether there's a bit on ilist_node "prev" pointer
    indicating whether it's the sentinel.  The default behaviour is to
    use a bit if and only if LLVM_ENABLE_ABI_BREAKING_CHECKS.

 2. Pass ilist_tag<TagA> and ilist_tag<TagB> to allow insertion of a
    single node into two different lists (simultaneously).

I have an immediate use-case for (1) ilist_sentinel_tracking: fixing the
validation semantics of MachineBasicBlock::reverse_iterator to match
ilist::reverse_iterator (ala r280032: see the comments at the end of the
commit message there).  I'm adding (2) ilist_tag in the same commit to
validate that the options framework supports expansion.  Justin Bogner
mentioned this might enable a possible cleanup in SelectionDAG, but I'll
leave this to others to explore.  In the meantime, the unit tests and
the comments for simple_ilist and ilist_node have usage examples.

Note that there's a layer of indirection to support optional,
out-of-order, template paramaters.  Internal classes are templated on an
instantiation of the non-variadic ilist_detail::node_options.
User-facing classes use ilist_detail::compute_node_options to compute
the correct instantiation of ilist_detail::node_options.

The comments for ilist_detail::is_valid_option describe how to add new
options (e.g., ilist_packed_int<int NumBits>).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@281167 91177308-0d34-0410-b5e6-96231b3b80d8
2016-09-11 16:20:53 +00:00
Duncan P. N. Exon Smith
0d65f1da0e ADT: Do not inherit from std::iterator in ilist_iterator
Inheriting from std::iterator uses more boiler-plate than manual
typedefs.  Avoid that in both ilist_iterator and
MachineInstrBundleIterator.

This has the side effect of removing ilist_iterator from certain ADL
lookups in namespace std; calls to std::next need to be qualified by
"std::" that didn't have to before.  The one case of this in-tree was
operating on a temporary, so I used the more compact operator++.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@280570 91177308-0d34-0410-b5e6-96231b3b80d8
2016-09-03 02:27:35 +00:00
Duncan P. N. Exon Smith
e3c3e55fc5 CodeGen: Avoid dereferencing end() when unconstifying iterators
Rather than doing a funny dance that relies on dereferencing end() not
crashing, add some API to MachineInstrBundleIterator to get a non-const
version of the iterator.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@278870 91177308-0d34-0410-b5e6-96231b3b80d8
2016-08-16 23:34:07 +00:00
Duncan P. N. Exon Smith
21bcd75d9b 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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@278478 91177308-0d34-0410-b5e6-96231b3b80d8
2016-08-12 05:05:36 +00:00
Duncan P. N. Exon Smith
fe912bf4b0 CodeGen: Make iterator-to-pointer conversion explicit, NFC
Remove the implicit conversion from MachineInstrBundleIterator to
MachineInstr*, leaving behind an explicit conversion.

I *think* this is the last ilist_iterator-related implicit conversion to
ilist_node subclass.  If I'm right, I can finally dig in and fix the UB
in ilist that these conversions were relying on.

Note that the implicit users of this conversion have already been
removed.  If you have out-of-tree code that doesn't update, you might be
able to buy some time by temporarily reverting this commit.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@276902 91177308-0d34-0410-b5e6-96231b3b80d8
2016-07-27 18:45:18 +00:00
Duncan P. N. Exon Smith
110284e56c CodeGen: Bring back MachineBasicBlock::iterator::getInstrIterator()...
This is a little embarrassing.

When I reverted r261504 (getIterator() => getInstrIterator()) in
r261567, I did a `git grep` to see if there were new calls to
`getInstrIterator()` that I needed to migrate.  There were 10-20 hits,
and I blindly did a `sed ...` before calling `ninja check`.

However, these were `MachineInstrBundleIterator::getInstrIterator()`,
which predated r261567.  Perhaps coincidentally, these had an identical
name and return type.

This commit undoes my careless sed and restores
`MachineBasicBlock::iterator::getInstrIterator()`.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@261577 91177308-0d34-0410-b5e6-96231b3b80d8
2016-02-22 21:30:15 +00:00
Duncan P. N. Exon Smith
20a62528ef Revert "CodeGen: MachineInstr::getIterator() => getInstrIterator(), NFC"
This reverts commit r261504, since it's not obvious the new name is
better:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160222/334298.html

I'll recommit if we get consensus that it's the right direction.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@261567 91177308-0d34-0410-b5e6-96231b3b80d8
2016-02-22 20:49:58 +00:00
Duncan P. N. Exon Smith
b64deed8bb CodeGen: Split bundle_iterator into a separate file, NFC
Split MachineBasicBlock::bundle_iterator into a separate file, and
rename the class to MachineBundleIterator.

This is a precursor to adding a `MachineInstr::getBundleIterator()`
accessor, which will eventually let us delete the final call to
getNodePtrUnchecked(), and then remove the UB from ilist_iterator.

As a drive-by, I removed an unnecessary second template parameter.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@261502 91177308-0d34-0410-b5e6-96231b3b80d8
2016-02-21 22:05:50 +00:00