[PATCH] D17732: Introduce @llvm.experimental.deoptimize

[Philip Reames via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

In http://reviews.llvm.org/D17732#371098, @sanjoy wrote:

> Re: doc comments -- I think did address all of them, can you point out the ones that you are not okay with in the current form?


You're correct.  For some reason the doc changes didn't show up in the diff between versions (or I just miss them).  All of the doc comments have been addressed satisfactorily.

With the current comments addressed, LGTM.


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1829
@@ +1828,3 @@
+
+        Value *CallArg = *DeoptCall->arg_begin();
+        assert((DeoptCall->arg_end() - DeoptCall->arg_begin()) == 1 && "Only one call argument allowed");
----------------
sanjoy wrote:
> reames wrote:
> > I'd intended in my previous comment to indicate that the intrinsic should take an unlimited number of untyped arguments.  Not one untyped argument.  
> I thought about that -- having a varargs signature is not more general (since you can just create an aggregate type / alloca and pass that in), and did not feel like it was worth the complexity (e.g. we'd probably not want the call to be a varargs call at the ABI level).
Hm, I disagree with you, but let's separate this out.  For this review, let's use a form with no arguments.  We can extend it later to support the multiple argument form if desired.  That can be done as a follow up change.  

(I specifically don't want the one argument form going in because it *requires* one argument when a completely legitimate runtime might not need any.)


http://reviews.llvm.org/D17732