| Message ID | 6b9bc14e-2c47-7b0e-7535-5e52824f06dc@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Record outer non-cleanup region in TREE EH. | expand |
On Wed, Jan 15, 2020 at 10:57 AM Martin Liška <mliska@suse.cz> wrote: > > Hi. > > The patch is about caching of outer non-CLEANUP region > for a leh_state. It's a spin off the > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93199#c19, > now it's not using a hash_map, but a cached value in leh_state. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? + + eh_region reg = state->cur_region; shouldn't that be reg = state->outer_non_cleanup? - if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state->cur_region)) + if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state)) { this_tf.region = gen_eh_region_cleanup (state->cur_region); this_state.cur_region = this_tf.region; + this_state.outer_non_cleanup = state->outer_non_cleanup; } else { this_tf.region = NULL; this_state.cur_region = state->cur_region; + this_state.outer_non_cleanup = this_state.cur_region; why can't we copy state->outer_non_cleanup here? } > Thanks, > Martin > > gcc/ChangeLog: > > 2020-01-14 Martin Liska <mliska@suse.cz> > > PR tree-optimization/93199 > * tree-eh.c (struct leh_state): Add > new field outer_non_cleanup. > (cleanup_is_dead_in): Pass leh_state instead > of eh_region. Add a checking that state->outer_non_cleanup > points to outer non-clean up region. > (lower_try_finally): Record outer_non_cleanup > for this_state. > (lower_catch): Likewise. > (lower_eh_filter): Likewise. > (lower_eh_must_not_throw): Likewise. > (lower_cleanup): Likewise. > --- > gcc/tree-eh.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > >
On 1/15/20 12:08 PM, Richard Biener wrote: > On Wed, Jan 15, 2020 at 10:57 AM Martin Liška <mliska@suse.cz> wrote: >> >> Hi. >> >> The patch is about caching of outer non-CLEANUP region >> for a leh_state. It's a spin off the >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93199#c19, >> now it's not using a hash_map, but a cached value in leh_state. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> Ready to be installed? > > + > + eh_region reg = state->cur_region; Huh, stupid mistake. > > shouldn't that be reg = state->outer_non_cleanup? > > - if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state->cur_region)) > + if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state)) > { > this_tf.region = gen_eh_region_cleanup (state->cur_region); > this_state.cur_region = this_tf.region; > + this_state.outer_non_cleanup = state->outer_non_cleanup; > } > else > { > this_tf.region = NULL; > this_state.cur_region = state->cur_region; > + this_state.outer_non_cleanup = this_state.cur_region; > > why can't we copy state->outer_non_cleanup here? We can, I'm re-testing that patch. Martin > > } > > >> Thanks, >> Martin >> >> gcc/ChangeLog: >> >> 2020-01-14 Martin Liska <mliska@suse.cz> >> >> PR tree-optimization/93199 >> * tree-eh.c (struct leh_state): Add >> new field outer_non_cleanup. >> (cleanup_is_dead_in): Pass leh_state instead >> of eh_region. Add a checking that state->outer_non_cleanup >> points to outer non-clean up region. >> (lower_try_finally): Record outer_non_cleanup >> for this_state. >> (lower_catch): Likewise. >> (lower_eh_filter): Likewise. >> (lower_eh_must_not_throw): Likewise. >> (lower_cleanup): Likewise. >> --- >> gcc/tree-eh.c | 31 +++++++++++++++++++++++++------ >> 1 file changed, 25 insertions(+), 6 deletions(-) >> >>
On 1/15/20 12:48 PM, Martin Liška wrote: > On 1/15/20 12:08 PM, Richard Biener wrote: >> On Wed, Jan 15, 2020 at 10:57 AM Martin Liška <mliska@suse.cz> wrote: >>> >>> Hi. >>> >>> The patch is about caching of outer non-CLEANUP region >>> for a leh_state. It's a spin off the >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93199#c19, >>> now it's not using a hash_map, but a cached value in leh_state. >>> >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >>> >>> Ready to be installed? >> >> + >> + eh_region reg = state->cur_region; > > Huh, stupid mistake. > >> >> shouldn't that be reg = state->outer_non_cleanup? >> >> - if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state->cur_region)) >> + if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state)) >> { >> this_tf.region = gen_eh_region_cleanup (state->cur_region); >> this_state.cur_region = this_tf.region; >> + this_state.outer_non_cleanup = state->outer_non_cleanup; >> } >> else >> { >> this_tf.region = NULL; >> this_state.cur_region = state->cur_region; >> + this_state.outer_non_cleanup = this_state.cur_region; >> >> why can't we copy state->outer_non_cleanup here? > > We can, I'm re-testing that patch. > > Martin > >> >> } >> >> >>> Thanks, >>> Martin >>> >>> gcc/ChangeLog: >>> >>> 2020-01-14 Martin Liska <mliska@suse.cz> >>> >>> PR tree-optimization/93199 >>> * tree-eh.c (struct leh_state): Add >>> new field outer_non_cleanup. >>> (cleanup_is_dead_in): Pass leh_state instead >>> of eh_region. Add a checking that state->outer_non_cleanup >>> points to outer non-clean up region. >>> (lower_try_finally): Record outer_non_cleanup >>> for this_state. >>> (lower_catch): Likewise. >>> (lower_eh_filter): Likewise. >>> (lower_eh_must_not_throw): Likewise. >>> (lower_cleanup): Likewise. >>> --- >>> gcc/tree-eh.c | 31 +++++++++++++++++++++++++------ >>> 1 file changed, 25 insertions(+), 6 deletions(-) >>> >>> > There's a tested version. Ready to be installed? Thanks, Martin
On Wed, Jan 15, 2020 at 3:48 PM Martin Liška <mliska@suse.cz> wrote: > > On 1/15/20 12:48 PM, Martin Liška wrote: > > On 1/15/20 12:08 PM, Richard Biener wrote: > >> On Wed, Jan 15, 2020 at 10:57 AM Martin Liška <mliska@suse.cz> wrote: > >>> > >>> Hi. > >>> > >>> The patch is about caching of outer non-CLEANUP region > >>> for a leh_state. It's a spin off the > >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93199#c19, > >>> now it's not using a hash_map, but a cached value in leh_state. > >>> > >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >>> > >>> Ready to be installed? > >> > >> + > >> + eh_region reg = state->cur_region; > > > > Huh, stupid mistake. > > > >> > >> shouldn't that be reg = state->outer_non_cleanup? > >> > >> - if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state->cur_region)) > >> + if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state)) > >> { > >> this_tf.region = gen_eh_region_cleanup (state->cur_region); > >> this_state.cur_region = this_tf.region; > >> + this_state.outer_non_cleanup = state->outer_non_cleanup; > >> } > >> else > >> { > >> this_tf.region = NULL; > >> this_state.cur_region = state->cur_region; > >> + this_state.outer_non_cleanup = this_state.cur_region; > >> > >> why can't we copy state->outer_non_cleanup here? > > > > We can, I'm re-testing that patch. > > > > Martin > > > >> > >> } > >> > >> > >>> Thanks, > >>> Martin > >>> > >>> gcc/ChangeLog: > >>> > >>> 2020-01-14 Martin Liska <mliska@suse.cz> > >>> > >>> PR tree-optimization/93199 > >>> * tree-eh.c (struct leh_state): Add > >>> new field outer_non_cleanup. > >>> (cleanup_is_dead_in): Pass leh_state instead > >>> of eh_region. Add a checking that state->outer_non_cleanup > >>> points to outer non-clean up region. > >>> (lower_try_finally): Record outer_non_cleanup > >>> for this_state. > >>> (lower_catch): Likewise. > >>> (lower_eh_filter): Likewise. > >>> (lower_eh_must_not_throw): Likewise. > >>> (lower_cleanup): Likewise. > >>> --- > >>> gcc/tree-eh.c | 31 +++++++++++++++++++++++++------ > >>> 1 file changed, 25 insertions(+), 6 deletions(-) > >>> > >>> > > > > There's a tested version. > > Ready to be installed? OK. Thanks, Richard. > Thanks, > Martin
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index dc80f574a2c..802c25d4b4b 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -356,6 +356,9 @@ struct leh_state split out into a separate structure so that we don't have to copy so much when processing other nodes. */ struct leh_tf_state *tf; + + /* Outer non-clean up region. */ + eh_region outer_non_cleanup; }; struct leh_tf_state @@ -1624,7 +1627,8 @@ decide_copy_try_finally (int ndests, bool may_throw, gimple_seq finally) return f_estimate < 40 || f_estimate * 2 < sw_estimate * 3; } -/* REG is the enclosing region for a possible cleanup region, or the region +/* REG is current region of a LEH state. + is the enclosing region for a possible cleanup region, or the region itself. Returns TRUE if such a region would be unreachable. Cleanup regions within a must-not-throw region aren't actually reachable @@ -1632,10 +1636,18 @@ decide_copy_try_finally (int ndests, bool may_throw, gimple_seq finally) routine will call terminate before unwinding. */ static bool -cleanup_is_dead_in (eh_region reg) +cleanup_is_dead_in (leh_state *state) { - while (reg && reg->type == ERT_CLEANUP) - reg = reg->outer; + if (flag_checking) + { + eh_region reg = state->cur_region; + while (reg && reg->type == ERT_CLEANUP) + reg = reg->outer; + + gcc_assert (reg == state->outer_non_cleanup); + } + + eh_region reg = state->cur_region; return (reg && reg->type == ERT_MUST_NOT_THROW); } @@ -1658,15 +1670,17 @@ lower_try_finally (struct leh_state *state, gtry *tp) this_tf.try_finally_expr = tp; this_tf.top_p = tp; this_tf.outer = state; - if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state->cur_region)) + if (using_eh_for_cleanups_p () && !cleanup_is_dead_in (state)) { this_tf.region = gen_eh_region_cleanup (state->cur_region); this_state.cur_region = this_tf.region; + this_state.outer_non_cleanup = state->outer_non_cleanup; } else { this_tf.region = NULL; this_state.cur_region = state->cur_region; + this_state.outer_non_cleanup = this_state.cur_region; } this_state.ehp_region = state->ehp_region; @@ -1768,6 +1782,7 @@ lower_catch (struct leh_state *state, gtry *tp) { try_region = gen_eh_region_try (state->cur_region); this_state.cur_region = try_region; + this_state.outer_non_cleanup = this_state.cur_region; } lower_eh_constructs_1 (&this_state, gimple_try_eval_ptr (tp)); @@ -1781,6 +1796,7 @@ lower_catch (struct leh_state *state, gtry *tp) emit_resx (&new_seq, try_region); this_state.cur_region = state->cur_region; + this_state.outer_non_cleanup = state->outer_non_cleanup; this_state.ehp_region = try_region; /* Add eh_seq from lowering EH in the cleanup sequence after the cleanup @@ -1857,6 +1873,7 @@ lower_eh_filter (struct leh_state *state, gtry *tp) this_region = gen_eh_region_allowed (state->cur_region, gimple_eh_filter_types (inner)); this_state.cur_region = this_region; + this_state.outer_non_cleanup = this_state.cur_region; } lower_eh_constructs_1 (&this_state, gimple_try_eval_ptr (tp)); @@ -1912,6 +1929,7 @@ lower_eh_must_not_throw (struct leh_state *state, gtry *tp) TREE_USED (this_region->u.must_not_throw.failure_decl) = 1; this_state.cur_region = this_region; + this_state.outer_non_cleanup = this_state.cur_region; } lower_eh_constructs_1 (&this_state, gimple_try_eval_ptr (tp)); @@ -1929,12 +1947,13 @@ lower_cleanup (struct leh_state *state, gtry *tp) eh_region this_region = NULL; struct leh_tf_state fake_tf; gimple_seq result; - bool cleanup_dead = cleanup_is_dead_in (state->cur_region); + bool cleanup_dead = cleanup_is_dead_in (state); if (flag_exceptions && !cleanup_dead) { this_region = gen_eh_region_cleanup (state->cur_region); this_state.cur_region = this_region; + this_state.outer_non_cleanup = state->outer_non_cleanup; } lower_eh_constructs_1 (&this_state, gimple_try_eval_ptr (tp));