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

[Jiong Wang <jiwang at tilera dot com>]

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?

Attachment: 01-revise.patch
Description: Text document