On Sep 16, 2009, at 6:50 AM, Duncan Sands wrote:> Hi Bill, > >> The reason to ask this is that >>> it could make the EH tables smaller and less cluttered if we >>> elide those areas which we know don't throw (the functions called >>> are marked 'nounwind', etc.). >> Sure, that's what the SawPotentiallyThrowing boolean is for. It is >> currently set for any call, but in fact needn't be set for 'nounwind' >> calls. When I wrote this stuff 'nounwind' information wasn't >> available >> at the codegen level, which is why this isn't done. In case you >> care, >> doing this would not hurt Ada :) > > now I think about it further, I guess SawPotentiallyThrowing is not > for > this exactly, you'll need to add some additional logic near this > place. >Yeah. The logic will need tweaking for sure. I'm also concerned about the _Unwind_resume() call. GCC emits a call site region for it in the exception table. We...kind of do that. It looks like it's being included in one of the "this is a region which isn't in a try-catch block, but it has a call in it, so lets add it to the exception table" areas. If I implement my suggestion, this will likely go away. I think Eric's working on something that will add the _Unwind_resume() call during selection DAG time, where we can place EH labels around it. But it's just another thing I have to worry about when doing this. :-) -bw
Hi Bill,> Yeah. The logic will need tweaking for sure. I'm also concerned about > the _Unwind_resume() call. GCC emits a call site region for it in the > exception table. We...kind of do that. It looks like it's being included > in one of the "this is a region which isn't in a try-catch block, but it > has a call in it, so lets add it to the exception table" areas.isn't that exactly how it should be? This is a call that will unwind out of the function, so C++ requires it to have a call site region just like any other call that we want to let unwind out of the function. I don't see why it needs any special logic. If I understood right the change you want to make is that if a call is known not to unwind then you want to omit adding a call-site entry if that saves some space in the call site table, which seems irrelevant to this, or am I missing something? By the way, LLVM "nounwind" calls are different to GCC no throw regions IIRC. If an exception is thrown in a GCC no throw region then it must (C++) result in a call to "terminate". These are not mapped to "nounwind", instead we create explicit "catch-all" filter expressions for this (IIRC). In LLVM it is undefined what happens if a call is marked "nounwind" but nonetheless an exception unwinds out of it. Thus you can add call-site entries for nounwind calls, or not add them, as you like - whatever is most convenient (eg: saves the most space). An interesting optimization which we don't do is to identify which calls correspond to a "catch-all" filter and not generate an entry for them in the call-site table (no need to add the filter either) - this saves space and the C++ runtime knows to handle this just like if we added the filter explicitly. If I> implement my suggestion, this will likely go away. I think Eric's > working on something that will add the _Unwind_resume() call during > selection DAG time, where we can place EH labels around it. But it's > just another thing I have to worry about when doing this. :-)I don't see why _Unwind_resume needs special handling. That said, I'm not opposed to having "unwind" be turned into a special SDAG node which targets can handle specially. I didn't do that because it seemed awkward and not that useful (since targets can already specify whether they want to call _Unwind_Resume or something else). Ciao, Duncan.
On Thu, Sep 17, 2009 at 3:40 AM, Duncan Sands <duncan.sands at math.u-psud.fr> wrote:> Hi Bill, > >> Yeah. The logic will need tweaking for sure. I'm also concerned about the >> _Unwind_resume() call. GCC emits a call site region for it in the exception >> table. We...kind of do that. It looks like it's being included in one of the >> "this is a region which isn't in a try-catch block, but it has a call in it, >> so lets add it to the exception table" areas. > > isn't that exactly how it should be? This is a call that will unwind > out of the function, so C++ requires it to have a call site region just > like any other call that we want to let unwind out of the function. I > don't see why it needs any special logic. If I understood right the > change you want to make is that if a call is known not to unwind then > you want to omit adding a call-site entry if that saves some space in > the call site table, which seems irrelevant to this, or am I missing > something? By the way, LLVM "nounwind" calls are different to GCC no > throw regions IIRC. If an exception is thrown in a GCC no throw region > then it must (C++) result in a call to "terminate". These are not > mapped to "nounwind", instead we create explicit "catch-all" filter > expressions for this (IIRC). In LLVM it is undefined what happens if > a call is marked "nounwind" but nonetheless an exception unwinds out > of it. Thus you can add call-site entries for nounwind calls, > or not add them, as you like - whatever is most convenient (eg: saves > the most space). An interesting optimization which we don't do is to > identify which calls correspond to a "catch-all" filter and not generate > an entry for them in the call-site table (no need to add the filter > either) - this saves space and the C++ runtime knows to handle this > just like if we added the filter explicitly. >There's a miscommunication here. :-) The _Unwind_resume call isn't marked with "nounwind", however it's not called through an "invoke" instruction, only a regular "call" instruction. From what I can see, the only reason it falls within a call site in the exception table is because we're generating call sites for areas of code without "invokes". If I implement my optimization to eliminate these call site entries which don't have "invoke" calls in them, then the _Unwind_resume call won't have an entry into the exception table, and that would be bad. That was all I'm saying here. -bw