On May 5, 2015 2:49:55 PM GMT+02:00, Richard Earnshaw
<richard.earns...@foss.arm.com> wrote:
>On 05/05/15 13:46, Richard Earnshaw wrote:
>> On 05/05/15 13:37, Richard Biener wrote:
>>> On May 5, 2015 1:01:59 PM GMT+02:00, Richard Earnshaw
><richard.earns...@foss.arm.com> wrote:
>>>> On 05/05/15 11:54, Richard Earnshaw wrote:
>>>>> On 05/05/15 08:32, Jakub Jelinek wrote:
>>>>>> On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
>>>>>>> So at least changing arm_needs_doubleword_align for
>non-aggregates
>>>> would
>>>>>>> likely not break anything that hasn't been broken already and
>would
>>>> unbreak
>>>>>>> the majority of cases.
>>>>>>
>>>>>> Attached (untested so far). It indeed changes code generated for
>>>>>> over-aligned va_arg, but as I believe you can't properly pass
>those
>>>> in the
>>>>>> ... caller, this should just fix it so that va_arg handling
>matches
>>>> the
>>>>>> caller (and likewise for callees for named argument passing).
>>>>>>
>>>>>>> The following testcase shows that eipa_sra changes alignment
>even
>>>> for the
>>>>>>> aggregates. Change aligned (8) to aligned (4) to see another
>>>> possibility.
>>>>>>
>>>>>> Actually I misread it, for the aggregates esra actually doesn't
>>>> change
>>>>>> anything, which is the reason why the testcase doesn't fail.
>>>>>> The problem with the scalars is that esra first changed it to the
>>>>>> over-aligned MEM_REFs and then later on eipa_sra used the types
>of
>>>> the
>>>>>> MEM_REFs created by esra.
>>>>>>
>>>>>> 2015-05-05 Jakub Jelinek <ja...@redhat.com>
>>>>>>
>>>>>> PR target/65956
>>>>>> * config/arm/arm.c (arm_needs_doubleword_align): For
>non-aggregate
>>>>>> types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type
>>>> itself.
>>>>>>
>>>>>> * gcc.c-torture/execute/pr65956.c: New test.
>>>>>>
>>>>>> --- gcc/config/arm/arm.c.jj 2015-05-04 21:51:42.000000000 +0200
>>>>>> +++ gcc/config/arm/arm.c 2015-05-05 09:20:52.481693337 +0200
>>>>>> @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
>>>>>> static bool
>>>>>> arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>>>>> {
>>>>>> - return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>>>>>> - || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>>>>>> + if (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY)
>>>>>> + return true;
>>>>>
>>>>> I don't think this is right (though I suspect the existing code
>has
>>>> the
>>>>> same problem). We should only look at mode if there is no type
>>>>> information. The problem is that GCC has a nasty habit of
>assigning
>>>>> real machine modes to things that are really BLKmode and we've run
>>>> into
>>>>> several cases where this has royally screwed things up. So for
>>>>> consistency in the ARM back-end we are careful to only use mode
>when
>>>>> type is NULL (=> it's a libcall).
>>>>>
>>>>>> + if (type == NULL_TREE)
>>>>>> + return false;
>>>>>> + if (AGGREGATE_TYPE_P (type))
>>>>>> + return TYPE_ALIGN (type) > PARM_BOUNDARY;
>>>>>> + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>>>>>> }
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> It ought to be possible to re-order this, though, to
>>>>>
>>>>> static bool
>>>>> arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>>>> {
>>>>> - return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>>>>> - || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>>>>> + if (type != NULL_TREE)
>>>>> + {
>>>>> + if (AGGREGATE_TYPE_P (type))
>>>>> + return TYPE_ALIGN (type) > PARM_BOUNDARY;
>>>>> + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) >
>PARM_BOUNDARY;
>>>>> + }
>>>>> + return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY);
>>>>> }
>>>>>
>>>>>
>>>>> Either way, this would need careful cross-testing against an
>existing
>>>>> compiler.
>>>>>
>>>>
>>>> It looks as though either patch would cause ABI incompatibility for
>>>>
>>>> typedef int alignedint __attribute__((aligned((8))));
>>>>
>>>> int __attribute__((weak)) foo (int a, alignedint b)
>>>> {return b;}
>>>>
>>>> void bar (alignedint x)
>>>> {
>>>> foo (1, x);
>>>> }
>>>>
>>>> Where currently gcc uses r2 as the argument register for b in foo.
>>>
>>> And for foo (1,2) or an int typed 2nd arg?
>>>
>>> Richard.
>>>
>>>> R.
>>>
>>>
>>
>>
>> As is
>>
>> int foo2 (int a, int b);
>>
>> void bar2 (alignedint x)
>> {
>> foo2 (1, x);
>> }
>>
>
>The real question here is why is TYPE the type of the value, rather
>than
>the type of the formal as expressed by the prototype (or implicit
>prototype in the case of variadics or K&R)? Surely this is the mid-end
>passing the wrong information to the back-end.
No - the issue is you are looking at the type of the value at all, instead of
at the type of the formal.
Richard.
>R.
>
>> ...
>>
>> R
>>
