[PATCH] D15289: Avoid inlining CallSites leading to unreachable

[hfinkel@anl.gov via llvm-commits]

hfinkel added a comment.

> Do you want to land this change first. And then bring BPI object here as a separate patch?


We can do that; the test cases will be good to have regardless.


================
Comment at: lib/Transforms/IPO/Inliner.cpp:558
@@ +557,3 @@
+    // with exit().
+    if (HasCallSites && HasUnreachableTerminatedBlock && F->hasName() &&
+        F->getName() != "main")
----------------
junbuml wrote:
> Gerolf wrote:
> > That is a bit too hacky. :-) There could be exception paths even in main where you want your optimization to fire. There needs to be a way to distinguish between "hot" and "cold" unreachable blocks, eg. if it is cold if is part of EH handling or if it is an exit returning an error code etc. And that heuristic should be applicable eg. for BranchProbabilityInfo::calcUnreachableHeuristics also eventually. I suggest a FIXME for now instead of checking for a specific function name.
> For me, checking function names for main() doesn't seem uncommon. Please let me know if there is any better way.
> 
> I agree that even in main we can distinguish real cold blocks, but I didn't want to make this patch bigger. I think a separate patch only for that could lead the review process much easier. So as you suggested, I want to add FIXME for now if reviewers are okay with it.
A FIXME is fine for now (so long as we address it in the near future).


http://reviews.llvm.org/D15289