[llvm] r278532 - ADT: Remove the ilist_nextprev_traits customization point
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 12 15:02:15 PDT 2016
> On 2016-Aug-12, at 15:01, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>>
>> On 2016-Aug-12, at 14:48, Hans Wennborg <hans at chromium.org> wrote:
>>
>> Just a heads up I'm getting compiler crashes building Chromium on
>> Windows with this (well, r278539 really). Still not sure what's going
>> on, though.. http://crbug.com/637413
>>
>> It's crashing here:
>>
>> void X86FrameLowering::emitEpilogue(MachineFunction &MF,
>> MachineBasicBlock &MBB) const {
>> const MachineFrameInfo &MFI = MF.getFrameInfo();
>> X86MachineFunctionInfo *X86FI = MF.getInfo<X86MachineFunctionInfo>();
>> MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator();
>> unsigned RetOpcode = MBBI->getOpcode(); <--- CRASH
>>
>>
>> I assume MBBI is an ilist_iterator<> of some kind, but I still haven't
>> been able to figure out what's up, or reproduce the crash on Linux :-/
>
> Weird, I don't know why that would be caused by this commit. Must affect the optimizer?
>
> Should be reproducible on Linux by adding:
>
> assert(MBBI != MBB.end());
>
> before using it.
>
> I don't know that could.
I don't know that *code*.
> Does something like the following make sense?
>
> if (MBBI == MBB.end())
> return;
>
> ??
>
>>
>>
>> On Fri, Aug 12, 2016 at 10:32 AM, Duncan P. N. Exon Smith via
>> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> Author: dexonsmith
>>> Date: Fri Aug 12 12:32:34 2016
>>> New Revision: 278532
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=278532&view=rev
>>> Log:
>>> ADT: Remove the ilist_nextprev_traits customization point
>>>
>>> No one is using the capability to implement next and prev another way
>>> (since lld stopped doing it in r278468). Remove the customization point
>>> by moving the API from ilist_nextprev_traits<T> to ilist_node_access.
>>>
>>> The old traits class is still useful/necessary API as a target for
>>> friends of node types that inherit privately from ilist_node.
>>> Eventually I plan to either remove it entirely or move the template
>>> parameters to the methods.
>>>
>>> (Note: if there's desire to bring back customization of next/prev
>>> pointers in the future (e.g., to pack some bits in there), I think a
>>> traits class like this is an awkward way to accomplish it. Instead, we
>>> should change ilist<T> to be ilist<ilist_node<T>>, and give an extra
>>> template parameter to ilist_node.)
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/ADT/ilist.h
>>> llvm/trunk/include/llvm/ADT/ilist_node.h
>>> llvm/trunk/include/llvm/IR/SymbolTableListTraits.h
>>>
>>> Modified: llvm/trunk/include/llvm/ADT/ilist.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist.h?rev=278532&r1=278531&r2=278532&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ADT/ilist.h (original)
>>> +++ llvm/trunk/include/llvm/ADT/ilist.h Fri Aug 12 12:32:34 2016
>>> @@ -50,20 +50,64 @@ namespace llvm {
>>> template<typename NodeTy, typename Traits> class iplist;
>>> template<typename NodeTy> class ilist_iterator;
>>>
>>> -/// ilist_nextprev_traits - A fragment for template traits for intrusive list
>>> -/// that provides default next/prev implementations for common operations.
>>> +/// An access class for next/prev on ilist_nodes.
>>> ///
>>> -template<typename NodeTy>
>>> -struct ilist_nextprev_traits {
>>> - static NodeTy *getPrev(NodeTy *N) { return N->getPrev(); }
>>> - static NodeTy *getNext(NodeTy *N) { return N->getNext(); }
>>> - static const NodeTy *getPrev(const NodeTy *N) { return N->getPrev(); }
>>> - static const NodeTy *getNext(const NodeTy *N) { return N->getNext(); }
>>> +/// This gives access to the private parts of ilist nodes. Nodes for an ilist
>>> +/// should friend this class if they inherit privately from ilist_node.
>>> +///
>>> +/// It's strongly discouraged to *use* this class outside of ilist
>>> +/// implementation.
>>> +struct ilist_node_access {
>>> + template <typename NodeTy> static NodeTy *getPrev(NodeTy *N) {
>>> + return N->getPrev();
>>> + }
>>> + template <typename NodeTy> static NodeTy *getNext(NodeTy *N) {
>>> + return N->getNext();
>>> + }
>>> + template <typename NodeTy> static const NodeTy *getPrev(const NodeTy *N) {
>>> + return N->getPrev();
>>> + }
>>> + template <typename NodeTy> static const NodeTy *getNext(const NodeTy *N) {
>>> + return N->getNext();
>>> + }
>>> +
>>> + template <typename NodeTy> static void setPrev(NodeTy *N, NodeTy *Prev) {
>>> + N->setPrev(Prev);
>>> + }
>>> + template <typename NodeTy> static void setNext(NodeTy *N, NodeTy *Next) {
>>> + N->setNext(Next);
>>> + }
>>> + template <typename NodeTy> static void setPrev(NodeTy *N, std::nullptr_t) {
>>> + N->setPrev(nullptr);
>>> + }
>>> + template <typename NodeTy> static void setNext(NodeTy *N, std::nullptr_t) {
>>> + N->setNext(nullptr);
>>> + }
>>> +};
>>> +
>>> +namespace ilist_detail {
>>>
>>> - static void setPrev(NodeTy *N, NodeTy *Prev) { N->setPrev(Prev); }
>>> - static void setNext(NodeTy *N, NodeTy *Next) { N->setNext(Next); }
>>> +template <class T> T &make();
>>> +
>>> +/// Type trait to check for a traits class that has a getNext member (as a
>>> +/// canary for any of the ilist_nextprev_traits API).
>>> +template <class TraitsT, class NodeT> struct HasGetNext {
>>> + typedef char Yes[1];
>>> + typedef char No[2];
>>> + template <size_t N> struct SFINAE {};
>>> +
>>> + template <class U, class V>
>>> + static Yes &hasGetNext(
>>> + SFINAE<sizeof(static_cast<NodeT *>(make<U>().getNext(&make<NodeT>())))>
>>> + * = 0);
>>> + template <class U, class V> static No &hasGetNext(...);
>>> +
>>> + static const bool value =
>>> + sizeof(hasGetNext<TraitsT, NodeT>(nullptr)) == sizeof(Yes);
>>> };
>>>
>>> +} // end namespace ilist_detail
>>> +
>>> template<typename NodeTy>
>>> struct ilist_traits;
>>>
>>> @@ -93,15 +137,15 @@ struct ilist_sentinel_traits {
>>> if (!Head) {
>>> Head = ilist_traits<NodeTy>::createSentinel();
>>> ilist_traits<NodeTy>::noteHead(Head, Head);
>>> - ilist_traits<NodeTy>::setNext(Head, nullptr);
>>> + ilist_node_access::setNext(Head, nullptr);
>>> return Head;
>>> }
>>> - return ilist_traits<NodeTy>::getPrev(Head);
>>> + return ilist_node_access::getPrev(Head);
>>> }
>>>
>>> /// noteHead - stash the sentinel into its default location
>>> static void noteHead(NodeTy *NewHead, NodeTy *Sentinel) {
>>> - ilist_traits<NodeTy>::setPrev(NewHead, Sentinel);
>>> + ilist_node_access::setPrev(NewHead, Sentinel);
>>> }
>>> };
>>>
>>> @@ -187,11 +231,9 @@ struct ilist_node_traits {
>>> /// By inheriting from this, you can easily use default implementations
>>> /// for all common operations.
>>> ///
>>> -template<typename NodeTy>
>>> -struct ilist_default_traits : public ilist_nextprev_traits<NodeTy>,
>>> - public ilist_sentinel_traits<NodeTy>,
>>> - public ilist_node_traits<NodeTy> {
>>> -};
>>> +template <typename NodeTy>
>>> +struct ilist_default_traits : public ilist_sentinel_traits<NodeTy>,
>>> + public ilist_node_traits<NodeTy> {};
>>>
>>> // Template traits for intrusive list. By specializing this template class, you
>>> // can change what next/prev fields are used to store the links...
>>> @@ -218,7 +260,6 @@ template <typename NodeTy>
>>> class ilist_iterator
>>> : public std::iterator<std::bidirectional_iterator_tag, NodeTy, ptrdiff_t> {
>>> public:
>>> - typedef ilist_traits<NodeTy> Traits;
>>> typedef std::iterator<std::bidirectional_iterator_tag, NodeTy, ptrdiff_t>
>>> super;
>>>
>>> @@ -279,12 +320,12 @@ public:
>>>
>>> // Increment and decrement operators...
>>> ilist_iterator &operator--() {
>>> - NodePtr = Traits::getPrev(NodePtr);
>>> + NodePtr = ilist_node_access::getPrev(NodePtr);
>>> assert(NodePtr && "--'d off the beginning of an ilist!");
>>> return *this;
>>> }
>>> ilist_iterator &operator++() {
>>> - NodePtr = Traits::getNext(NodePtr);
>>> + NodePtr = ilist_node_access::getNext(NodePtr);
>>> return *this;
>>> }
>>> ilist_iterator operator--(int) {
>>> @@ -348,8 +389,11 @@ template<typename NodeTy> struct simplif
>>> /// and the successor pointer for the sentinel (which always stays at the
>>> /// end of the list) is always null.
>>> ///
>>> -template<typename NodeTy, typename Traits=ilist_traits<NodeTy> >
>>> -class iplist : public Traits {
>>> +template <typename NodeTy, typename Traits = ilist_traits<NodeTy>>
>>> +class iplist : public Traits, ilist_node_access {
>>> + static_assert(!ilist_detail::HasGetNext<Traits, NodeTy>::value,
>>> + "ilist next and prev links are not customizable!");
>>> +
>>> mutable NodeTy *Head;
>>>
>>> // Use the prev node pointer of 'head' as the tail pointer. This is really a
>>>
>>> Modified: llvm/trunk/include/llvm/ADT/ilist_node.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist_node.h?rev=278532&r1=278531&r2=278532&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ADT/ilist_node.h (original)
>>> +++ llvm/trunk/include/llvm/ADT/ilist_node.h Fri Aug 12 12:32:34 2016
>>> @@ -36,17 +36,13 @@ protected:
>>> ilist_half_node() : Prev(nullptr) {}
>>> };
>>>
>>> -template<typename NodeTy>
>>> -struct ilist_nextprev_traits;
>>> -
>>> +struct ilist_node_access;
>>> template <typename NodeTy> class ilist_iterator;
>>>
>>> -/// ilist_node - Base class that provides next/prev services for nodes
>>> -/// that use ilist_nextprev_traits or ilist_default_traits.
>>> -///
>>> +/// Base class that provides next/prev services for ilist nodes.
>>> template<typename NodeTy>
>>> class ilist_node : private ilist_half_node<NodeTy> {
>>> - friend struct ilist_nextprev_traits<NodeTy>;
>>> + friend struct ilist_node_access;
>>> friend struct ilist_traits<NodeTy>;
>>> friend struct ilist_half_embedded_sentinel_traits<NodeTy>;
>>> friend struct ilist_embedded_sentinel_traits<NodeTy>;
>>>
>>> Modified: llvm/trunk/include/llvm/IR/SymbolTableListTraits.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/SymbolTableListTraits.h?rev=278532&r1=278531&r2=278532&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/IR/SymbolTableListTraits.h (original)
>>> +++ llvm/trunk/include/llvm/IR/SymbolTableListTraits.h Fri Aug 12 12:32:34 2016
>>> @@ -69,8 +69,7 @@ template <typename NodeTy> class SymbolT
>>> //
>>> template <typename ValueSubClass>
>>> class SymbolTableListTraits
>>> - : public ilist_nextprev_traits<ValueSubClass>,
>>> - public SymbolTableListSentinelTraits<ValueSubClass>,
>>> + : public SymbolTableListSentinelTraits<ValueSubClass>,
>>> public ilist_node_traits<ValueSubClass> {
>>> typedef SymbolTableList<ValueSubClass> ListTy;
>>> typedef
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list