[PATCH] D12979: Avoid inlining in exception handling context

[Jun Bum Lim via llvm-commits]

junbuml added inline comments.

================
Comment at: lib/Transforms/IPO/PruneEH.cpp:362
@@ +361,3 @@
+  // call void @__cxa_throw(i8* %exception, .. )
+  for (User *FnU : FnThrowE->users()) {
+    auto *ThrowCall = dyn_cast<CallInst>(FnU);
----------------
hfinkel wrote:
> junbuml wrote:
> > hfinkel wrote:
> > > junbuml wrote:
> > > > hfinkel wrote:
> > > > > Why are you looking for users of __cxa_throw? Can we make the assumption that all user of __cxa_allocate_exception are essentially cold, and just look for the users of return values of that function?
> > > > > 
> > > > I'm trying to find all actual calls for __cxa_throw() by looking at the users of FnThrowE where an exception is thrown. 
> > > > 
> > > > Here, I specifically check the exception allocated to be thrown (__cxa_throw()) in throw statement, which should be cold unless the program logic highly rely on throw statement.
> > > > 
> > > > When a exception handling class is created in throw statement, 
> > > > the constructor must take the exception used in __cxa_throw() as its first parameter.
> > > > 
> > > > Please correct me if I miss something.
> > > > 
> > > I understand what you're doing, but my question is this: Is the call to __cxa_allocate_exception itself cold? If so, perhaps that's all we need to find. Maybe looking for the __cxa_throw calls is an unnecessary complication.
> > > 
> > I believe __cxa_allocate_exception() itself must be cold in almost all normal c++ program. But, I just wanted to make sure the exception allocated is actually used for throwing. If checking __cxa_throw() doesn't make it too complicated, I believe there is no harm with it. I can also leave FIXME about it.  Please let me know your thought!
> > I believe cxa_allocate_exception() itself must be cold in almost all normal c++ program. But, I just wanted to make sure the exception allocated is actually used for throwing. If checking cxa_throw() doesn't make it too complicated, I believe there is no harm with it. I can also leave FIXME about it. Please let me know your thought!
> 
> Okay.
> 
> Let me also ask something I should have asked in the very beginning: Why are you doing this here and not in Clang? It seems that, in Clang, it would be simple to have a piece of state that records whether we're in a throw statement or a catch block, and marks the call sites appropriately.
I just wanted to place it somewhere exception handling specific pass, instead of directly touching inliner. If you believe Clang (ItaniumCXXABI.cpp ?) is better place I can do the same thing in there. 
 


http://reviews.llvm.org/D12979