[PATCH] D14094: Add multikey_qsort to STLExtras.h

[Jim Grosbach via llvm-commits]

grosbach added inline comments.

================
Comment at: include/llvm/ADT/STLExtras.h:376
@@ +375,3 @@
+    ++Pos;
+    goto tailcall;
+  }
----------------
ruiu wrote:
> dblaikie wrote:
> > grosbach wrote:
> > > ruiu wrote:
> > > > grosbach wrote:
> > > > > Does the compiler not auto-recognize the tail call pattern here? If not, it seems like it should.
> > > > Unlike Scheme we cannot assume that the tail call optimization is always in place. If a compiler does tail call optimization only in -O2 and not in -O0, this code could cause SEGV only in -O0 for some input. That would be confusing. So I think this is needed.
> > > I'm not sure I agree. -O0 vs. -O2 (or -Os) builds of the compiler (and Debug vs. Release in general) already have radically different memory usage patterns and algorithmic behavior (various +asserts stuff has n^2 algorithms, e.g.).
> > > 
> > > Is this a theoretical concern, or are you seeing crashes on real examples with a debug build of the compiler?
> > Eh, I think we tend not to write recursive algorithms where there aren't nice/known/low bounds, even if the compiler can optimize them away (same reason we write ++i instead of i++, etc).
> > 
> > That said, I wouldn't write it with goto if I could help it - I'd rather find a more natural iterative expression of the issue. (even just a simple "while (true) / return" thing I'd prefer over a raw goto, but a do/while loop seems plausible here)
> test/MC/COFF/section-name-encoding.s fails if this is not tail-optimized. But as long as it is tail-optimized, everything should be fine. So this is kind of theoretical.
That's fair. To be clear, it's the "goto" I'm having trouble swallowing. A do/while would be totally reasonable.


http://reviews.llvm.org/D14094