Re: [RFC/TileGX 1/6] fix args alignment bug

[Jiong Wang <jiwang@tilera.com>]

Ok, if no one object, Walter (GNU appointed TileGX gcc/binutils 
maintainer) will checkin all these updated TileGX patches.

thanks.
---
Regards,
Jiong

于 2013/1/29 22:48, Jiong Wang 写道:
> PING, any feedback?
>
> I had fixed all issues according to feedback, these patches cut tilegx 
> Dejagnu unexpected failures from 120 to about 50.
>
> so is it OK to commit them? if it's OK to commit, can any one help 
> commit them? we do not have a gdb account yet.
>
> thanks
>
> Regards,
> Jiong
> Tilera Corporation.
>
>
> 于 2013/1/22 18:28, Jiong Wang 写道:
>> 于 2013/1/18 23:00, Jiong Wang 写道:
>>> Hi Joel,
>>>
>>>   thanks for your careful review
>>>
>>>>> -  /* Loop backwards through arguments to determine stack 
>>>>> alignment.  */
>>>>> -  alignlen = 0;
>>>>> -
>>>>> -  for (j = nargs - 1; j >= i; j--)
>>>>> -    {
>>>>> -      typelen = TYPE_LENGTH (value_enclosing_type (args[j]));
>>>>> -      alignlen += (typelen + 3) & (~3);
>>>>> -    }
>>>>> -
>>>>> -  if (alignlen & 0x4)
>>>>> -    stack_dest -= 4;
>>>> Do you have any hint as to why this code was written the way it was?
>>>> It doesn't seem like the type of code that would be added by accident.
>>>> Did Jeff (the apparent author) misunderstand the ABI?
>>>
>>>       actually, these these copied from our tilepro target which is 
>>> a pure 32bit target, and the
>>> compiler for that target is not gcc initially. so there maybe some 
>>> historic reason for those code.
>>>
>>>      but for tilegx, the ABI is actually very clear, 10 regs for 
>>> arguments,  then push on stack.
>>>
>>>      so I just delete them, gdb works ok on both daily usage and 
>>> dejagnu
>>>>
>>>>>         typelen = TYPE_LENGTH (value_enclosing_type (args[j]));
>>>>> -      slacklen = ((typelen + 3) & (~3)) - typelen;
>>>>> +      slacklen = ((typelen + 7) & (~7)) - typelen;
>>>> This is a detail, but can you use utils.c:align_up, in this case?
>>>> (I find those midly more readable than these incantations, but
>>>> it's OK if you prefer the code to stay as is).
>>>       thanks, I change to align_up to reuse existing helper function
>>>>
>>>>>     /* Add 2 words for linkage space to the stack.  */
>>>>> -  stack_dest = stack_dest - 8;
>>>>> -  write_memory (stack_dest, two_zero_words, 8);
>>>>> +  stack_dest = stack_dest - 16;
>>>>> +  write_memory (stack_dest, four_zero_words, 16);
>>>> It looks like you need to adjust the comment as well.
>>>
>>>    fixed.
>>>
>>>   is this new 01-revise.patch ok for this issue?
>>>
>> Ping, are this and the other updated tilegx patches OK?
>>
>> Thanks,
>> Jiong
>>
>