Re: [rfa?] Add frame_align(); Was: ARM stack alignment on hand called functions

[Andrew Cagney <ac131313@redhat.com>]

> At 10:40 28/11/2002 +0000, Richard Earnshaw wrote:
>> Richard Earnshaw <rearnsha@arm.com> writes:
>>
>> |> > +/* Ensure that the ARM's stack pointer has the correct alignment for a
>> |> > +   new frame.  */
>> |> > +static CORE_ADDR
>> |> > +arm_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
>> |> > +{
>> |> > +  return (addr & -16);
>> |> > +}
>> |>
>> |> Yuck, two's complement assumption.
>>
>> No, -16 is implicitly cast to bfd_vma, which is unsigned, and this
>> operation is completely defined independent of the representation of
>> signed integers.
> 
> Hmm, strictly speaking you are correct.  I don't have to like it though,
> and it means that the code is heavily dependent on the non-obvious fact
> that addr is an unsigned type to get the correct behaviour (if bfd_vma
> were to be changed to a signed type then this code would quietly break).
> 
> So I'll change my comment to:
> 
> Yuck, implicit cast of negative number to unsigned.
> 
> I notice that the h8300-tdep.c file defines some macros to do this sort of rounding ... round_up() and round_down().     As aligning addresses is a reasonable common thing to do maybe these macros should be moved to a more generic place and used whenever such rounding is required.    It doesn't fix the implicit cast issue, but would at least make finding where such alignment occurs easier.

Hmm, the've been breeding.  So does the MIPS and s390 ....
Equivalent functions that spell out each step vis:

	ULONGEST mask = -(ULONGEST)(n);
	ULONGEST maddr = (addr & mask);
	return maddr;

and have a 10:1 comment:code ratio sound like a good idea.

A GDB goal can be to debug this at -O3 :-)

Andrew