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

[David Li via Phabricator via llvm-commits]

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


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:110-119
+  for (auto &BB : *F) {
+    TerminatorInst *TI = BB.getTerminator();
+    if (isa<ReturnInst>(TI)) {
+      ReturnBlock = &BB;
+      ReturnCount++;
+    }
   }
----------------
wmi wrote:
> Here we add a limitation that the function only has one return BB to be a partial inline candidate. Is it required?  Can we walk forward from entry instead of backward from the return block so as to avoid the requirement of finding single return BB?
the restriction no it is not required. Changed as suggested. 

(Note if the extracted region has other return instruction, extra code will be generated (to handle the return status in caller), so the call to the outlined function is slightly higher).


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:158-166
+  // Now walk up the chain till the function entry is reached:
+  BasicBlock *CurrEntryInChain = LastEntryInChain;
+  do {
+    Entries.push_back(CurrEntryInChain);
+    if (CurrEntryInChain == EntryBlock)
+      break;
+
----------------
wmi wrote:
> Here we allows a node on the chain with more than two successors. Will such case bring us problem later during extracting?
Changed the code to limit 2 successors. More than one successors can cause problems or lead to very inefficient outline call (to handle 'multiple entries').


https://reviews.llvm.org/D32249