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

[Philip Reames via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

My previous documentation comments have not been addressed.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5447
@@ -5446,1 +5446,3 @@
+  case Intrinsic::experimental_deoptimize:
+    return "__llvm_deoptimize";
   }
----------------
possibly we should have experimental in the name?  
__llvm_experimental_deoptimize?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1814
@@ +1813,3 @@
+      auto NewEnd = remove_if(Returns, [](ReturnInst *RI) {
+        return RI->getParent()->getTerminatingDeoptimizeCall();
+      });
----------------
minor: the implicit bool conversion here is confusing.  nullptr != X would be clearer.  Alternatively, have the lambda explicitly return bool.

================
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");
----------------
I'd intended in my previous comment to indicate that the intrinsic should take an unlimited number of untyped arguments.  Not one untyped argument.  


http://reviews.llvm.org/D17732