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

[David Li via Phabricator via llvm-commits]

davidxl marked 9 inline comments as done.
davidxl added inline comments.


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:43
+    "max-num-inline-blocks", cl::init(5), cl::Hidden,
+    cl::desc("Max Number of Blocks  To be Partialled Inined"));
+
----------------
davide wrote:
> typo, `partially inlined`. BTW, do you have numbers for different values of `MaxNumInlineBlocks` ?
Currently it is set  to a high number  for stress testing purpose.  When partial inliner is enabled in the pipeline, the default value needs to be tuned depending on opt level.


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:120-131
+  auto IsSuccessor = [](BasicBlock *Succ, BasicBlock *BB) {
+    return is_contained(successors(BB), Succ);
+  };
+
+  auto SuccSize = [](BasicBlock *BB) {
+    return std::distance(succ_begin(BB), succ_end(BB));
+  };
----------------
davide wrote:
> These could probably live in `BasicBlockUtils`.
will leave it here for now.


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:134-139
+    if (IsReturnBlock(Succ1))
+      return std::make_tuple(Succ1, Succ2);
+    else if (IsReturnBlock(Succ2))
+      return std::make_tuple(Succ2, Succ1);
+    else
+      return std::make_tuple<BasicBlock *, BasicBlock *>(nullptr, nullptr);
----------------
davide wrote:
> Also this one. Please remove `else` after `return` 
removed else.


https://reviews.llvm.org/D32249