PATCH: PR target/46195: r165965 regresses i386 darwin

[Uros Bizjak]

On Fri, Oct 29, 2010 at 7:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>> This function is a simplified version of contains_aligned_value_p,
>>> which should return true/false.
>>
>> It is not simplified, only the top condition is changed ...
>>
>> -  if (((TARGET_SSE && SSE_REG_MODE_P (mode))
>> -       || mode == TDmode
>> -       || mode == TFmode
>> -       || mode == TCmode)
>> -      && (!TYPE_USER_ALIGN (type) || TYPE_ALIGN (type) > 128))
>> -    return true;
>> +  if (mode == XFmode || mode == XCmode)
>> +    return false;
>>
>> ... and else was added:
>>
>>  if (AGGREGATE_TYPE_P (type))
>>    {
>> ...
>>    }
>> +  else
>> +    return TYPE_ALIGN (type) >= 128;
>>
>> Why can't this be part of existing contains_aligned_value_p?
>
> Because contains_aligned_value_p is wrong, which causes:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44948
> We use contains_aligned_value_p to compute the old/wrong
> value only when we warn psABI changes.

I see. So, please rename existing contains_aligned_value_p to
ix86_compat_aligned_value_p, and please add a big comment explaining
that the function is obsolete and is only be used for compatibility
with previous versions. For consistency,
ix86_old_function_arg_boundary should also be renamed to
ix86_compat_function_arg_boundary and similar comment about obsolete
compat function should be added there.

New function can then be named ix86_contains_aligned_value_p, since it
substitutes existing obsolete function.

>>> We have to check mode since type may be NULL. Long double
>>> is a very special case for 32bit psABI where normal alignment of a
>>> type/mode != its alignment when passed on stack.  So we have
>>> to check when normal alignment is 128 with the alignment of mode/type
>>> is determined by long double.
>>
>> I think that following code is much more readable.
>>
>>  if (!TARGET_64BIT)
>>    {
>>      /* i386 ABI defines XFmode arguments to be 4 byte aligned.  */
>>      if (!type)
>>        {
>>          if (mode == XFmode || mode == XCmode)
>>            align = PARM_BOUNDARY;
>>        }
>>      else
>>        {
>>          if (!contains_aligned_value_p (type))
>>            align = PARM_BOUNDARY;
>
>
> contains_aligned_value_p may give the wrong answer
> and lead to:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44948

Yeah, new function should be used here, but please keep the form above
for clarity (I hope it is correct, it is untested).

Thanks,
Uros.