Re: [PING] Fix PR46399 - missing mode promotion for libcall args

[NightStrike <nightstrike@gmail.com>]

On Wed, May 4, 2011 at 9:45 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, May 4, 2011 at 3:45 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/5/4 Richard Guenther <richard.guenther@gmail.com>:
>>> On Mon, Apr 18, 2011 at 10:17 AM, Andreas Krebbel
>>> <krebbel@linux.vnet.ibm.com> wrote:
>>>> Hi,
>>>>
>>>> the attached patch uses the existing promote_function_mode hook.  For
>>>> a libcall neither TYPE nor FNTYPE is available so I had to change a
>>>> few related function in order to deal with that.
>>>>
>>>> The patch also fixes the s390 DFP problems.
>>>>
>>>> Bye,
>>>>
>>>> -Andreas-
>>>>
>>>>
>>>> 2011-04-18  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>
>>>>
>>>>        * calls.c (emit_library_call_value_1): Invoke
>>>>        promote_function_mode hook on libcall arguments.
>>>>        * explow.c (promote_function_mode, promote_mode): Handle TYPE
>>>>        argument being NULL.
>>>>        * targhooks.c (default_promote_function_mode): Lisewise.
>>>>        * config/s390/s390.c (s390_promote_function_mode): Likewise.
>>>>        * config/sparc/sparc.c (sparc_promote_function_mode): Likewise.
>>>>
>>>>        * doc/tm.texi: Document that TYPE argument might be NULL.
>>>>
>>>>
>>>> Index: gcc/calls.c
>>>> ===================================================================
>>>> *** gcc/calls.c.orig
>>>> --- gcc/calls.c
>>>> *************** emit_library_call_value_1 (int retval, r
>>>> *** 3484,3489 ****
>>>> --- 3484,3490 ----
>>>>      {
>>>>        rtx val = va_arg (p, rtx);
>>>>        enum machine_mode mode = (enum machine_mode) va_arg (p, int);
>>>> +       int unsigned_p = 0;
>>>>
>>>>        /* We cannot convert the arg value to the mode the library wants here;
>>>>         must do it earlier where we know the signedness of the arg.  */
>>>> *************** emit_library_call_value_1 (int retval, r
>>>> *** 3531,3539 ****
>>>>          val = force_operand (XEXP (slot, 0), NULL_RTX);
>>>>        }
>>>>
>>>> !       argvec[count].value = val;
>>>>        argvec[count].mode = mode;
>>>> !
>>>>        argvec[count].reg = targetm.calls.function_arg (&args_so_far, mode,
>>>>                                                      NULL_TREE, true);
>>>>
>>>> --- 3532,3540 ----
>>>>          val = force_operand (XEXP (slot, 0), NULL_RTX);
>>>>        }
>>>>
>>>> !       mode = promote_function_mode (NULL_TREE, mode, &unsigned_p, NULL_TREE, 0);
>>>>        argvec[count].mode = mode;
>>>> !       argvec[count].value = convert_modes (mode, GET_MODE (val), val, 0);
>>>>        argvec[count].reg = targetm.calls.function_arg (&args_so_far, mode,
>>>>                                                      NULL_TREE, true);
>>>>
>>>> Index: gcc/config/s390/s390.c
>>>> ===================================================================
>>>> *** gcc/config/s390/s390.c.orig
>>>> --- gcc/config/s390/s390.c
>>>> *************** s390_promote_function_mode (const_tree t
>>>> *** 8742,8748 ****
>>>>    if (INTEGRAL_MODE_P (mode)
>>>>        && GET_MODE_SIZE (mode) < UNITS_PER_LONG)
>>>>      {
>>>> !       if (POINTER_TYPE_P (type))
>>>>        *punsignedp = POINTERS_EXTEND_UNSIGNED;
>>>>        return Pmode;
>>>>      }
>>>> --- 8742,8748 ----
>>>>    if (INTEGRAL_MODE_P (mode)
>>>>        && GET_MODE_SIZE (mode) < UNITS_PER_LONG)
>>>>      {
>>>> !       if (type != NULL_TREE && POINTER_TYPE_P (type))
>>>>        *punsignedp = POINTERS_EXTEND_UNSIGNED;
>>>>        return Pmode;
>>>>      }
>>>> Index: gcc/explow.c
>>>> ===================================================================
>>>> *** gcc/explow.c.orig
>>>> --- gcc/explow.c
>>>> *************** enum machine_mode
>>>> *** 771,776 ****
>>>> --- 771,787 ----
>>>>  promote_function_mode (const_tree type, enum machine_mode mode, int *punsignedp,
>>>>                       const_tree funtype, int for_return)
>>>>  {
>>>> +   /* Called without a type node for a libcall.  */
>>>> +   if (type == NULL_TREE)
>>>> +     {
>>>> +       if (INTEGRAL_MODE_P (mode))
>>>> +       return targetm.calls.promote_function_mode (NULL_TREE, mode,
>>>> +                                                   punsignedp, funtype,
>>>> +                                                   for_return);
>>>> +       else
>>>> +       return mode;
>>>> +     }
>>>> +
>>>>    switch (TREE_CODE (type))
>>>>      {
>>>>      case INTEGER_TYPE:   case ENUMERAL_TYPE:   case BOOLEAN_TYPE:
>>>> *************** enum machine_mode
>>>> *** 791,796 ****
>>>> --- 802,813 ----
>>>>  promote_mode (const_tree type ATTRIBUTE_UNUSED, enum machine_mode mode,
>>>>              int *punsignedp ATTRIBUTE_UNUSED)
>>>>  {
>>>> +   /* For libcalls this is invoked without TYPE from the backends
>>>> +      TARGET_PROMOTE_FUNCTION_MODE hooks.  Don't do anything in that
>>>> +      case.  */
>>>> +   if (type == NULL_TREE)
>>>> +     return mode;
>>>> +
>>>
>>> This broke bootstrap
>>>
>>> /space/rguenther/src/svn/trunk/gcc/explow.c: In function 'promote_mode':
>>> /space/rguenther/src/svn/trunk/gcc/explow.c:815:3: error: ISO C90
>>> forbids mixed declarations and code [-Werror=edantic]
>>> cc1: all warnings being treated as errors
>>>
>>>
>>>>    /* FIXME: this is the same logic that was there until GCC 4.4, but we
>>>>       probably want to test POINTERS_EXTEND_UNSIGNED even if PROMOTE_MODE
>>>>       is not defined.  The affected targets are M32C, S390, SPARC.  */
>>>> Index: gcc/config/sparc/sparc.c
>>>> ===================================================================
>>>> *** gcc/config/sparc/sparc.c.orig
>>>> --- gcc/config/sparc/sparc.c
>>>> *************** init_cumulative_args (struct sparc_args
>>>> *** 4965,4977 ****
>>>>  /* Handle promotion of pointer and integer arguments.  */
>>>>
>>>>  static enum machine_mode
>>>> ! sparc_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
>>>>                               enum machine_mode mode,
>>>> !                              int *punsignedp ATTRIBUTE_UNUSED,
>>>>                               const_tree fntype ATTRIBUTE_UNUSED,
>>>>                               int for_return ATTRIBUTE_UNUSED)
>>>>  {
>>>> !   if (POINTER_TYPE_P (type))
>>>>      {
>>>>        *punsignedp = POINTERS_EXTEND_UNSIGNED;
>>>>        return Pmode;
>>>> --- 4965,4977 ----
>>>>  /* Handle promotion of pointer and integer arguments.  */
>>>>
>>>>  static enum machine_mode
>>>> ! sparc_promote_function_mode (const_tree type,
>>>>                               enum machine_mode mode,
>>>> !                              int *punsignedp,
>>>>                               const_tree fntype ATTRIBUTE_UNUSED,
>>>>                               int for_return ATTRIBUTE_UNUSED)
>>>>  {
>>>> !   if (type != NULL_TREE && POINTER_TYPE_P (type))
>>>>      {
>>>>        *punsignedp = POINTERS_EXTEND_UNSIGNED;
>>>>        return Pmode;
>>>> Index: gcc/doc/tm.texi.in
>>>> ===================================================================
>>>> *** gcc/doc/tm.texi.in.orig
>>>> --- gcc/doc/tm.texi.in
>>>> *************** which an incoming parameter is copied, o
>>>> *** 952,957 ****
>>>> --- 952,959 ----
>>>>  then the hook should return the same mode as @code{promote_mode}, though
>>>>  the signedness may be different.
>>>>
>>>> + @var{type} can be omitted when promoting function arguments of libcalls.
>>>> +
>>>>  The default is to not promote arguments and return values.  You can
>>>>  also define the hook to @code{default_promote_function_mode_always_promote}
>>>>  if you would like to apply the same rules given by @code{PROMOTE_MODE}.
>>>> Index: gcc/doc/tm.texi
>>>> ===================================================================
>>>> *** gcc/doc/tm.texi.orig
>>>> --- gcc/doc/tm.texi
>>>> *************** which an incoming parameter is copied, o
>>>> *** 962,967 ****
>>>> --- 962,969 ----
>>>>  then the hook should return the same mode as @code{promote_mode}, though
>>>>  the signedness may be different.
>>>>
>>>> + @var{type} can be omitted when promoting function arguments of libcalls.
>>>> +
>>>>  The default is to not promote arguments and return values.  You can
>>>>  also define the hook to @code{default_promote_function_mode_always_promote}
>>>>  if you would like to apply the same rules given by @code{PROMOTE_MODE}.
>>>> Index: gcc/targhooks.c
>>>> ===================================================================
>>>> *** gcc/targhooks.c.orig
>>>> --- gcc/targhooks.c
>>>> *************** default_promote_function_mode (const_tre
>>>> *** 124,130 ****
>>>>                               const_tree funtype ATTRIBUTE_UNUSED,
>>>>                               int for_return ATTRIBUTE_UNUSED)
>>>>  {
>>>> !   if (for_return == 2)
>>>>      return promote_mode (type, mode, punsignedp);
>>>>    return mode;
>>>>  }
>>>> --- 124,130 ----
>>>>                               const_tree funtype ATTRIBUTE_UNUSED,
>>>>                               int for_return ATTRIBUTE_UNUSED)
>>>>  {
>>>> !   if (type != NULL_TREE && for_return == 2)
>>>>      return promote_mode (type, mode, punsignedp);
>>>>    return mode;
>>>>  }
>>>>
>>>
>>
>>
>> Yes, following patch fix that:
>>
>> Index: explow.c
>> ===================================================================
>> --- explow.c    (revision 173374)
>> +++ explow.c    (working copy)
>> @@ -802,6 +802,10 @@
>>  promote_mode (const_tree type ATTRIBUTE_UNUSED, enum machine_mode mode,
>>              int *punsignedp ATTRIBUTE_UNUSED)
>>  {
>> +#ifdef PROMOTE_MODE
>> +  const enum tree_code code;
>> +  int unsignedp;
>> +#endif
>>   /* For libcalls this is invoked without TYPE from the backends
>>      TARGET_PROMOTE_FUNCTION_MODE hooks.  Don't do anything in that
>>      case.  */
>> @@ -812,8 +816,8 @@
>>      probably want to test POINTERS_EXTEND_UNSIGNED even if PROMOTE_MODE
>>      is not defined.  The affected targets are M32C, S390, SPARC.  */
>>  #ifdef PROMOTE_MODE
>> -  const enum tree_code code = TREE_CODE (type);
>> -  int unsignedp = *punsignedp;
>> +  code = TREE_CODE (type);
>> +  unsignedp = *punsignedp;
>>
>>   switch (code)
>>     {
>>
>> Ok for apply?
>
> It doesn't work ;)
>
> I'm in stage3 and will commit a fix once it passed.
>
> Richard.
>
>> Regards,
>> Kai
>>
>

Did you commit a fix yet?  PR46399 is still unconfirmed.