Re: [patch] Fix over-optimization of calls to pure function

[Richard Biener <richard dot guenther at gmail dot com>]

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