[PATCH] D76977: [lld-macho] Implement basic export trie

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.
Herald added a project: lld-macho.
Herald added a reviewer: lld-macho.


================
Comment at: lld/MachO/ExportTrie.cpp:184
+    // This is the tail-call-optimized version of the following:
+    // multikeySort(vec.slice(i, j - i), lastPos, pos + 1, node);
+    vec = vec.slice(i, j - i);
----------------
int3 wrote:
> smeenai wrote:
> > int3 wrote:
> > > int3 wrote:
> > > > smeenai wrote:
> > > > > Did you mean sortAndBuild?
> > > > > 
> > > > > Is there an easy way to either express this as a loop or trust that the compiler will do the tail call optimization? I don't mind the goto very much, but I'm wondering how it compares to the alternatives.
> > > > Oops, this was copypasta from llvm-mc's original implementation (they called it multikeySort there)
> > > > 
> > > > The TCO was also from that implementation, but I think it's a "standard" quicksort optimization too, at least for the normal two-way quicksort. Wikipedia says that it's "suggested by Sedgwick and widely used in practice": https://en.wikipedia.org/wiki/Quicksort#Optimizations
> > > It seems like there's no way to force the compiler to do TCO. There are only attributes to tell clang *not* to do TCO.
> > Yeah. LLVM has a `musttail` attribute, but I don't think that's exposed to clang, and IIRC even that doesn't guarantee a tail call. Ideally the optimizer would be able to figure this out on its own, but there's no guarantee of that.
> > 
> > Any opinions on the current `goto` structure vs. wrapping the whole thing in a `while (!vec.empty())` and adding an explicit return in the `isTerminal` case? I think that'd be equivalent, though I guess the `goto` makes the tail recursive call more apparent.
> Yeah I think `while()` here would make for more confusing code for the reason you mentioned (plus add more indentation :P)
I randomly came across this thread again. https://blog.reverberate.org/2021/04/21/musttail-efficient-interpreters.html is a fun related read :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76977/new/

https://reviews.llvm.org/D76977