* [rfc] exception handling in gimple @ 2009-09-03 1:23 Richard Henderson 2009-09-08 14:34 ` Michael Matz 2009-09-08 15:16 ` Michael Matz 0 siblings, 2 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-03 1:23 UTC (permalink / raw) To: GCC Patches [-- Attachment #1: Type: text/plain, Size: 649 bytes --] I've yet to write the ChangeLog entry, and I'm just re-running the build test cycle after resolving some merge errors with Matz's last gimple expanson patch, but I think this patch is pretty close to done. The previous test run, with vta merge included, had no C++ regressions, 2 Ada failures that I don't believe were regressions, and 2 Java failures that I didn't get around to examining. I've tested sjlj support as well; there are 10 Ada regressions wrt dwarf2 that need to be examined, but there were no C++ regressions, so it's in fairly good shape. So this is well beyond the WIP stage, and I'd like to get some real feedback on it. r~ [-- Attachment #2: d-eh-6.gz --] [-- Type: application/x-gzip, Size: 98954 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-03 1:23 [rfc] exception handling in gimple Richard Henderson @ 2009-09-08 14:34 ` Michael Matz 2009-09-08 16:25 ` Richard Henderson 2009-09-08 15:16 ` Michael Matz 1 sibling, 1 reply; 31+ messages in thread From: Michael Matz @ 2009-09-08 14:34 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Wed, 2 Sep 2009, Richard Henderson wrote: > So this is well beyond the WIP stage, and I'd like to get some real > feedback on it. I haven't read nearly all of it yet, but one thing I noticed. You introduce some new predicates (which is wonderful :) ), and use them in some places where it's not immediately clear they're equivalent: --- a/gcc/dce.c +++ b/gcc/dce.c @@ -81,10 +81,8 @@ deletable_insn_p_1 (rtx body) default: if (volatile_refs_p (body)) return false; - - if (flag_non_call_exceptions && may_trap_p (body)) + if (insn_could_throw_p (body)) return false; Or e.g.: --- a/gcc/dse.c +++ b/gcc/dse.c @@ -2531,7 +2531,7 @@ scan_insn (bb_info_t bb_info, rtx insn) them. */ if ((GET_CODE (PATTERN (insn)) == CLOBBER) || volatile_refs_p (PATTERN (insn)) - || (flag_non_call_exceptions && may_trap_p (PATTERN (insn))) + || insn_could_throw_p (insn) (at least cse.c has a similar hunk) Your new insn_could_throw_p() always returns true for calls, no matter if they possibly are marked as nothrow, or are pure/const. I think this pessimizes the above hunks (though I haven't checked the surroundings). Further, insn_could_throw_p() takes a full instruction, not a pattern, but at least in the first hunk you have the pattern already. A general remark: The exception machinery and how it's implemented always sorely lacked high level internal documentation. E.g. what exactly is in struct eh_landing_pad_d.post_landing_pad vs .landing_pad, and similar, how are the high level language constructs implemented with the different eh-tree nodes (what's e.g. EH_FILTER and how does it relate to try/catch/throw, and so on), . I somewhat hoped an exception rewrite would add something enlightening from the author of also the older exception stuff so I can wrap my little mind around it more easily. In the past I always figured out what is supposed to be doing what, but I forget after a few months and always have to relearn the ugly details by browsing sources instead of comments :) Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-08 14:34 ` Michael Matz @ 2009-09-08 16:25 ` Richard Henderson 2009-09-09 0:07 ` Richard Henderson 0 siblings, 1 reply; 31+ messages in thread From: Richard Henderson @ 2009-09-08 16:25 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/08/2009 07:33 AM, Michael Matz wrote: > I haven't read nearly all of it yet, but one thing I noticed. You > introduce some new predicates (which is wonderful :) ), and use them in > some places where it's not immediately clear they're equivalent: > > --- a/gcc/dce.c > +++ b/gcc/dce.c > @@ -81,10 +81,8 @@ deletable_insn_p_1 (rtx body) > default: > if (volatile_refs_p (body)) > return false; > - > - if (flag_non_call_exceptions&& may_trap_p (body)) > + if (insn_could_throw_p (body)) > return false; You're right that this one isn't equivalent; I had already found that bug and moved the insn_could_throw_p check to deleteable_insn_p. > - || (flag_non_call_exceptions&& may_trap_p (PATTERN (insn))) > + || insn_could_throw_p (insn) > > (at least cse.c has a similar hunk) > > Your new insn_could_throw_p() always returns true for calls, no matter if > they possibly are marked as nothrow, or are pure/const. I think this > pessimizes the above hunks (though I haven't checked the surroundings). > Further, insn_could_throw_p() takes a full instruction, not a pattern, but > at least in the first hunk you have the pattern already. Yes, insn_could_throw_p, like stmt_could_throw_p, is supposed to disregard the on-the-side EH information (in this case REG_EH_REGION) so that we can tell if the on-the-side EH information is necessary. You're probably right that DSE could use the insn_nothrow_p predicate instead. However, I wanted to make the transformations as close to direct as possible. In some instances, like CSE, the pass isn't prepared to deal with REG_EH_REGION notes at all. So the fact that a call is const, and therefore nothrow, doesn't really help since there's still a REG_EH_REGION 0 note to deal with. > A general remark: The exception machinery and how it's implemented always > sorely lacked high level internal documentation. E.g. what exactly is in > struct eh_landing_pad_d.post_landing_pad vs .landing_pad, and similar, how > are the high level language constructs implemented with the different > eh-tree nodes (what's e.g. EH_FILTER and how does it relate to > try/catch/throw, and so on), . I somewhat hoped an exception rewrite > would add something enlightening from the author of also the older > exception stuff so I can wrap my little mind around it more easily. In > the past I always figured out what is supposed to be doing what, but I > forget after a few months and always have to relearn the ugly details by > browsing sources instead of comments :) I did add a page or two of documentation about the EH lowering process and the 99 passes that seem to be involved. But I can certainly add more that describe the data structures. I'll finish up some varray to vec conversion I'm in the middle of, add those docs, and get a revised patch out tonight. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-08 16:25 ` Richard Henderson @ 2009-09-09 0:07 ` Richard Henderson 2009-09-09 8:07 ` Olivier Hainque ` (5 more replies) 0 siblings, 6 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-09 0:07 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 208 bytes --] Still writing the changelog, but here's the version that's current in my tree as of this afternoon. Notable changes from the last version are: documentation added, varrays removed, and some bugs fixed. r~ [-- Attachment #2: d-eh-8.gz --] [-- Type: application/x-gzip, Size: 106992 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 0:07 ` Richard Henderson @ 2009-09-09 8:07 ` Olivier Hainque 2009-09-09 10:36 ` Michael Matz 2009-09-09 8:43 ` Eric Botcazou ` (4 subsequent siblings) 5 siblings, 1 reply; 31+ messages in thread From: Olivier Hainque @ 2009-09-09 8:07 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches, hainque Richard Henderson wrote: > Still writing the changelog, but here's the version that's > current in my tree as of this afternoon. Very nice to see this area all refreshed :) Documentation-wise, FWIW, we have had a blurb about the runtime tables organization for while in ada/raise-gcc.c, quoted below. We found it very useful on several occasions to help understanding the personality routines implementations. The intent for this piece was to convey the general structure, which in principle belongs to a more common place. There are only tiny bits of Ada specific info there, easy to adjust if need be. This was written a long time ago, from our understanding of the organization at that time, so there might be a few inaccuracies. This would easier to maintain and by the most knowledgable people if it were in a more common place. Anyway, just to let you know that we'd be happy to see this info moved/updated to a more common place if you think it would be useful. Your current EH circuitry rewrite sounds like a nice possible opportunity for this. Olivier -- /* There are three major runtime tables involved, generated by the GCC back-end. Contents slightly vary depending on the underlying implementation scheme (dwarf zero cost / sjlj). ======================================= * Tables for the dwarf zero cost case * ======================================= call_site [] ------------------------------------------------------------------- * region-start | region-length | landing-pad | first-action-index * ------------------------------------------------------------------- Identify possible actions to be taken and where to resume control for that when an exception propagates through a pc inside the region delimited by start and length. A null landing-pad indicates that nothing is to be done. Otherwise, first-action-index provides an entry into the action[] table which heads a list of possible actions to be taken (see below). If it is determined that indeed an action should be taken, that is, if one action filter matches the exception being propagated, then control should be transfered to landing-pad. A null first-action-index indicates that there are only cleanups to run there. action [] ------------------------------- * action-filter | next-action * ------------------------------- This table contains lists (called action chains) of possible actions associated with call-site entries described in the call-site [] table. There is at most one action list per call-site entry. A null action-filter indicates a cleanup. Non null action-filters provide an index into the ttypes [] table (see below), from which information may be retrieved to check if it matches the exception being propagated. action-filter > 0 means there is a regular handler to be run, action-filter < 0 means there is a some "exception_specification" data to retrieve, which is only relevant for C++ and should never show up for Ada. next-action indexes the next entry in the list. 0 indicates there is no other entry. ttypes [] --------------- * ttype-value * --------------- A null value indicates a catch-all handler in C++, and an "others" handler in Ada. Non null values are used to match the exception being propagated: In C++ this is a pointer to some rtti data, while in Ada this is an exception id. The special id value 1 indicates an "all_others" handler. For C++, this table is actually also used to store "exception specification" data. The differentiation between the two kinds of entries is made by the sign of the associated action filter, which translates into positive or negative offsets from the so called base of the table: Exception Specification data is stored at positive offsets from the ttypes table base, which Exception Type data is stored at negative offsets: --------------------------------------------------------------------------- Here is a quick summary of the tables organization: +-- Unwind_Context (pc, ...) | |(pc) | | CALL-SITE[] | | +=============================================================+ | | region-start + length | landing-pad | first-action-index | | +=============================================================+ +-> | pc range 0 => no-action 0 => cleanups only | | !0 => jump @ N --+ | +====================================================== | ====+ | | ACTION [] | | +==========================================================+ | | action-filter | next-action | | +==========================================================+ | | 0 => cleanup | | | >0 => ttype index for handler ------+ 0 => end of chain | <-+ | <0 => ttype index for spec data | | +==================================== | ===================+ | | TTYPES [] | | Offset negated from +=====================+ | the actual base. | ttype-value | | +============+=====================+ | | | 0 => "others" | | | ... | 1 => "all others" | <---+ | | X => exception id | | handlers +---------------------+ | | ... | | ... | ... | | | ... | +============+=====================+ <<------ Table base | ... | ... | | specs | ... | (should not see negative filter | ... | ... | values for Ada). +============+=====================+ ============================ * Tables for the sjlj case * ============================ So called "function contexts" are pushed on a context stack by calls to _Unwind_SjLj_Register on function entry, and popped off at exit points by calls to _Unwind_SjLj_Unregister. The current call_site for a function is updated in the function context as the function's code runs along. The generic unwinding engine in _Unwind_RaiseException walks the function context stack and not the actual call chain. The ACTION and TTYPES tables remain unchanged, which allows to search them during the propagation phase to determine whether or not the propagated exception is handled somewhere. When it is, we only "jump" up once directly to the context where the handler will be found. Besides, this allows "break exception unhandled" to work also The CALL-SITE table is setup differently, though: the pc attached to the unwind context is a direct index into the table, so the entries in this table do not hold region bounds any more. A special index (-1) is used to indicate that no action is possibly connected with the context at hand, so null landing pads cannot appear in the table. Additionally, landing pad values in the table do not represent code address to jump at, but so called "dispatch" indices used by a common landing pad for the function to switch to the appropriate post-landing-pad. +-- Unwind_Context (pc, ...) | | pc = call-site index | 0 => terminate (should not see this for Ada) | -1 => no-action | | CALL-SITE[] | | +=====================================+ | | landing-pad | first-action-index | | +=====================================+ +-> | 0 => cleanups only | | dispatch index N | +=====================================+ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 8:07 ` Olivier Hainque @ 2009-09-09 10:36 ` Michael Matz 0 siblings, 0 replies; 31+ messages in thread From: Michael Matz @ 2009-09-09 10:36 UTC (permalink / raw) To: Olivier Hainque; +Cc: Richard Henderson, GCC Patches Hi, On Wed, 9 Sep 2009, Olivier Hainque wrote: > Documentation-wise, FWIW, we have had a blurb about the runtime tables > organization for while in ada/raise-gcc.c, quoted below. We found it > very useful on several occasions to help understanding the personality > routines implementations. Yes, I found it also very useful in the past to understand what the LSDA data actually contains :) Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 0:07 ` Richard Henderson 2009-09-09 8:07 ` Olivier Hainque @ 2009-09-09 8:43 ` Eric Botcazou 2009-09-09 15:34 ` Richard Henderson 2009-09-09 14:34 ` Michael Matz ` (3 subsequent siblings) 5 siblings, 1 reply; 31+ messages in thread From: Eric Botcazou @ 2009-09-09 8:43 UTC (permalink / raw) To: Richard Henderson; +Cc: gcc-patches, Michael Matz > Still writing the changelog, but here's the version that's > current in my tree as of this afternoon. Notable changes > from the last version are: documentation added, varrays > removed, and some bugs fixed. Nice documentation. Some proofreading: An exception is an event that can be "thrown" from within a + function. This event can then be "caught" by the callers of + the function. Missing space before "This". + The representation of exceptions changes several times during + the compliation process: compilation + the TRY nodes to straght-line code. Statements that had been within straight-line + which allows us to assign map the set of types manipulated by + all of the CATCH and EH_FILTER regions to a set of integers. assign or map? + GIMPLE_EH_DISPATCH statements to switch or conditional branches to a switch? -- Eric Botcazou ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 8:43 ` Eric Botcazou @ 2009-09-09 15:34 ` Richard Henderson 0 siblings, 0 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-09 15:34 UTC (permalink / raw) To: Eric Botcazou; +Cc: gcc-patches, Michael Matz On 09/09/2009 01:39 AM, Eric Botcazou wrote: > Nice documentation. Some proofreading: All fixed, thanks. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 0:07 ` Richard Henderson 2009-09-09 8:07 ` Olivier Hainque 2009-09-09 8:43 ` Eric Botcazou @ 2009-09-09 14:34 ` Michael Matz 2009-09-09 15:22 ` Richard Henderson 2009-09-10 0:15 ` Richard Henderson 2009-09-10 2:28 ` Richard Henderson ` (2 subsequent siblings) 5 siblings, 2 replies; 31+ messages in thread From: Michael Matz @ 2009-09-09 14:34 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Tue, 8 Sep 2009, Richard Henderson wrote: > Still writing the changelog, but here's the version that's current in my > tree as of this afternoon. Notable changes from the last version are: > documentation added, Marvelous. While reading the except.c hunks I noticed this: + /* Handle three cases here: + (1) no reachable handler and no edge, + (2) reachable handler and an existing edge to post-landing-pad, + (3) reachable handler and a missing edge. + ??? Not sure why case 3 slips through to this point, it sure + seems like we shouldn't be introducing new statements that can + throw during the gimple->rtl lowering process. The alternative + is that we're losing edges somewhere, which is equally bad. */ Have you meanwhile found out why case 3 can happen there? Because, as you say, expansion shouldn't introduce newly throwing things. Perhaps something to do with builtin expansion that happen to expand (for whatever reason) to normal calls that suddenly might throw? Also, in the sjlj RTL expanders it's still building a cmp-jump sequence in the (one) landing pad. Shouldn't this eventually also be done on the tree-side already (to be able to use a switch)? +/* YOUAREHERE: Replace with expand_resx. */ + +rtx +expand_builtin_eh_copy_values (tree exp) I AM WHERE? :) Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 14:34 ` Michael Matz @ 2009-09-09 15:22 ` Richard Henderson 2009-09-10 0:15 ` Richard Henderson 1 sibling, 0 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-09 15:22 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/09/2009 07:34 AM, Michael Matz wrote: > While reading the except.c hunks I noticed this: > > + /* Handle three cases here: > + (1) no reachable handler and no edge, > + (2) reachable handler and an existing edge to post-landing-pad, > + (3) reachable handler and a missing edge. > + ??? Not sure why case 3 slips through to this point, it sure > + seems like we shouldn't be introducing new statements that can > + throw during the gimple->rtl lowering process. The alternative > + is that we're losing edges somewhere, which is equally bad. */ > > Have you meanwhile found out why case 3 can happen there? No, though I have a theory. I'll experiment with that today. > Also, in the sjlj RTL expanders it's still building a cmp-jump sequence > in the (one) landing pad. Shouldn't this eventually also be done on the > tree-side already (to be able to use a switch)? I'm not sure I can generate the same rtl from trees, because of where I go off and place all the bits from the parts of expand_builtin_setjmp_receiver. I do think I can rearrange things such that I can call back into expand_case though. We'll see. > +/* YOUAREHERE: Replace with expand_resx. */ Oops. =) r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 14:34 ` Michael Matz 2009-09-09 15:22 ` Richard Henderson @ 2009-09-10 0:15 ` Richard Henderson 2009-09-10 10:49 ` Michael Matz 1 sibling, 1 reply; 31+ messages in thread From: Richard Henderson @ 2009-09-10 0:15 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 850 bytes --] On 09/09/2009 07:34 AM, Michael Matz wrote: > Have you meanwhile found out why case 3 can happen there? Because, as you > say, expansion shouldn't introduce newly throwing things. Perhaps > something to do with builtin expansion that happen to expand (for whatever > reason) to normal calls that suddenly might throw? I've regtested with the first patch applied, and the assert doesn't trigger anymore. I'm not sure what was going on originally. > Also, in the sjlj RTL expanders it's still building a cmp-jump sequence > in the (one) landing pad. Shouldn't this eventually also be done on the > tree-side already (to be able to use a switch)? The second patch generates a switch statement for sjlj dispatch from within the finish_eh_generation pass. It works on a spot-checked test case; I'll let the full build cycle run overnight. r~ [-- Attachment #2: commit-18d098b --] [-- Type: text/plain, Size: 1822 bytes --] commit 18d098b8ac53d32e1d62e6abc93bff9d7d42979f Author: Richard Henderson <rth@twiddle.net> Date: Wed Sep 9 12:00:19 2009 -0700 Remove fixup for missing EH edge during finish_eh_generation. diff --git a/gcc/except.c b/gcc/except.c index 7a9ea8f..18916f2 100644 --- a/gcc/except.c +++ b/gcc/except.c @@ -1388,29 +1388,21 @@ finish_eh_generation (void) if (e->flags & EDGE_EH) break; - /* Handle three cases here: - (1) no reachable handler and no edge, - (2) reachable handler and an existing edge to post-landing-pad, - (3) reachable handler and a missing edge. - ??? Not sure why case 3 slips through to this point, it sure - seems like we shouldn't be introducing new statements that can - throw during the gimple->rtl lowering process. The alternative - is that we're losing edges somewhere, which is equally bad. */ - if (lp == NULL) - { - gcc_assert (e == NULL); - continue; - } - if (e == NULL) - e = make_edge (bb, BLOCK_FOR_INSN (lp->landing_pad), EDGE_EH); - else + /* We should not have generated any new throwing insns during this + pass, and we should not have lost any EH edges, so we only need + to handle two cases here: + (1) reachable handler and an existing edge to post-landing-pad, + (2) no reachable handler and no edge. */ + gcc_assert ((lp != NULL) == (e != NULL)); + if (lp != NULL) { gcc_assert (BB_HEAD (e->dest) == label_rtx (lp->post_landing_pad)); - redirect_edge_succ (e, BLOCK_FOR_INSN (lp->landing_pad)); + + redirect_edge_succ (e, BLOCK_FOR_INSN (lp->landing_pad)); + e->flags |= (CALL_P (BB_END (bb)) + ? EDGE_ABNORMAL | EDGE_ABNORMAL_CALL + : EDGE_ABNORMAL); } - e->flags |= (CALL_P (BB_END (bb)) - ? EDGE_ABNORMAL | EDGE_ABNORMAL_CALL - : EDGE_ABNORMAL); } } [-- Attachment #3: d-eh-sjlj-case --] [-- Type: text/plain, Size: 9457 bytes --] * cfgbuild.c (find_bb_boundaries): Don't start a new BB for tablejump labels. * except.c (sjlj_emit_dispatch_table): Build a switch statement. * expr.c (expand_expr_real_2): Handle NON_LVALUE_EXPR. * gimple.c (gimple_build_switch_nlabels): Rename from gimple_build_switch_1; export; handle null default_label. (gimple_build_switch): Handle null default_label. (gimple_build_switch_vec): Likewise. * gimple.h (gimple_build_switch_nlabels): Declare. diff --git a/gcc/cfgbuild.c b/gcc/cfgbuild.c index a7827ab..7d87a7a 100644 --- a/gcc/cfgbuild.c +++ b/gcc/cfgbuild.c @@ -444,8 +444,10 @@ find_bb_boundaries (basic_block bb) { enum rtx_code code = GET_CODE (insn); - /* On code label, split current basic block. */ - if (code == CODE_LABEL) + /* In case we've previously seen an insn that effects a control + flow transfer, split the block. */ + if ((flow_transfer_insn || code == CODE_LABEL) + && inside_basic_block_p (insn)) { fallthru = split_block (bb, PREV_INSN (insn)); if (flow_transfer_insn) @@ -463,36 +465,10 @@ find_bb_boundaries (basic_block bb) bb = fallthru->dest; remove_edge (fallthru); flow_transfer_insn = NULL_RTX; - if (LABEL_ALT_ENTRY_P (insn)) + if (code == CODE_LABEL && LABEL_ALT_ENTRY_P (insn)) make_edge (ENTRY_BLOCK_PTR, bb, 0); } - /* __builtin_unreachable () may cause a barrier to be emitted in - the middle of a BB. We need to split it in the same manner - as if the barrier were preceded by a control_flow_insn_p - insn. */ - if (code == BARRIER && !flow_transfer_insn) - flow_transfer_insn = prev_nonnote_insn_bb (insn); - - /* In case we've previously seen an insn that effects a control - flow transfer, split the block. */ - if (flow_transfer_insn && inside_basic_block_p (insn)) - { - fallthru = split_block (bb, PREV_INSN (insn)); - BB_END (bb) = flow_transfer_insn; - - /* Clean up the bb field for the insns between the blocks. */ - for (x = NEXT_INSN (flow_transfer_insn); - x != BB_HEAD (fallthru->dest); - x = NEXT_INSN (x)) - if (!BARRIER_P (x)) - set_block_for_insn (x, NULL); - - bb = fallthru->dest; - remove_edge (fallthru); - flow_transfer_insn = NULL_RTX; - } - if (control_flow_insn_p (insn)) flow_transfer_insn = insn; if (insn == end) diff --git a/gcc/except.c b/gcc/except.c index 18916f2..eecb7a1 100644 --- a/gcc/except.c +++ b/gcc/except.c @@ -1226,12 +1226,13 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch) enum machine_mode unwind_word_mode = targetm.unwind_word_mode (); enum machine_mode filter_mode = targetm.eh_return_filter_mode (); eh_landing_pad lp; - rtx mem, dispatch, seq, fc, before, exc_ptr_reg, filter_reg; + rtx mem, seq, fc, before, exc_ptr_reg, filter_reg; rtx first_reachable_label; basic_block bb; eh_region r; edge e; int i, disp_index; + gimple switch_stmt; fc = crtl->eh.sjlj_fc; @@ -1252,12 +1253,7 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch) = gen_rtx_EXPR_LIST (VOIDmode, dispatch_label, forced_labels); #endif - /* Load up dispatch index, exc_ptr and filter values from the - function context. */ - mem = adjust_address (fc, TYPE_MODE (integer_type_node), - sjlj_fc_call_site_ofs); - dispatch = copy_to_reg (mem); - + /* Load up exc_ptr and filter values from the function context. */ mem = adjust_address (fc, unwind_word_mode, sjlj_fc_data_ofs); if (unwind_word_mode != ptr_mode) { @@ -1276,11 +1272,24 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch) filter_reg = force_reg (filter_mode, mem); /* Jump to one of the directly reachable regions. */ - /* ??? This really ought to be using a switch statement. */ disp_index = 0; first_reachable_label = NULL; + /* If there's exactly one call site in the function, don't bother + generating a switch statement. */ + switch_stmt = NULL; + if (num_dispatch > 1) + { + tree disp; + + mem = adjust_address (fc, TYPE_MODE (integer_type_node), + sjlj_fc_call_site_ofs); + disp = make_tree (integer_type_node, mem); + + switch_stmt = gimple_build_switch_nlabels (num_dispatch, disp, NULL); + } + for (i = 1; VEC_iterate (eh_landing_pad, cfun->eh->lp_array, i, lp); ++i) if (lp && lp->post_landing_pad) { @@ -1289,10 +1298,24 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch) start_sequence (); lp->landing_pad = dispatch_label; - label = gen_label_rtx (); + + if (num_dispatch > 1) + { + tree t_label, case_elt; + + t_label = create_artificial_label (UNKNOWN_LOCATION); + case_elt = build3 (CASE_LABEL_EXPR, void_type_node, + build_int_cst (NULL, disp_index), + NULL, t_label); + gimple_switch_set_label (switch_stmt, disp_index, case_elt); + + label = label_rtx (t_label); + } + else + label = gen_label_rtx (); + if (disp_index == 0) first_reachable_label = label; - emit_label (label); r = lp->region; @@ -1310,18 +1333,26 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch) e->count = bb->count; e->probability = REG_BR_PROB_BASE; - emit_cmp_and_jump_insns (dispatch, GEN_INT (disp_index), EQ, NULL_RTX, - TYPE_MODE (integer_type_node), 0, label); - disp_index++; } gcc_assert (disp_index == num_dispatch); - expand_builtin_trap (); + + if (num_dispatch > 1) + { + expand_case (switch_stmt); + expand_builtin_trap (); + } seq = get_insns (); end_sequence (); bb = emit_to_new_bb_before (seq, first_reachable_label); + if (num_dispatch == 1) + { + e = make_edge (bb, bb->next_bb, EDGE_FALLTHRU); + e->count = bb->count; + e->probability = REG_BR_PROB_BASE; + } } static void @@ -1335,7 +1366,7 @@ sjlj_build_landing_pads (void) VEC_safe_grow (int, heap, sjlj_lp_call_site_index, num_dispatch); num_dispatch = sjlj_assign_call_site_values (); - if (num_dispatch) + if (num_dispatch > 0) { rtx dispatch_label = gen_label_rtx (); int align = STACK_SLOT_ALIGNMENT (sjlj_fc_type_node, diff --git a/gcc/expr.c b/gcc/expr.c index ab0ce23..2c235d0 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -7237,6 +7237,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, switch (code) { + case NON_LVALUE_EXPR: case PAREN_EXPR: CASE_CONVERT: if (treeop0 == error_mark_node) diff --git a/gcc/gimple.c b/gcc/gimple.c index c32805f..2cda548 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -790,14 +790,15 @@ gimple_build_resx (int region) NLABELS is the number of labels in the switch excluding the default. DEFAULT_LABEL is the default label for the switch statement. */ -static inline gimple -gimple_build_switch_1 (unsigned nlabels, tree index, tree default_label) +gimple +gimple_build_switch_nlabels (unsigned nlabels, tree index, tree default_label) { /* nlabels + 1 default label + 1 index. */ gimple p = gimple_build_with_ops (GIMPLE_SWITCH, ERROR_MARK, - nlabels + 1 + 1); + 1 + (default_label != NULL) + nlabels); gimple_switch_set_index (p, index); - gimple_switch_set_default_label (p, default_label); + if (default_label) + gimple_switch_set_default_label (p, default_label); return p; } @@ -812,15 +813,14 @@ gimple gimple_build_switch (unsigned nlabels, tree index, tree default_label, ...) { va_list al; - unsigned i; - gimple p; - - p = gimple_build_switch_1 (nlabels, index, default_label); + unsigned i, offset; + gimple p = gimple_build_switch_nlabels (nlabels, index, default_label); /* Store the rest of the labels. */ va_start (al, default_label); - for (i = 1; i <= nlabels; i++) - gimple_switch_set_label (p, i, va_arg (al, tree)); + offset = (default_label != NULL); + for (i = 0; i < nlabels; i++) + gimple_switch_set_label (p, i + offset, va_arg (al, tree)); va_end (al); return p; @@ -836,14 +836,13 @@ gimple_build_switch (unsigned nlabels, tree index, tree default_label, ...) gimple gimple_build_switch_vec (tree index, tree default_label, VEC(tree, heap) *args) { - unsigned i; - unsigned nlabels = VEC_length (tree, args); - gimple p = gimple_build_switch_1 (nlabels, index, default_label); + unsigned i, offset, nlabels = VEC_length (tree, args); + gimple p = gimple_build_switch_nlabels (nlabels, index, default_label); - /* Put labels in labels[1 - (nlabels + 1)]. - Default label is in labels[0]. */ - for (i = 1; i <= nlabels; i++) - gimple_switch_set_label (p, i, VEC_index (tree, args, i - 1)); + /* Copy the labels from the vector to the switch statement. */ + offset = (default_label != NULL); + for (i = 0; i < nlabels; i++) + gimple_switch_set_label (p, i + offset, VEC_index (tree, args, i)); return p; } diff --git a/gcc/gimple.h b/gcc/gimple.h index c2b87ef..92aae60 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -794,6 +794,7 @@ gimple gimple_build_try (gimple_seq, gimple_seq, enum gimple_try_flags); gimple gimple_build_wce (gimple_seq); gimple gimple_build_resx (int); gimple gimple_build_eh_dispatch (int); +gimple gimple_build_switch_nlabels (unsigned, tree, tree); gimple gimple_build_switch (unsigned, tree, tree, ...); gimple gimple_build_switch_vec (tree, tree, VEC(tree,heap) *); gimple gimple_build_omp_parallel (gimple_seq, tree, tree, tree); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 0:15 ` Richard Henderson @ 2009-09-10 10:49 ` Michael Matz 2009-09-10 11:41 ` Joseph S. Myers ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Michael Matz @ 2009-09-10 10:49 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Wed, 9 Sep 2009, Richard Henderson wrote: > > Also, in the sjlj RTL expanders it's still building a cmp-jump > > sequence in the (one) landing pad. Shouldn't this eventually also be > > done on the tree-side already (to be able to use a switch)? > > The second patch generates a switch statement for sjlj dispatch from > within the finish_eh_generation pass. It works on a spot-checked test > case; I'll let the full build cycle run overnight. Cool. @@ -7237,6 +7237,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, switch (code) { + case NON_LVALUE_EXPR: case PAREN_EXPR: CASE_CONVERT: if (treeop0 == error_mark_node) Blaehh... reminds me that I once wanted to try to get rid of that tree code, or at least that fold produces it when outside gimple form (the latter probably is the reason that you need that hunk, as you build a new gimple switch while expanding). Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 10:49 ` Michael Matz @ 2009-09-10 11:41 ` Joseph S. Myers 2009-09-28 15:00 ` CONVERT_EXPR cleanup Michael Matz 2009-09-10 16:25 ` [rfc] exception handling in gimple Richard Henderson 2009-09-10 16:46 ` Richard Henderson 2 siblings, 1 reply; 31+ messages in thread From: Joseph S. Myers @ 2009-09-10 11:41 UTC (permalink / raw) To: Michael Matz; +Cc: Richard Henderson, GCC Patches On Thu, 10 Sep 2009, Michael Matz wrote: > + case NON_LVALUE_EXPR: > case PAREN_EXPR: > CASE_CONVERT: > if (treeop0 == error_mark_node) > > Blaehh... reminds me that I once wanted to try to get rid of that tree > code, or at least that fold produces it when outside gimple form (the > latter probably is the reason that you need that hunk, as you build a > new gimple switch while expanding). Finishing the existing incomplete transition from the separate NOP_EXPR and CONVERT_EXPR tree codes to just having CONVERT_EXPR would be nice as well. Most places do now treat them the same via CASE_CONVERT etc.; I don't know what we have left that might receive both codes and treats them differently, that needs resolving before one of the codes can go away. The C front end should no longer need fold to generate NON_LVALUE_EXPR to ensure invalid code is diagnosed; lvalue checks are now done before folding and c_fully_fold_internal removes any NON_LVALUE_EXPRs created by fold. I don't know what requirements the C++ front end may still place on fold in this regard. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 31+ messages in thread
* CONVERT_EXPR cleanup 2009-09-10 11:41 ` Joseph S. Myers @ 2009-09-28 15:00 ` Michael Matz 2009-09-28 15:25 ` Joseph S. Myers 0 siblings, 1 reply; 31+ messages in thread From: Michael Matz @ 2009-09-28 15:00 UTC (permalink / raw) To: Joseph S. Myers; +Cc: GCC Patches Hi, On Thu, 10 Sep 2009, Joseph S. Myers wrote: > Finishing the existing incomplete transition from the separate NOP_EXPR > and CONVERT_EXPR tree codes to just having CONVERT_EXPR would be nice as > well. Most places do now treat them the same via CASE_CONVERT etc.; I > don't know what we have left that might receive both codes and treats > them differently, that needs resolving before one of the codes can go > away. I half-heartedly checked, and it's surprisingly not much. What I did was this in tree.c: @@ -3547,6 +3547,9 @@ build1_stat (enum tree_code code, tree t #endif tree t; + if (code == CONVERT_EXPR) + code = NOP_EXPR; and looking for fallout. So effectively never producing CONVERT_EXPR anymore. It's one missing warning from the C++ frontend, for C only DFP related things that I can fix easily in tree.c:get_narrower, nothing for fortran. I didn't check Ada, java or objc. So, would you be open to changing all CONVERT_EXPR in the C frontend to NOP_EXPR (that is in convert.c, c-parser.c and c-typeck.c)? Most build1's of the middle-end that create CONVERT_EXPRs look already bogus. FWIW, the patch I lightly tested was the one below. Ciao, Michael. -- Index: tree.c =================================================================== --- tree.c (revision 152233) +++ tree.c (working copy) @@ -3547,6 +3547,9 @@ build1_stat (enum tree_code code, tree t #endif tree t; + if (code == CONVERT_EXPR) + code = NOP_EXPR; + #ifdef GATHER_STATISTICS switch (TREE_CODE_CLASS (code)) { @@ -7561,6 +7564,23 @@ get_unwidened (tree op, tree for_type) return win; } \f +/* Given a TYPE returns a number that represents sort of a kind of that + type, namely if it's integral, real, decimal real or fixed point. */ +static int +kind_of_type (tree type) +{ + if (INTEGRAL_TYPE_P (type)) + return 1; + else if (DECIMAL_FLOAT_TYPE_P (type)) + return 2; + else if (SCALAR_FLOAT_TYPE_P (type)) + return 3; + else if (FIXED_POINT_TYPE_P (type)) + return 4; + else + gcc_unreachable (); +} + /* Return OP or a simpler expression for a narrower value which can be sign-extended or zero-extended to give back OP. Store in *UNSIGNEDP_PTR either 1 if the value should be zero-extended @@ -7572,7 +7592,7 @@ get_narrower (tree op, int *unsignedp_pt int uns = 0; int first = 1; tree win = op; - bool integral_p = INTEGRAL_TYPE_P (TREE_TYPE (op)); + int typekind = kind_of_type (TREE_TYPE (op)); while (TREE_CODE (op) == NOP_EXPR) { @@ -7609,13 +7629,12 @@ get_narrower (tree op, int *unsignedp_pt uns = TYPE_UNSIGNED (TREE_TYPE (op)); first = 0; op = TREE_OPERAND (op, 0); - /* Keep trying to narrow, but don't assign op to win if it - would turn an integral type into something else. */ - if (INTEGRAL_TYPE_P (TREE_TYPE (op)) != integral_p) - continue; } - win = op; + /* If we see a change in type kind, don't set win, but + continue trying to narrow. */ + if (kind_of_type (TREE_TYPE (op)) == typekind) + win = op; } if (TREE_CODE (op) == COMPONENT_REF ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: CONVERT_EXPR cleanup 2009-09-28 15:00 ` CONVERT_EXPR cleanup Michael Matz @ 2009-09-28 15:25 ` Joseph S. Myers 2009-09-28 15:44 ` Michael Matz 0 siblings, 1 reply; 31+ messages in thread From: Joseph S. Myers @ 2009-09-28 15:25 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On Mon, 28 Sep 2009, Michael Matz wrote: > So, would you be open to changing all CONVERT_EXPR in the C frontend to > NOP_EXPR (that is in convert.c, c-parser.c and c-typeck.c)? Most build1's > of the middle-end that create CONVERT_EXPRs look already bogus. I think the correct thing to be changing is places that can receive one or both of those codes, to treat them the same (with any consequent fixes needed), rather than arbitrarily changing places generating one code to generate the other. (I'd prefer CONVERT_EXPR rather than NOP_EXPR to be the name that finally survives the unification.) -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: CONVERT_EXPR cleanup 2009-09-28 15:25 ` Joseph S. Myers @ 2009-09-28 15:44 ` Michael Matz 0 siblings, 0 replies; 31+ messages in thread From: Michael Matz @ 2009-09-28 15:44 UTC (permalink / raw) To: Joseph S. Myers; +Cc: GCC Patches Hi, On Mon, 28 Sep 2009, Joseph S. Myers wrote: > > So, would you be open to changing all CONVERT_EXPR in the C frontend to > > NOP_EXPR (that is in convert.c, c-parser.c and c-typeck.c)? Most build1's > > of the middle-end that create CONVERT_EXPRs look already bogus. > > I think the correct thing to be changing is places that can receive one or > both of those codes, to treat them the same (with any consequent fixes > needed), rather than arbitrarily changing places generating one code to > generate the other. Not generating the one in the first place (and consequentially fixing that) would lead to exactly the same end-result. But yes, I can also change all of the left explicit tests to CONVERT_EXPR_P/CASE_CONVERT if that feels more assuring. > (I'd prefer CONVERT_EXPR rather than NOP_EXPR to be the name that > finally survives the unification.) Yes, I just did it this way, because there are much more occurences of NOP_EXPR than CONVERT_EXPR. It just was a ball park check, remember :) Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 10:49 ` Michael Matz 2009-09-10 11:41 ` Joseph S. Myers @ 2009-09-10 16:25 ` Richard Henderson 2009-09-10 16:46 ` Richard Henderson 2 siblings, 0 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-10 16:25 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/10/2009 03:49 AM, Michael Matz wrote: > + case NON_LVALUE_EXPR: > case PAREN_EXPR: > CASE_CONVERT: > if (treeop0 == error_mark_node) > > Blaehh... reminds me that I once wanted to try to get rid of that tree > code, or at least that fold produces it when outside gimple form (the > latter probably is the reason that you need that hunk, as you build a > new gimple switch while expanding). Actually, it's generated within expand_case itself. It folds (INDEX - 0) to NON_LVALUE_EXPR(INDEX). I've no idea how this gets handled differently for switch statements coming from gimple originally. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 10:49 ` Michael Matz 2009-09-10 11:41 ` Joseph S. Myers 2009-09-10 16:25 ` [rfc] exception handling in gimple Richard Henderson @ 2009-09-10 16:46 ` Richard Henderson 2 siblings, 0 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-10 16:46 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/10/2009 03:49 AM, Michael Matz wrote: > + case NON_LVALUE_EXPR: > case PAREN_EXPR: > CASE_CONVERT: > if (treeop0 == error_mark_node) > > Blaehh... reminds me that I once wanted to try to get rid of that tree > code, or at least that fold produces it when outside gimple form (the > latter probably is the reason that you need that hunk, as you build a > new gimple switch while expanding). Actually, it's generated within expand_case itself. It folds (INDEX - 0) to NON_LVALUE_EXPR(INDEX). I've no idea how this gets handled differently for switch statements coming from gimple originally. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 0:07 ` Richard Henderson ` (2 preceding siblings ...) 2009-09-09 14:34 ` Michael Matz @ 2009-09-10 2:28 ` Richard Henderson 2009-09-10 2:32 ` Richard Henderson 2009-09-15 15:43 ` H.J. Lu 5 siblings, 0 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-10 2:28 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 310 bytes --] On 09/08/2009 05:07 PM, Richard Henderson wrote: > Still writing the changelog, but here's the version that's > current in my tree as of this afternoon. Notable changes > from the last version are: documentation added, varrays > removed, and some bugs fixed. Here's the changelog for the patch I posted. r~ [-- Attachment #2: d-eh-9.cl --] [-- Type: application/simple-filter+xml, Size: 18528 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 0:07 ` Richard Henderson ` (3 preceding siblings ...) 2009-09-10 2:28 ` Richard Henderson @ 2009-09-10 2:32 ` Richard Henderson 2009-09-10 10:55 ` Michael Matz 2009-09-15 15:43 ` H.J. Lu 5 siblings, 1 reply; 31+ messages in thread From: Richard Henderson @ 2009-09-10 2:32 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 196 bytes --] One additional fix from the d-eh-8 patch: Two new openmp failures after merging from mainline and pulling in the free_lang_data changes. We weren't getting std::terminate properly mangled. r~ [-- Attachment #2: commit-5a6851a --] [-- Type: text/plain, Size: 788 bytes --] commit 5a6851a6135c91044836e53f4963cc9dfa772825 Author: Richard Henderson <rth@twiddle.net> Date: Wed Sep 9 13:14:32 2009 -0700 Mark must_not_throw failure_decl as used. diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index 68d25a4..ff6dc8f 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -1741,6 +1741,11 @@ lower_eh_must_not_throw (struct leh_state *state, gimple tp) = gimple_eh_must_not_throw_fndecl (inner); this_region->u.must_not_throw.failure_loc = gimple_location (tp); + /* In order to get mangling applied to this decl, we must mark it + used now. Otherwise, pass_ipa_free_lang_data won't think it + needs to happen. */ + TREE_USED (this_region->u.must_not_throw.failure_decl) = 1; + this_state = *state; this_state.cur_region = this_region; ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 2:32 ` Richard Henderson @ 2009-09-10 10:55 ` Michael Matz 2009-09-10 16:28 ` Richard Henderson 0 siblings, 1 reply; 31+ messages in thread From: Michael Matz @ 2009-09-10 10:55 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Wed, 9 Sep 2009, Richard Henderson wrote: > Two new openmp failures after merging from mainline and pulling in the > free_lang_data changes. We weren't getting std::terminate properly > mangled. I'd try it on trunk now, still some time left until end of September :) Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 10:55 ` Michael Matz @ 2009-09-10 16:28 ` Richard Henderson 2009-09-11 10:42 ` Michael Matz 0 siblings, 1 reply; 31+ messages in thread From: Richard Henderson @ 2009-09-10 16:28 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/10/2009 03:55 AM, Michael Matz wrote: > I'd try it on trunk now, still some time left until end of September :) Huh? I am trying it on trunk. I just meant that a recent pull from mainline brought in new failures. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-10 16:28 ` Richard Henderson @ 2009-09-11 10:42 ` Michael Matz 2009-09-11 15:23 ` Richard Henderson 0 siblings, 1 reply; 31+ messages in thread From: Michael Matz @ 2009-09-11 10:42 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Thu, 10 Sep 2009, Richard Henderson wrote: > On 09/10/2009 03:55 AM, Michael Matz wrote: > > I'd try it on trunk now, still some time left until end of September :) > > Huh? I am trying it on trunk. Oh, I meant trying in the sense of forcing others to try. By checking in :) Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-11 10:42 ` Michael Matz @ 2009-09-11 15:23 ` Richard Henderson 0 siblings, 0 replies; 31+ messages in thread From: Richard Henderson @ 2009-09-11 15:23 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/11/2009 03:42 AM, Michael Matz wrote: > Oh, I meant trying in the sense of forcing others to try. By checking in > :) An idea to which I am not opposed. I think I'll wait til Monday, when I'll actually be around to deal with the fallout. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-09 0:07 ` Richard Henderson ` (4 preceding siblings ...) 2009-09-10 2:32 ` Richard Henderson @ 2009-09-15 15:43 ` H.J. Lu 2009-12-22 1:54 ` H.J. Lu 5 siblings, 1 reply; 31+ messages in thread From: H.J. Lu @ 2009-09-15 15:43 UTC (permalink / raw) To: Richard Henderson; +Cc: Michael Matz, GCC Patches On Tue, Sep 8, 2009 at 5:07 PM, Richard Henderson <rth@redhat.com> wrote: > Still writing the changelog, but here's the version that's > current in my tree as of this afternoon. Notable changes > from the last version are: documentation added, varrays > removed, and some bugs fixed. > It caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41360 -- H.J. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-15 15:43 ` H.J. Lu @ 2009-12-22 1:54 ` H.J. Lu 2010-03-14 17:58 ` H.J. Lu 0 siblings, 1 reply; 31+ messages in thread From: H.J. Lu @ 2009-12-22 1:54 UTC (permalink / raw) To: Richard Henderson; +Cc: Michael Matz, GCC Patches On Tue, Sep 15, 2009 at 7:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Sep 8, 2009 at 5:07 PM, Richard Henderson <rth@redhat.com> wrote: >> Still writing the changelog, but here's the version that's >> current in my tree as of this afternoon. Notable changes >> from the last version are: documentation added, varrays >> removed, and some bugs fixed. >> > > It caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41360 > This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42450 -- H.J. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-12-22 1:54 ` H.J. Lu @ 2010-03-14 17:58 ` H.J. Lu 0 siblings, 0 replies; 31+ messages in thread From: H.J. Lu @ 2010-03-14 17:58 UTC (permalink / raw) To: Richard Henderson; +Cc: Michael Matz, GCC Patches On Mon, Dec 21, 2009 at 2:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Sep 15, 2009 at 7:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Sep 8, 2009 at 5:07 PM, Richard Henderson <rth@redhat.com> wrote: >>> Still writing the changelog, but here's the version that's >>> current in my tree as of this afternoon. Notable changes >>> from the last version are: documentation added, varrays >>> removed, and some bugs fixed. >>> >> >> It caused: >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41360 >> > > This also caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42450 > > It also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43365 -- H.J. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-03 1:23 [rfc] exception handling in gimple Richard Henderson 2009-09-08 14:34 ` Michael Matz @ 2009-09-08 15:16 ` Michael Matz 2009-09-08 16:26 ` Richard Henderson 1 sibling, 1 reply; 31+ messages in thread From: Michael Matz @ 2009-09-08 15:16 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Wed, 2 Sep 2009, Richard Henderson wrote: > So this is well beyond the WIP stage, and I'd like to get some real > feedback on it. [still reading patch linearly, skipping the two large files :) ] I think this (tree-cfg.c): - /* OpenMP directives alter control flow. */ - if (is_gimple_omp (t)) - return true; + case GIMPLE_EH_DISPATCH: + /* EH_DISPATCH branches to the individual catch handlers at + this level of a try or allowed-exceptions region. It can + fallthru to the next statement as well. */ + return true; + + case GIMPLE_OMP_PARALLEL: ... and all other GIMPLE_OMP_ stuff explicitely listed ... is better done like: switch (code) { case call: ... case dispatch: ... default: if (is_gimple_omp (t)) ... else break; } If we have a nice predicate already we should use it to reduce maintenance costs. Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-08 15:16 ` Michael Matz @ 2009-09-08 16:26 ` Richard Henderson 2009-09-08 16:42 ` Richard Henderson 0 siblings, 1 reply; 31+ messages in thread From: Richard Henderson @ 2009-09-08 16:26 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches On 09/08/2009 08:16 AM, Michael Matz wrote: > + case GIMPLE_OMP_PARALLEL: > ... and all other GIMPLE_OMP_ stuff explicitely listed ... > > is better done like: > > switch (code) { > case call: ... > case dispatch: ... > default: > if (is_gimple_omp (t)) > ... > else > break; > } > > If we have a nice predicate already we should use it to reduce maintenance > costs. How about I add a "CASE_GIMPLE_OMP"? No maintainence overlap if we implement is_gimple_omp in terms of that. r~ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-08 16:26 ` Richard Henderson @ 2009-09-08 16:42 ` Richard Henderson 2009-09-09 14:38 ` Michael Matz 0 siblings, 1 reply; 31+ messages in thread From: Richard Henderson @ 2009-09-08 16:42 UTC (permalink / raw) To: Michael Matz; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 179 bytes --] On 09/08/2009 09:26 AM, Richard Henderson wrote: > How about I add a "CASE_GIMPLE_OMP"? No maintainence overlap > if we implement is_gimple_omp in terms of that. I.e. this. r~ [-- Attachment #2: z --] [-- Type: text/plain, Size: 3665 bytes --] diff --git a/gcc/gimple.h b/gcc/gimple.h index 74fd656..c2b87ef 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -4207,23 +4207,32 @@ gimple_return_set_retval (gimple gs, tree retval) /* Returns true when the gimple statment STMT is any of the OpenMP types. */ +#define CASE_GIMPLE_OMP \ + case GIMPLE_OMP_PARALLEL: \ + case GIMPLE_OMP_TASK: \ + case GIMPLE_OMP_FOR: \ + case GIMPLE_OMP_SECTIONS: \ + case GIMPLE_OMP_SECTIONS_SWITCH: \ + case GIMPLE_OMP_SINGLE: \ + case GIMPLE_OMP_SECTION: \ + case GIMPLE_OMP_MASTER: \ + case GIMPLE_OMP_ORDERED: \ + case GIMPLE_OMP_CRITICAL: \ + case GIMPLE_OMP_RETURN: \ + case GIMPLE_OMP_ATOMIC_LOAD: \ + case GIMPLE_OMP_ATOMIC_STORE: \ + case GIMPLE_OMP_CONTINUE + static inline bool is_gimple_omp (const_gimple stmt) { - return (gimple_code (stmt) == GIMPLE_OMP_PARALLEL - || gimple_code (stmt) == GIMPLE_OMP_TASK - || gimple_code (stmt) == GIMPLE_OMP_FOR - || gimple_code (stmt) == GIMPLE_OMP_SECTIONS - || gimple_code (stmt) == GIMPLE_OMP_SECTIONS_SWITCH - || gimple_code (stmt) == GIMPLE_OMP_SINGLE - || gimple_code (stmt) == GIMPLE_OMP_SECTION - || gimple_code (stmt) == GIMPLE_OMP_MASTER - || gimple_code (stmt) == GIMPLE_OMP_ORDERED - || gimple_code (stmt) == GIMPLE_OMP_CRITICAL - || gimple_code (stmt) == GIMPLE_OMP_RETURN - || gimple_code (stmt) == GIMPLE_OMP_ATOMIC_LOAD - || gimple_code (stmt) == GIMPLE_OMP_ATOMIC_STORE - || gimple_code (stmt) == GIMPLE_OMP_CONTINUE); + switch (gimple_code (stmt)) + { + CASE_GIMPLE_OMP: + return true; + default: + return false; + } } diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index f7b6ee4..c428cea 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -2821,20 +2821,7 @@ is_ctrl_altering_stmt (gimple t) fallthru to the next statement as well. */ return true; - case GIMPLE_OMP_PARALLEL: - case GIMPLE_OMP_TASK: - case GIMPLE_OMP_FOR: - case GIMPLE_OMP_SECTIONS: - case GIMPLE_OMP_SECTIONS_SWITCH: - case GIMPLE_OMP_SINGLE: - case GIMPLE_OMP_SECTION: - case GIMPLE_OMP_MASTER: - case GIMPLE_OMP_ORDERED: - case GIMPLE_OMP_CRITICAL: - case GIMPLE_OMP_RETURN: - case GIMPLE_OMP_ATOMIC_LOAD: - case GIMPLE_OMP_ATOMIC_STORE: - case GIMPLE_OMP_CONTINUE: + CASE_GIMPLE_OMP: /* OpenMP directives alter control flow. */ return true; @@ -4273,17 +4260,6 @@ verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED) static bool verify_types_in_gimple_stmt (gimple stmt) { - if (is_gimple_omp (stmt)) - { - /* OpenMP directives are validated by the FE and never operated - on by the optimizers. Furthermore, GIMPLE_OMP_FOR may contain - non-gimple expressions when the main index variable has had - its address taken. This does not affect the loop itself - because the header of an GIMPLE_OMP_FOR is merely used to determine - how to setup the parallel iteration. */ - return false; - } - switch (gimple_code (stmt)) { case GIMPLE_ASSIGN: @@ -4322,6 +4298,15 @@ verify_types_in_gimple_stmt (gimple stmt) case GIMPLE_EH_DISPATCH: return false; + CASE_GIMPLE_OMP: + /* OpenMP directives are validated by the FE and never operated + on by the optimizers. Furthermore, GIMPLE_OMP_FOR may contain + non-gimple expressions when the main index variable has had + its address taken. This does not affect the loop itself + because the header of an GIMPLE_OMP_FOR is merely used to determine + how to setup the parallel iteration. */ + return false; + case GIMPLE_DEBUG: return verify_gimple_debug (stmt); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple 2009-09-08 16:42 ` Richard Henderson @ 2009-09-09 14:38 ` Michael Matz 0 siblings, 0 replies; 31+ messages in thread From: Michael Matz @ 2009-09-09 14:38 UTC (permalink / raw) To: Richard Henderson; +Cc: GCC Patches Hi, On Tue, 8 Sep 2009, Richard Henderson wrote: > On 09/08/2009 09:26 AM, Richard Henderson wrote: > > How about I add a "CASE_GIMPLE_OMP"? No maintainence overlap > > if we implement is_gimple_omp in terms of that. > > I.e. this. Works for me. Ciao, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2010-03-14 17:30 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-09-03 1:23 [rfc] exception handling in gimple Richard Henderson 2009-09-08 14:34 ` Michael Matz 2009-09-08 16:25 ` Richard Henderson 2009-09-09 0:07 ` Richard Henderson 2009-09-09 8:07 ` Olivier Hainque 2009-09-09 10:36 ` Michael Matz 2009-09-09 8:43 ` Eric Botcazou 2009-09-09 15:34 ` Richard Henderson 2009-09-09 14:34 ` Michael Matz 2009-09-09 15:22 ` Richard Henderson 2009-09-10 0:15 ` Richard Henderson 2009-09-10 10:49 ` Michael Matz 2009-09-10 11:41 ` Joseph S. Myers 2009-09-28 15:00 ` CONVERT_EXPR cleanup Michael Matz 2009-09-28 15:25 ` Joseph S. Myers 2009-09-28 15:44 ` Michael Matz 2009-09-10 16:25 ` [rfc] exception handling in gimple Richard Henderson 2009-09-10 16:46 ` Richard Henderson 2009-09-10 2:28 ` Richard Henderson 2009-09-10 2:32 ` Richard Henderson 2009-09-10 10:55 ` Michael Matz 2009-09-10 16:28 ` Richard Henderson 2009-09-11 10:42 ` Michael Matz 2009-09-11 15:23 ` Richard Henderson 2009-09-15 15:43 ` H.J. Lu 2009-12-22 1:54 ` H.J. Lu 2010-03-14 17:58 ` H.J. Lu 2009-09-08 15:16 ` Michael Matz 2009-09-08 16:26 ` Richard Henderson 2009-09-08 16:42 ` Richard Henderson 2009-09-09 14:38 ` Michael Matz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).