[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 16:05:17 PDT 2016
Can you give me the backtraces? I strongly suspect a similar problem, but I can't investigate on Darwin without knowing where it crashes. (Perhaps r278572 will be sufficient...)
> On 2016-Aug-12, at 15:54, Yung, Douglas <douglas.yung at sony.com> wrote:
>
> Hi Duncan,
>
> I just wanted to add that I'm seeing 13 failures in the regression tests on the Windows PS4 bot (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast) that I suspect are due to this change. The problem only seems to exist for us when building on Windows, so that might be why Hans has not been able to reproduce it on linux. To narrow it down, I synced up to r278532, saw the build failure, and then patched it with the subsequent fixes, r278537 and r278539 only. Once I did that, I saw the following 13 test failures:
>
> Failing Tests (13):
> LLVM :: CodeGen/X86/2010-02-23-RematImplicitSubreg.ll
> LLVM :: CodeGen/X86/2011-03-08-Sched-crash.ll
> LLVM :: CodeGen/X86/2012-08-17-legalizer-crash.ll
> LLVM :: CodeGen/X86/avx-splat.ll
> LLVM :: CodeGen/X86/compare-inf.ll
> LLVM :: CodeGen/X86/copy-eflags.ll
> LLVM :: CodeGen/X86/fast-isel-i1.ll
> LLVM :: CodeGen/X86/machine-cse.ll
> LLVM :: CodeGen/X86/no-and8ri8.ll
> LLVM :: CodeGen/X86/shrink-wrap-chkstk.ll
> LLVM :: CodeGen/X86/x86-shrink-wrap-unwind.ll
> LLVM :: CodeGen/X86/x86-shrink-wrapping.ll
> LLVM :: Transforms/CodeExtractor/X86/InheritTargetAttributes.ll
>
> It's not immediately obvious to me why your change is causing these failures, but this (as well as another failure) is causing the PS4 Windows bot to be red at the moment. Can you take a look into this?
>
> Douglas Yung
>
>> -----Original Message-----
>> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
>> Behalf Of Hans Wennborg via llvm-commits
>> Sent: Friday, August 12, 2016 14:49
>> To: Duncan P. N. Exon Smith
>> Cc: llvm-commits
>> Subject: Re: [llvm] r278532 - ADT: Remove the ilist_nextprev_traits
>> customization point
>>
>> 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/SymbolT
>>> ableListTraits.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
>> _______________________________________________
>> 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