diff mbox

Fix PR ipa/64813

Message ID 54D9E67C.209@suse.cz
State New
Headers show

Commit Message

Martin Liška Feb. 10, 2015, 11:07 a.m. UTC 
Hello.

Following patch is fix for PR ipa/64813. The patch was tested on a darwin target
by Dominique.

Ready for trunk?
Thanks,
Martin

Comments

Richard Biener Feb. 10, 2015, 12:50 p.m. UTC | #1 
On Tue, Feb 10, 2015 at 12:07 PM, Martin Liška <mliska@suse.cz> wrote:
> Hello.
>
> Following patch is fix for PR ipa/64813. The patch was tested on a darwin
> target
> by Dominique.
>
> Ready for trunk?

-      if (!(gimple_call_flags (call) & ECF_NORETURN))
+      if (!alias_is_noreturn)

that was technically unnecessary, right?  The call flags properly
return ECF_NORETURN already?

Ok if that is the case.

Thanks,
Roichard.

> Thanks,
> Martin
Martin Liška Feb. 10, 2015, 3:56 p.m. UTC | #2 
On 02/10/2015 01:50 PM, Richard Biener wrote:
> On Tue, Feb 10, 2015 at 12:07 PM, Martin Liška <mliska@suse.cz> wrote:
>> Hello.
>>
>> Following patch is fix for PR ipa/64813. The patch was tested on a darwin
>> target
>> by Dominique.
>>
>> Ready for trunk?
>
> -      if (!(gimple_call_flags (call) & ECF_NORETURN))
> +      if (!alias_is_noreturn)
>
> that was technically unnecessary, right?  The call flags properly
> return ECF_NORETURN already?
>
> Ok if that is the case.

Hi.

You are right, !(gimple_call_flags (call) & ECF_NORETURN) returns a correct value.
Motivation for replacement is not to repeat the same condition, I hope the value
of alias_is_noreturn is correct in all uses.

Thanks,
Martin

>
> Thanks,
> Roichard.
>
>> Thanks,
>> Martin
Jakub Jelinek Feb. 10, 2015, 4 p.m. UTC | #3 
On Tue, Feb 10, 2015 at 04:56:40PM +0100, Martin Liška wrote:
> On 02/10/2015 01:50 PM, Richard Biener wrote:
> >On Tue, Feb 10, 2015 at 12:07 PM, Martin Liška <mliska@suse.cz> wrote:
> >>Hello.
> >>
> >>Following patch is fix for PR ipa/64813. The patch was tested on a darwin
> >>target
> >>by Dominique.
> >>
> >>Ready for trunk?
> >
> >-      if (!(gimple_call_flags (call) & ECF_NORETURN))
> >+      if (!alias_is_noreturn)
> >
> >that was technically unnecessary, right?  The call flags properly
> >return ECF_NORETURN already?
> >
> >Ok if that is the case.
> 
> Hi.
> 
> You are right, !(gimple_call_flags (call) & ECF_NORETURN) returns a correct value.
> Motivation for replacement is not to repeat the same condition, I hope the value
> of alias_is_noreturn is correct in all uses.

And gimple_call_flags (call) & ECF_NORETURN doesn't?  I mean, if you could
initialize alias_is_noreturn to that, it would be nicer.

	Jakub
Jan Hubicka Feb. 10, 2015, 8:25 p.m. UTC | #4 
> Hello.
> 
> Following patch is fix for PR ipa/64813. The patch was tested on a darwin target
> by Dominique.
> 
> Ready for trunk?
OK,
Honza
> Thanks,
> Martin

> >From ce1c7031e2616d840ce55559c1bc45587d64134f Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Fri, 30 Jan 2015 15:12:59 +0100
> Subject: [PATCH] Handle noreturn function thunk creation.
> 
> gcc/ChangeLog:
> 
> 2015-02-09  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/64813
> 	* cgraphunit.c (cgraph_node::expand_thunk): Do not create
> 	a return value for call to a function that is noreturn.
> ---
>  gcc/cgraphunit.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 8280fc4..77c8dd8 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1572,6 +1572,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>  
>        gcall *call;
>        greturn *ret;
> +      bool alias_is_noreturn = TREE_THIS_VOLATILE (alias);
>  
>        if (in_lto_p)
>  	get_untransformed_body ();
> @@ -1608,7 +1609,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>        bsi = gsi_start_bb (bb);
>  
>        /* Build call to the function being thunked.  */
> -      if (!VOID_TYPE_P (restype))
> +      if (!VOID_TYPE_P (restype) && !alias_is_noreturn)
>  	{
>  	  if (DECL_BY_REFERENCE (resdecl))
>  	    {
> @@ -1667,14 +1668,14 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>        callees->call_stmt = call;
>        gimple_call_set_from_thunk (call, true);
>        gimple_call_set_with_bounds (call, instrumentation_clone);
> -      if (restmp)
> +      if (restmp && !alias_is_noreturn)
>  	{
>            gimple_call_set_lhs (call, restmp);
>  	  gcc_assert (useless_type_conversion_p (TREE_TYPE (restmp),
>  						 TREE_TYPE (TREE_TYPE (alias))));
>  	}
>        gsi_insert_after (&bsi, call, GSI_NEW_STMT);
> -      if (!(gimple_call_flags (call) & ECF_NORETURN))
> +      if (!alias_is_noreturn)
>  	{
>  	  if (restmp && !this_adjusting
>  	      && (fixed_offset || virtual_offset))
> -- 
> 2.1.2
>
Martin Liška Feb. 11, 2015, 12:46 p.m. UTC | #5 
On 02/10/2015 05:00 PM, Jakub Jelinek wrote:
> On Tue, Feb 10, 2015 at 04:56:40PM +0100, Martin Liška wrote:
>> On 02/10/2015 01:50 PM, Richard Biener wrote:
>>> On Tue, Feb 10, 2015 at 12:07 PM, Martin Liška <mliska@suse.cz> wrote:
>>>> Hello.
>>>>
>>>> Following patch is fix for PR ipa/64813. The patch was tested on a darwin
>>>> target
>>>> by Dominique.
>>>>
>>>> Ready for trunk?
>>>
>>> -      if (!(gimple_call_flags (call) & ECF_NORETURN))
>>> +      if (!alias_is_noreturn)
>>>
>>> that was technically unnecessary, right?  The call flags properly
>>> return ECF_NORETURN already?
>>>
>>> Ok if that is the case.
>>
>> Hi.
>>
>> You are right, !(gimple_call_flags (call) & ECF_NORETURN) returns a correct value.
>> Motivation for replacement is not to repeat the same condition, I hope the value
>> of alias_is_noreturn is correct in all uses.
>
> And gimple_call_flags (call) & ECF_NORETURN doesn't?  I mean, if you could
> initialize alias_is_noreturn to that, it would be nicer.
>
> 	Jakub
>

Hello.

There are actually 3 places I need to guard if a function non-return.
First place is responsible for result type creation:

       /* Build call to the function being thunked.  */
       if (!VOID_TYPE_P (restype) && !alias_is_noreturn)

Where I can create guard just based on TREE_THIS_VOLATILE of the called function.
Once the new gimple call is created I can use:

(gimple_call_flags (call) & ECF_NORETURN

So that's the reason I create one bool value (alias_is_noreturn) and the very
beginning of expand_thunk method. Dost it make sense?

Thanks,
Martin
Richard Biener Feb. 11, 2015, 1:19 p.m. UTC | #6 
On Wed, Feb 11, 2015 at 1:46 PM, Martin Liška <mliska@suse.cz> wrote:
> On 02/10/2015 05:00 PM, Jakub Jelinek wrote:
>>
>> On Tue, Feb 10, 2015 at 04:56:40PM +0100, Martin Liška wrote:
>>>
>>> On 02/10/2015 01:50 PM, Richard Biener wrote:
>>>>
>>>> On Tue, Feb 10, 2015 at 12:07 PM, Martin Liška <mliska@suse.cz> wrote:
>>>>>
>>>>> Hello.
>>>>>
>>>>> Following patch is fix for PR ipa/64813. The patch was tested on a
>>>>> darwin
>>>>> target
>>>>> by Dominique.
>>>>>
>>>>> Ready for trunk?
>>>>
>>>>
>>>> -      if (!(gimple_call_flags (call) & ECF_NORETURN))
>>>> +      if (!alias_is_noreturn)
>>>>
>>>> that was technically unnecessary, right?  The call flags properly
>>>> return ECF_NORETURN already?
>>>>
>>>> Ok if that is the case.
>>>
>>>
>>> Hi.
>>>
>>> You are right, !(gimple_call_flags (call) & ECF_NORETURN) returns a
>>> correct value.
>>> Motivation for replacement is not to repeat the same condition, I hope
>>> the value
>>> of alias_is_noreturn is correct in all uses.
>>
>>
>> And gimple_call_flags (call) & ECF_NORETURN doesn't?  I mean, if you could
>> initialize alias_is_noreturn to that, it would be nicer.
>>
>>         Jakub
>>
>
> Hello.
>
> There are actually 3 places I need to guard if a function non-return.
> First place is responsible for result type creation:
>
>       /* Build call to the function being thunked.  */
>       if (!VOID_TYPE_P (restype) && !alias_is_noreturn)
>
> Where I can create guard just based on TREE_THIS_VOLATILE of the called
> function.
> Once the new gimple call is created I can use:
>
> (gimple_call_flags (call) & ECF_NORETURN
>
> So that's the reason I create one bool value (alias_is_noreturn) and the
> very
> beginning of expand_thunk method. Dost it make sense?

Yes, and I think your patch is ok.

Thanks,
Richard.

> Thanks,
> Martin
diff mbox

Patch

From ce1c7031e2616d840ce55559c1bc45587d64134f Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Fri, 30 Jan 2015 15:12:59 +0100
Subject: [PATCH] Handle noreturn function thunk creation.

gcc/ChangeLog:

2015-02-09  Martin Liska  <mliska@suse.cz>

	PR ipa/64813
	* cgraphunit.c (cgraph_node::expand_thunk): Do not create
	a return value for call to a function that is noreturn.
---
 gcc/cgraphunit.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 8280fc4..77c8dd8 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1572,6 +1572,7 @@  cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
 
       gcall *call;
       greturn *ret;
+      bool alias_is_noreturn = TREE_THIS_VOLATILE (alias);
 
       if (in_lto_p)
 	get_untransformed_body ();
@@ -1608,7 +1609,7 @@  cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       bsi = gsi_start_bb (bb);
 
       /* Build call to the function being thunked.  */
-      if (!VOID_TYPE_P (restype))
+      if (!VOID_TYPE_P (restype) && !alias_is_noreturn)
 	{
 	  if (DECL_BY_REFERENCE (resdecl))
 	    {
@@ -1667,14 +1668,14 @@  cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       callees->call_stmt = call;
       gimple_call_set_from_thunk (call, true);
       gimple_call_set_with_bounds (call, instrumentation_clone);
-      if (restmp)
+      if (restmp && !alias_is_noreturn)
 	{
           gimple_call_set_lhs (call, restmp);
 	  gcc_assert (useless_type_conversion_p (TREE_TYPE (restmp),
 						 TREE_TYPE (TREE_TYPE (alias))));
 	}
       gsi_insert_after (&bsi, call, GSI_NEW_STMT);
-      if (!(gimple_call_flags (call) & ECF_NORETURN))
+      if (!alias_is_noreturn)
 	{
 	  if (restmp && !this_adjusting
 	      && (fixed_offset || virtual_offset))
-- 
2.1.2