[PATCH] D32249: [PartialInl] Enhance partial inliner to handle more complex conditions

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

Some minor comments.



================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:68
+  // The set of blocks in Entries that that are predecessors to NonReturnBlock
+  SmallVector<BasicBlock *, 4> NonReturnBlockPreds;
+};
----------------
I find NonReturnBlockPreds has been set but not used. 


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:136
+      return std::make_tuple(Succ1, Succ2);
+    else if (IsReturnBlock(Succ2))
+      return std::make_tuple(Succ2, Succ1);
----------------
we can change the else if to if.


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:222-223
+
+  if (!CheckAndNormalizeCandidate(OutliningInfo.get()))
+    return std::unique_ptr<FunctionOutliningInfo>();
+
----------------
should we use an assertion here instead of an early exit, so we can catch mistake instead of giving up silently.


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:340-343
   ToExtract.push_back(NewNonReturnBlock);
   for (BasicBlock &BB : *DuplicateFunction)
-    if (&BB != NewEntryBlock && &BB != NewReturnBlock &&
-        &BB != NewNonReturnBlock)
+    if (!ToBeInlined(&BB) && &BB != NewNonReturnBlock)
       ToExtract.push_back(&BB);
----------------
ToBeInlined(NewNonReturnBlock) is also false, so can we just do:
  for (BasicBlock &BB : *DuplicateFunction)
    if (!ToBeInlined(&BB))
      ToExtract.push_back(&BB);


https://reviews.llvm.org/D32249