<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 17, 2015 at 11:26 AM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2015 Jun 16, at 21:40, Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>> wrote:<br>
><br>
> Hi Duncan,<br>
><br>
> Singly linked lists are used throught LLVM: ArrayRecycler, Registry, LiveInterval, SCEVUnknown, CGBlockInfo, MacroArgCache, ...DIEValueList.<br>
><br>
> What do you think about implementing a singly-linked ADT?<br>
><br>
<br>
</span>Interesting idea.<br>
<br>
I've taken a look at the other singly-linked lists you mentioned.  Most<br>
of these are some version of a free/recycling list (ArrayRecycler,<br>
Registry, MacroArgCache, etc.), which need super simple push/pop API and<br>
no memory management.<br>
<br>
LiveInterval and CGBlock also need iteration and a way to conditionally<br>
remove elements (something like `remove_if()`), and in a few cases it<br>
would be convenient to have API to destruct and/or destroy nodes.<br>
<br>
I think these would all be straightforward to formalize into an ADT.  We<br>
could even use it to implement a value-based `llvm::forward_list<>` that<br>
avoided the sorting bug in libstdc++ (which gets "stable" exactly<br>
backwards) that David hit when refactoring tablegen.<br>
<br>
Unfortunately, I think it'd be hard to reuse for DIE::Children and<br>
DIEValueList.  The latter wants to preserve insertion order and needs<br>
`push_back()` API (the former can technically get away with a timely<br>
call to `reverse()`, but it's awkward).  Supporting `push_back()`<br>
efficiently requires pointing at the last element somehow, either via an<br>
extra pointer or circular logic (what my latest patch does for<br>
DIE::Children).<br></blockquote><div><br></div><div>One of the other problems I hit when trying to value-ify the DIE children is that the point of DIE creation is, in some particular cases, divorced from the knowledge of which container it's going to go in to. Yet pointer identity is needed before then. <br><br>At least that's how I recall it - may be worth going down that path again just so we can write it up clearly for posterity, I dunno.<br><br>If it is possible, then I would love/prefer to use an existing non-intrusive container (or writing one if we have to). at least could prototype with std::list, then, as you say, implement a std::forward_list-like device with the bonus that we could fix sort and do the circular trick to allow insertion at the end, etc.<br><br>(& sorry, will get to proper review of this patch soon... )</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
We could configure this stuff via templates -- I'd be open to the idea<br>
-- but I think the intersection between the implementations would be<br>
practically nil.  Even the iterator implementations need to be<br>
completely different.  If these are the only `push_back()`-enabled<br>
slists in tree, is it premature to abstract it?  Would we even want the<br>
same name as "normal" slists?<br>
<br>
(Regardless, I think it would be a great idea for someone to ADT-ify the<br>
other linked lists!)<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> 2015-06-05 3:24 GMT+03:00 Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>>:<br>
><br>
> > On 2015 May 28, at 14:14, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> ><br>
> ><br>
> >> On 2015-May-26, at 15:30, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> >><br>
> >> David, let me know if you have any comments on the `DIE::Children`<br>
> >> stuff (patch 0008 in particular, although I think Eric wanted your<br>
> >> opinion on 0007 as well).<br>
> >><br>
> >>> On 2015-May-26, at 14:41, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
> >>> 0007 - LGTM, might want to have Dave take a look at it as well.<br>
> >>> 0008 - reverseChildren and finalizeChildren is pretty gross. Also, I'm going to punt on the rest of this to Dave.<br>
> ><br>
> > I've committed all but these two.  I've attached rebased versions<br>
> > that incorporate (most of) Fred's feedback.<br>
> ><br>
> > David, I found a way to avoid the reverseChildren/finalizeChildren<br>
> > thing without paying for an extra pointer, but it means that<br>
> > `DIE` would no longer use `std::unique_ptr<>` internally.  (High<br>
> > level: change the sibling list to a circular linked list, point at<br>
> > the last child instead of the first, and store a "this-is-last"<br>
> > marker on the last child to support iteration.)<br>
> ><br>
> > stop-reversing-children.patch implements this on top of 0008;<br>
> > stop-reversing-children-with-0008.patch has that patch and 0008<br>
> > merged together.  I think this might be the better way to do it,<br>
> > but I'm open to other ideas.<br>
> ><br>
> > If you're okay with this approach, maybe I'll rewrite 0007 to<br>
> > save an extra pointer there as well?<br>
><br>
> Ping.  Patches reattached.<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>