[llvm] r222935 - Simplify some more ownership using forward_list<T> rather than vector<unique_ptr<T>>

Duncan P. N. Exon Smith dexonsmith at apple.com
Fri Dec 19 16:07:14 PST 2014


> On 2014-Dec-19, at 16:02, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Fri, Dec 19, 2014 at 2:22 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> > On 2014-Dec-19, at 14:11, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Wed, Dec 3, 2014 at 3:57 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > I looked into test/CodeGen/X86/compact-unwind.ll.  llvm-mc was giving
> > different results.  Given that you changed `tablegen`, I frankly gave
> > up on figuring out exactly why.
> >
> > I went back and reviewed the patch for changed semantics, and took a
> > shot in the dark.  The `sort()` at the end is different from the
> > old `sort()`, because items have been front-loaded instead of back-
> > loaded.
> >
> > The fix is to sort it backwards and then reverse the list.  This passes
> > `check` for me.
> >
> > Any preferences on this approach, versus using std::list (paying an extra pointer per element) and pushing onto the end instead? (hmm, or maybe we can keep an iterator to the tail end of the list and just use that as an insert hint while still using std::forward_list...  - not sure if that's worth it)
> 
> I'm not sure which approach makes sense here.  How memory constrained
> does `tablegen` get?  `std::list<>` seems easiest if the extra pointer
> doesn't matter.
> 
> Welp... figured out why I wasn't seeing those failures you saw. std::forward_list<>::sort in libstdc++ (even in 4.9) is unstable (& in fact exactly inverted - which was exactly what was required). So your fix actually caused exactly the same failures for me as you'd seen from my original patch...  
> 
> In any case, that means we can't rely on its behavior in LLVM and I'll just use std::list<> instead. Lame.
>  

That's a shame :(.

> 
> >
> > Also - did you happen to verify that your change causes the diffs on the AsmMatcher.inc files you mentioned to go away entirely, or were there other remaining changes I should hunt for/verify?
> >
> 
> There were some other minor diffs that might be worth looking at.
> 
> IIRC, PR21735 describes them.
> 
> > I don't know why this is platform-specific... I can't think of why
> > that makes sense.
> >
> > Yeah, that's still rather confusing.
> >
> 





More information about the llvm-commits mailing list