[PATCH] D24315: [CGP] Be less conservative about tail-duplicating a ret to allow tail calls

[Kyle Butt via llvm-commits]

iteratee added inline comments.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2008
@@ -2013,2 +2007,3 @@
   SmallVector<CallInst*, 4> TailCalls;
+  bool AllowDifferingSizes;
   if (PN) {
----------------
mkuper wrote:
> iteratee wrote:
> > mkuper wrote:
> > > iteratee wrote:
> > > > It's kind of awkward to pass this as an output parameter and not use it.
> > > > Would it be better to make it a pointer argument with a default value of nullptr?
> > > I agree it's awkward, I'm just not sure the other way around (having a temporary in the callee, and then setting after a null check) is less awkward, overall. Same goes for other possible solutions (returning a pair, say).
> > > 
> > > In any case, I don't feel strongly about this, and I don't think the LLVM coding standards say anything concrete about this. So if you significantly prefer a pointer, let me know, and I'll change it.
> > How about this:
> > Use a local variable in attributesPermitTailCall.
> > Use make_scope_exit to set the output parameter on exit only if it isn't null.
> That'll work too (I didn't even know we had make_scope_exit!), but it seems like overkill.
> 
> Another awkward option that doesn't involve make_scope_exit, as long as we're at it:
> 
> ```
> ... bool *ADS = nullptr) {
>   bool TempADS;
>   bool &AllowDifferingSizes = ADS ? *ADS : TempADS;
> ...
> }
> ```
Go with that. That's the least awkward of all of them.


https://reviews.llvm.org/D24315