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

[pinskia at gmail dot com]

> On May 19, 2014, at 2:39 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
>> 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)

I thought we had decided a long time ago that pure and const functions could not throw and that was the documented behavior. 

Thanks,
Andrew

> 
> 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