[PATCH] D14552: Teach the inliner to track deoptimization state

[Philip Reames via llvm-commits]

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

LGTM w/comments addressed.  I don't think this needs another round of review.


================
Comment at: include/llvm/IR/Instructions.h:1463
@@ +1462,3 @@
+
+  /// \brief Create a clone of \p CI with a different set of operand bundles and
+  /// insert it before CI.
----------------
The difference in insertion logic here is confusing and different from every other create function.  Please stick to convention.

================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:386
@@ +385,3 @@
+      if (CS.hasOperandBundles())
+        CodeInfo->OperandBundleCallSites.push_back(NewInst);
+
----------------
CodeInfo might be null, please guard here and other as well.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1171
@@ +1170,3 @@
+
+        OpDefs.clear();
+        CallSite ICS(I);
----------------
minor: reserve final size

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1175
@@ +1174,3 @@
+          auto ChildOB = ICS.getOperandBundleAt(i);
+          if (ChildOB.getTagID() != LLVMContext::OB_deopt) {
+            // If the inlined call has other operand bundles, let them be
----------------
Minor:
if (is_deopt) {}
else { }

might be clearer than if !deopt, continue

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1185
@@ +1184,3 @@
+          // inlined call's deoptimization continuation.
+          std::vector<Value *> MergedDeoptArgs(ParentDeopt.Inputs.begin(),
+                                               ParentDeopt.Inputs.end());
----------------
minor: reserve the final size


http://reviews.llvm.org/D14552