[llvm] r278532 - ADT: Remove the ilist_nextprev_traits customization point
Yung, Douglas via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 12 17:12:39 PDT 2016
Hi Duncan,
Here is a stack trace from the failure of CodeGen\X86\fast-isel-i1.ll.
Command: llc.exe fast-isel-i1.ll -mtriple=x86_64-apple-darwin10 -fast-isel -fast-isel-abort=1
In the Visual Studio debugger, I get "Unhandled exception at 0x000000013FF1708F in llc.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF."
The stack trace at the point of failure is:
> llc.exe!llvm::MachineInstr::getOpcode() Line 286 C++
llc.exe!llvm::X86FrameLowering::emitEpilogue(llvm::MachineFunction & MF, llvm::MachineBasicBlock & MBB) Line 1483 C++
llc.exe!`anonymous namespace'::PEI::insertPrologEpilogCode(llvm::MachineFunction & Fn) Line 977 C++
llc.exe!`anonymous namespace'::PEI::runOnMachineFunction(llvm::MachineFunction & Fn) Line 211 C++
llc.exe!llvm::MachineFunctionPass::runOnFunction(llvm::Function & F) Line 60 C++
llc.exe!llvm::FPPassManager::runOnFunction(llvm::Function & F) Line 1522 C++
llc.exe!llvm::FPPassManager::runOnModule(llvm::Module & M) Line 1543 C++
llc.exe!`anonymous namespace'::MPPassManager::runOnModule(llvm::Module & M) Line 1599 C++
llc.exe!llvm::legacy::PassManagerImpl::run(llvm::Module & M) Line 1702 C++
llc.exe!llvm::legacy::PassManager::run(llvm::Module & M) Line 1734 C++
llc.exe!compileModule(char * * argv, llvm::LLVMContext & Context) Line 507 C++
llc.exe!main(int argc, char * * argv) Line 273 C++
llc.exe!__tmainCRTStartup() Line 626 C
llc.exe!mainCRTStartup() Line 466 C
kernel32.dll!0000000076f259bd() Unknown
ntdll.dll!000000007705a2e1() Unknown
The failure is here:
/// Returns the opcode of this MachineInstr.
unsigned getOpcode() const { return MCID->Opcode; } <== MCID has a garbage value
Hope this helps, and I can generate the other stack traces as well if it helps, just let me know.
Douglas Yung
> -----Original Message-----
> From: hwennborg at google.com [mailto:hwennborg at google.com] On Behalf Of
> Hans Wennborg
> Sent: Friday, August 12, 2016 16:45
> To: Duncan P. N. Exon Smith
> Cc: Yung, Douglas; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r278532 - ADT: Remove the ilist_nextprev_traits
> customization point
>
> On Fri, Aug 12, 2016 at 4:15 PM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
> >
> >> 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!).
>
> I committed r278577 for another instance of this. Halfway through a
> Chromium build now :-)
>
>
> >>> -----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