Patchwork [middle-end] Fix PR 44878, IA64 build failure, partial inlining

login
register
mail settings
Submitter Steve Ellcey
Date July 21, 2010, 9 p.m.
Message ID <1279746011.9821.113.camel@hpsje.cup.hp.com>
Download mbox | patch
Permalink /patch/59502/
State New
Headers show

Comments

Steve Ellcey - July 21, 2010, 9 p.m.
Here is my latest patch to fix this problem.  I think the issue involves
the for_return argument to promote_function_mode.  The comments for
promote_function_mode simply say that this argument is non-zero if we
are promoting a return value (and not an argument).

But the routine calls targetm.calls.promote_function_mode and if that 
is set to default_promote_function_mode, the code checks for 
(for_return == 2) and calls promote_mode if it is.  This behaviour
is documented in tm.texi:

        @var{for_return} allows to distinguish the promotion of
        arguments and return values.  If it is @code{1}, a return value
        is being promoted and @code{TARGET_FUNCTION_VALUE} must perform
        the same promotions done here.  If it is @code{2}, the returned
        mode should be that of the register in which an incoming
        parameter is copied, or the outgoing result is computed; then
        the hook should return the same mode as @code{promote_mode},
        though the signedness may be different.
        
Given this, it seems that expand_value_return should check for a
reference return and call promote_function_mode with 2 instead of 1
for that case.  This patch allows me to bootstrap on IA64 and the
testing seems to be going OK.  Assuming it works, is this patch OK to
checkin?

Steve Ellcey
sje@cup.hp.com
Richard Guenther - July 21, 2010, 9:08 p.m.
On Wed, Jul 21, 2010 at 11:00 PM, Steve Ellcey <sje@cup.hp.com> wrote:
>
> Here is my latest patch to fix this problem.  I think the issue involves
> the for_return argument to promote_function_mode.  The comments for
> promote_function_mode simply say that this argument is non-zero if we
> are promoting a return value (and not an argument).
>
> But the routine calls targetm.calls.promote_function_mode and if that
> is set to default_promote_function_mode, the code checks for
> (for_return == 2) and calls promote_mode if it is.  This behaviour
> is documented in tm.texi:
>
>        @var{for_return} allows to distinguish the promotion of
>        arguments and return values.  If it is @code{1}, a return value
>        is being promoted and @code{TARGET_FUNCTION_VALUE} must perform
>        the same promotions done here.  If it is @code{2}, the returned
>        mode should be that of the register in which an incoming
>        parameter is copied, or the outgoing result is computed; then
>        the hook should return the same mode as @code{promote_mode},
>        though the signedness may be different.
>
> Given this, it seems that expand_value_return should check for a
> reference return and call promote_function_mode with 2 instead of 1
> for that case.  This patch allows me to bootstrap on IA64 and the
> testing seems to be going OK.  Assuming it works, is this patch OK to
> checkin?

Hm, it sounds reasonable.  But I'd like someone more familiar with
the code review the patch.

Thanks,
Richard.

> Steve Ellcey
> sje@cup.hp.com
>
> Index: stmt.c
> ===================================================================
> --- stmt.c      (revision 162360)
> +++ stmt.c      (working copy)
> @@ -1595,8 +1595,11 @@ expand_value_return (rtx val)
>       tree type = TREE_TYPE (decl);
>       int unsignedp = TYPE_UNSIGNED (type);
>       enum machine_mode old_mode = DECL_MODE (decl);
> -      enum machine_mode mode = promote_function_mode (type, old_mode,
> -                                                     &unsignedp, funtype, 1);
> +      enum machine_mode mode;
> +      if (DECL_BY_REFERENCE (decl))
> +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 2);
> +      else
> +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 1);
>
>       if (mode != old_mode)
>        val = convert_modes (mode, old_mode, val, unsignedp);
>
>
Ian Taylor - July 22, 2010, 8:11 a.m.
Richard Guenther <richard.guenther@gmail.com> writes:

> On Wed, Jul 21, 2010 at 11:00 PM, Steve Ellcey <sje@cup.hp.com> wrote:
>>
>> Here is my latest patch to fix this problem.  I think the issue involves
>> the for_return argument to promote_function_mode.  The comments for
>> promote_function_mode simply say that this argument is non-zero if we
>> are promoting a return value (and not an argument).
>>
>> But the routine calls targetm.calls.promote_function_mode and if that
>> is set to default_promote_function_mode, the code checks for
>> (for_return == 2) and calls promote_mode if it is.  This behaviour
>> is documented in tm.texi:
>>
>>        @var{for_return} allows to distinguish the promotion of
>>        arguments and return values.  If it is @code{1}, a return value
>>        is being promoted and @code{TARGET_FUNCTION_VALUE} must perform
>>        the same promotions done here.  If it is @code{2}, the returned
>>        mode should be that of the register in which an incoming
>>        parameter is copied, or the outgoing result is computed; then
>>        the hook should return the same mode as @code{promote_mode},
>>        though the signedness may be different.
>>
>> Given this, it seems that expand_value_return should check for a
>> reference return and call promote_function_mode with 2 instead of 1
>> for that case.  This patch allows me to bootstrap on IA64 and the
>> testing seems to be going OK.  Assuming it works, is this patch OK to
>> checkin?
>
> Hm, it sounds reasonable.  But I'd like someone more familiar with
> the code review the patch.

This looks OK to me.

Thanks.

Ian

>> ===================================================================
>> --- stmt.c      (revision 162360)
>> +++ stmt.c      (working copy)
>> @@ -1595,8 +1595,11 @@ expand_value_return (rtx val)
>>       tree type = TREE_TYPE (decl);
>>       int unsignedp = TYPE_UNSIGNED (type);
>>       enum machine_mode old_mode = DECL_MODE (decl);
>> -      enum machine_mode mode = promote_function_mode (type, old_mode,
>> -                                                     &unsignedp, funtype, 1);
>> +      enum machine_mode mode;
>> +      if (DECL_BY_REFERENCE (decl))
>> +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 2);
>> +      else
>> +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 1);
>>
>>       if (mode != old_mode)
>>        val = convert_modes (mode, old_mode, val, unsignedp);
>>
>>
Richard Henderson - July 22, 2010, 6:28 p.m.
On 07/21/2010 02:00 PM, Steve Ellcey wrote:
> +      enum machine_mode mode;
> +      if (DECL_BY_REFERENCE (decl))
> +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 2);
> +      else
> +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 1);

I think the logic is good.  I would merely combine the two calls:

  mode = promote_function_mode (type, old_mode, &unsignedp, funtype,
				(DECL_BY_REFERENCE (decl) ? 2 : 1));


r~
Steve Ellcey - July 22, 2010, 6:31 p.m.
On Thu, 2010-07-22 at 11:28 -0700, Richard Henderson wrote:
> On 07/21/2010 02:00 PM, Steve Ellcey wrote:
> > +      enum machine_mode mode;
> > +      if (DECL_BY_REFERENCE (decl))
> > +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 2);
> > +      else
> > +        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 1);
> 
> I think the logic is good.  I would merely combine the two calls:
> 
>   mode = promote_function_mode (type, old_mode, &unsignedp, funtype,
> 				(DECL_BY_REFERENCE (decl) ? 2 : 1));
> 
> 
> r~

I just checked in the original version a minute ago.  Should I change
it?

Steve Ellcey
sje@cup.hp.com
Richard Henderson - July 22, 2010, 6:36 p.m.
On 07/22/2010 11:31 AM, Steve Ellcey wrote:
> I just checked in the original version a minute ago.  Should I change
> it?

I suppose not.

Better of course would be to have a pass that performs this
optimization automatically of course.  ;-)


r~

Patch

Index: stmt.c
===================================================================
--- stmt.c      (revision 162360)
+++ stmt.c      (working copy)
@@ -1595,8 +1595,11 @@  expand_value_return (rtx val)
       tree type = TREE_TYPE (decl);
       int unsignedp = TYPE_UNSIGNED (type);
       enum machine_mode old_mode = DECL_MODE (decl);
-      enum machine_mode mode = promote_function_mode (type, old_mode,
-                                                     &unsignedp, funtype, 1);
+      enum machine_mode mode;
+      if (DECL_BY_REFERENCE (decl))
+        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 2);
+      else
+        mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 1);
 
       if (mode != old_mode)
        val = convert_modes (mode, old_mode, val, unsignedp);