This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Fix over-optimization of calls to pure function
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 19 May 2014 11:39:43 +0200
- Subject: Re: [patch] Fix over-optimization of calls to pure function
- Authentication-results: sourceware.org; auth=none
- References: <5409358 dot 5kA2CkRFhr at polaris>
On Mon, May 19, 2014 at 10:58 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this fixes an over-optimization of the GIMPLE optimizer, whereby two otherwise
> identical calls to a pure function present in different EH regions are CSEd,
> which changes the semantics of the program because the second EH handler is
> not invoked:
>
> begin
> I := F(0);
> exception
> when E => N := N + 1;
> end;
>
> begin
> I := F(0);
> exception
> when E => N := N +1;
> end;
>
> Two passes (DOM and FRE) can optimize this construct and a test modelled on
> stmt_can_throw_internal is used to coax them into not doing so.
>
> Tested on x86_64-suse-linux, OK for the mainline?
Heh, interesting.
Please keep the "redundant" test, it speeds up comparison on hash
collisions. As you are on it I'd use size_int (lr).
I think it's ok to CSE foo (0) for
try {
foo (0);
} catch (...) { side-effect }
foo (0);
but not
foo(0);
try {
foo (0);
} catch (...) { side-effect }
but I don't see a clever way to preserve one but not the other
(at least in SCCVN). Do you think it's worth trying to preserve
that?
Otherwise the patch looks good to me. (bonus point for a C++
testcase as Ada is not in our testing requirements and it would
be nice to exercise this w/o Ada)
Thanks,
Richard.
>
> 2014-05-19 Eric Botcazou <ebotcazou@adacore.com>
>
> * tree-ssa-dom.c (hashable_expr_equal_p) <EXPR_CALL>: Also compare the
> EH region of calls to pure functions that can throw an exception.
> * tree-ssa-sccvn.c (vn_reference_eq): Remove redundant test.
> (copy_reference_ops_from_call): Also copy the EH region of the call if
> it can throw an exception.
>
>
> 2014-05-19 Eric Botcazou <ebotcazou@adacore.com>
>
> * gnat.dg/opt35.adb: New test.
> * gnat.dg/opt35_pkg.ad[sb]: New helper.
>
>
> --
> Eric Botcazou