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

Jiong Wang
2013-01-22T10:28:57Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb18030"; format=flowed, Size: 1907 bytes --]

ÓÚ 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