[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:15:27 PDT 2016
> On 2016-Aug-12, at 16:12, Yung, Douglas <douglas.yung at sony.com> wrote:
>
> Hi Duncan,
>
> Your change in r278572 seems to have helped, the number of failures is down to 8. (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/9911).
Yes, I had just seen that myself and was about to comment!
> (The sancov failure is unrelated to your change) I'm still building a debug build of the compiler and will send a callstack for the failure when I have it.
Thanks very much. Let me know if you need me to revert r278532-42 while we investigate (happy to revert as long as you can send me the backtraces!).
> Douglas Yung
>
> PS - Sorry, I did miss the rest of the thread, I didn't see it because my email client had not indexed the emails yet so they did not appear when I was searching. Sorry about that!
No problem :).
>
>> -----Original Message-----
>> From: dexonsmith at apple.com [mailto:dexonsmith at apple.com]
>> Sent: Friday, August 12, 2016 16:05
>> To: Yung, Douglas
>> Cc: Hans Wennborg; llvm-commits at lists.llvm.org
>> Subject: Re: [llvm] r278532 - ADT: Remove the ilist_nextprev_traits
>> customization point
>>
>> 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