[llvm] r278532 - ADT: Remove the ilist_nextprev_traits customization point

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 12 14:48:42 PDT 2016


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 :-/


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