[patch/rfc] align_up, align_down

[patch/rfc] align_up, align_down

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

Hello,

This patch introduces two utility functions:

	align_up (v, n);
	align_down (v, n);

for [re]aligning addresses vis:

+   addr = align_up (addr, 8); -- VALUE needs 8 byte alignment
+   write_memory (addr, value, len);
+   addr += len;

+   sp = align_down (sp - len, 16); -- Keep SP 16 byte aligned
+   write_memory (sp, value, len);

It then goes through and replaces all occurances of round_up / 
round_down and align_up / align_down with these globals.

You'll notice that I chose align_XXX rather than round_XXX.  I think 
this better reflects the intended usage.  I've noticed a lot of code doing:

	write_memory (addr, data, len);
	addr += round_up (len, 16);

instead of:

	addr = align_up (addr, 16);
	write_memory (addr, data, len);
	addr += len;

as the former may not result in ADDR having the required alignment.  The 
PPC SVr4 Altivec ABI, for instance, switches between 8 and 16 byte 
alignment making the former code very wrong.

anyway, thoughts?

I'll pick it up in a week,
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 16569 bytes --]

2003-09-12  Andrew Cagney  <cagney@redhat.com>

	* utils.c (align_up, align_down): New functions.
	* defs.h (align_up, align_down): Declare.
	* ppc-sysv-tdep.c (align_up, align_down): Delete functions.
	* s390-tdep.c: Replace "round_up" and "round_down" with "align_up"
	and "align_down".
	(round_up, round_down): Delete functions.
	* mips-tdep.c: Replace ROUND_UP and ROUND_DOWN with "align_up" and
	"align_down".
	(ROUND_DOWN, ROUND_UP): Delete macros.
	(mips_dump_tdep): Do not print "ROUND_UP" or "ROUND_DOWN".
	* h8300-tdep.c: Replace "round_up" and "round_down" with
	"align_up" and "align_down".
	(round_up, round_down): Delete macros.
	* frv-tdep.c: Replace ROUND_UP and ROUND_DOWN with "align_up" and
	"align_down".
	(ROUND_UP, ROUND_DOWN): Delete macros.

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.130
diff -u -r1.130 defs.h
--- defs.h	12 Sep 2003 18:40:16 -0000	1.130
+++ defs.h	12 Sep 2003 19:55:42 -0000
@@ -1264,4 +1264,36 @@
 #define ISATTY(FP)	(isatty (fileno (FP)))
 #endif
 
+/* Ensure that V is aligned to an N byte boundary (B's assumed to be a
+   power of 2).  Round up/down when necessary.  Examples of correct
+   use include:
+
+   addr = align_up (addr, 8); -- VALUE needs 8 byte alignment
+   write_memory (addr, value, len);
+   addr += len;
+
+   and:
+
+   sp = align_down (sp - len, 16); -- Keep SP 16 byte aligned
+   write_memory (sp, value, len);
+
+   Note that uses such as:
+
+   write_memory (addr, value, len);
+   addr += align_up (len, 8);
+
+   and:
+
+   sp -= align_up (len, 8);
+   write_memory (sp, value, len);
+
+   are typically not correct as they don't ensure that the address (SP
+   or ADDR) is correctly aligned (relying on previous alignment to
+   keep things right).  This is also why the methods are called
+   "align_..." instead of "round_..." as the latter reads better with
+   this incorrect coding style.  */
+
+extern ULONGEST align_up (ULONGEST v, int n);
+extern ULONGEST align_down (ULONGEST v, int n);
+
 #endif /* #ifndef DEFS_H */
Index: frv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/frv-tdep.c,v
retrieving revision 1.52
diff -u -r1.52 frv-tdep.c
--- frv-tdep.c	12 Sep 2003 18:40:16 -0000	1.52
+++ frv-tdep.c	12 Sep 2003 19:55:42 -0000
@@ -760,14 +760,11 @@
   return frameless_look_for_prologue (frame);
 }
 
-#define ROUND_UP(n,a) (((n)+(a)-1) & ~((a)-1))
-#define ROUND_DOWN(n,a) ((n) & ~((a)-1))
-
 static CORE_ADDR
 frv_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
 {
   /* Require dword alignment.  */
-  return ROUND_DOWN (sp, 8);
+  return align_down (sp, 8);
 }
 
 static CORE_ADDR
@@ -795,14 +792,14 @@
 
   stack_space = 0;
   for (argnum = 0; argnum < nargs; ++argnum)
-    stack_space += ROUND_UP (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 4);
+    stack_space += align_up (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 4);
 
   stack_space -= (6 * 4);
   if (stack_space > 0)
     sp -= stack_space;
 
   /* Make sure stack is dword aligned. */
-  sp = ROUND_DOWN (sp, 8);
+  sp = align_down (sp, 8);
 
   stack_offset = 0;
 
@@ -852,7 +849,7 @@
 		     argnum, *((int *)val), stack_offset, (int) (sp + stack_offset));
 #endif
 	      write_memory (sp + stack_offset, val, partial_len);
-	      stack_offset += ROUND_UP(partial_len, 4);
+	      stack_offset += align_up (partial_len, 4);
 	    }
 	  len -= partial_len;
 	  val += partial_len;
Index: h8300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
retrieving revision 1.78
diff -u -r1.78 h8300-tdep.c
--- h8300-tdep.c	11 Sep 2003 15:17:14 -0000	1.78
+++ h8300-tdep.c	12 Sep 2003 19:55:42 -0000
@@ -559,12 +559,6 @@
     }
 }
 
-/* Round N up or down to the nearest multiple of UNIT.
-   Evaluate N only once, UNIT several times.
-   UNIT must be a power of two.  */
-#define round_up(n, unit)   (((n) + (unit) - 1) & -(unit))
-#define round_down(n, unit) ((n) & -(unit))
-
 /* Function: push_dummy_call
    Setup the function arguments for calling a function in the inferior.
    In this discussion, a `word' is 16 bits on the H8/300s, and 32 bits
@@ -641,12 +635,12 @@
   int argument;
 
   /* First, make sure the stack is properly aligned.  */
-  sp = round_down (sp, wordsize);
+  sp = align_down (sp, wordsize);
 
   /* Now make sure there's space on the stack for the arguments.  We
      may over-allocate a little here, but that won't hurt anything.  */
   for (argument = 0; argument < nargs; argument++)
-    stack_alloc += round_up (TYPE_LENGTH (VALUE_TYPE (args[argument])),
+    stack_alloc += align_up (TYPE_LENGTH (VALUE_TYPE (args[argument])),
                              wordsize);
   sp -= stack_alloc;
 
@@ -665,7 +659,7 @@
       char *contents = (char *) VALUE_CONTENTS (args[argument]);
 
       /* Pad the argument appropriately.  */
-      int padded_len = round_up (len, wordsize);
+      int padded_len = align_up (len, wordsize);
       char *padded = alloca (padded_len);
 
       memset (padded, 0, padded_len);
Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.229
diff -u -r1.229 mips-tdep.c
--- mips-tdep.c	12 Sep 2003 18:40:17 -0000	1.229
+++ mips-tdep.c	12 Sep 2003 19:55:44 -0000
@@ -2822,18 +2822,12 @@
   return 0;
 }
 
-/* Macros to round N up or down to the next A boundary; 
-   A must be a power of two.  */
-
-#define ROUND_DOWN(n,a) ((n) & ~((a)-1))
-#define ROUND_UP(n,a) (((n)+(a)-1) & ~((a)-1))
-
 /* Adjust the address downward (direction of stack growth) so that it
    is correctly aligned for a new stack frame.  */
 static CORE_ADDR
 mips_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
-  return ROUND_DOWN (addr, 16);
+  return align_down (addr, 16);
 }
 
 static CORE_ADDR
@@ -2862,21 +2856,21 @@
      aligned.  For n32 and n64, stack frames need to be 128-bit
      aligned, so we round to this widest known alignment.  */
 
-  sp = ROUND_DOWN (sp, 16);
-  struct_addr = ROUND_DOWN (struct_addr, 16);
+  sp = align_down (sp, 16);
+  struct_addr = align_down (struct_addr, 16);
 
   /* Now make space on the stack for the args.  We allocate more
      than necessary for EABI, because the first few arguments are
      passed in registers, but that's OK.  */
   for (argnum = 0; argnum < nargs; argnum++)
-    len += ROUND_UP (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
+    len += align_up (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
 		     MIPS_STACK_ARGSIZE);
-  sp -= ROUND_UP (len, 16);
+  sp -= align_up (len, 16);
 
   if (mips_debug)
     fprintf_unfiltered (gdb_stdlog, 
-			"mips_eabi_push_dummy_call: sp=0x%s allocated %d\n",
-			paddr_nz (sp), ROUND_UP (len, 16));
+			"mips_eabi_push_dummy_call: sp=0x%s allocated %ld\n",
+			paddr_nz (sp), (long) align_up (len, 16));
 
   /* Initialize the integer and float register pointers.  */
   argreg = A0_REGNUM;
@@ -3083,7 +3077,7 @@
 	         only needs to be adjusted when it has been used.  */
 
 	      if (stack_used_p)
-		stack_offset += ROUND_UP (partial_len, MIPS_STACK_ARGSIZE);
+		stack_offset += align_up (partial_len, MIPS_STACK_ARGSIZE);
 	    }
 	}
       if (mips_debug)
@@ -3124,19 +3118,19 @@
      aligned.  For n32 and n64, stack frames need to be 128-bit
      aligned, so we round to this widest known alignment.  */
 
-  sp = ROUND_DOWN (sp, 16);
-  struct_addr = ROUND_DOWN (struct_addr, 16);
+  sp = align_down (sp, 16);
+  struct_addr = align_down (struct_addr, 16);
 
   /* Now make space on the stack for the args.  */
   for (argnum = 0; argnum < nargs; argnum++)
-    len += ROUND_UP (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
+    len += align_up (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
 		     MIPS_STACK_ARGSIZE);
-  sp -= ROUND_UP (len, 16);
+  sp -= align_up (len, 16);
 
   if (mips_debug)
     fprintf_unfiltered (gdb_stdlog, 
-			"mips_n32n64_push_dummy_call: sp=0x%s allocated %d\n",
-			paddr_nz (sp), ROUND_UP (len, 16));
+			"mips_n32n64_push_dummy_call: sp=0x%s allocated %ld\n",
+			paddr_nz (sp), (long) align_up (len, 16));
 
   /* Initialize the integer and float register pointers.  */
   argreg = A0_REGNUM;
@@ -3314,7 +3308,7 @@
 	         adjusted when it has been used.  */
 
 	      if (stack_used_p)
-		stack_offset += ROUND_UP (partial_len, MIPS_STACK_ARGSIZE);
+		stack_offset += align_up (partial_len, MIPS_STACK_ARGSIZE);
 	    }
 	}
       if (mips_debug)
@@ -3355,19 +3349,19 @@
      aligned.  For n32 and n64, stack frames need to be 128-bit
      aligned, so we round to this widest known alignment.  */
 
-  sp = ROUND_DOWN (sp, 16);
-  struct_addr = ROUND_DOWN (struct_addr, 16);
+  sp = align_down (sp, 16);
+  struct_addr = align_down (struct_addr, 16);
 
   /* Now make space on the stack for the args.  */
   for (argnum = 0; argnum < nargs; argnum++)
-    len += ROUND_UP (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
+    len += align_up (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
 		     MIPS_STACK_ARGSIZE);
-  sp -= ROUND_UP (len, 16);
+  sp -= align_up (len, 16);
 
   if (mips_debug)
     fprintf_unfiltered (gdb_stdlog, 
-			"mips_o32_push_dummy_call: sp=0x%s allocated %d\n",
-			paddr_nz (sp), ROUND_UP (len, 16));
+			"mips_o32_push_dummy_call: sp=0x%s allocated %ld\n",
+			paddr_nz (sp), (long) align_up (len, 16));
 
   /* Initialize the integer and float register pointers.  */
   argreg = A0_REGNUM;
@@ -3478,7 +3472,7 @@
 	      argreg += FP_REGISTER_DOUBLE ? 1 : 2;
 	    }
 	  /* Reserve space for the FP register.  */
-	  stack_offset += ROUND_UP (len, MIPS_STACK_ARGSIZE);
+	  stack_offset += align_up (len, MIPS_STACK_ARGSIZE);
 	}
       else
 	{
@@ -3622,7 +3616,7 @@
 		 refered to as their "home".  Consequently, space is
 		 always allocated.  */
 
-	      stack_offset += ROUND_UP (partial_len, MIPS_STACK_ARGSIZE);
+	      stack_offset += align_up (partial_len, MIPS_STACK_ARGSIZE);
 	    }
 	}
       if (mips_debug)
@@ -3663,19 +3657,19 @@
      aligned.  For n32 and n64, stack frames need to be 128-bit
      aligned, so we round to this widest known alignment.  */
 
-  sp = ROUND_DOWN (sp, 16);
-  struct_addr = ROUND_DOWN (struct_addr, 16);
+  sp = align_down (sp, 16);
+  struct_addr = align_down (struct_addr, 16);
 
   /* Now make space on the stack for the args.  */
   for (argnum = 0; argnum < nargs; argnum++)
-    len += ROUND_UP (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
+    len += align_up (TYPE_LENGTH (VALUE_TYPE (args[argnum])), 
 		     MIPS_STACK_ARGSIZE);
-  sp -= ROUND_UP (len, 16);
+  sp -= align_up (len, 16);
 
   if (mips_debug)
     fprintf_unfiltered (gdb_stdlog, 
-			"mips_o64_push_dummy_call: sp=0x%s allocated %d\n",
-			paddr_nz (sp), ROUND_UP (len, 16));
+			"mips_o64_push_dummy_call: sp=0x%s allocated %ld\n",
+			paddr_nz (sp), (long) align_up (len, 16));
 
   /* Initialize the integer and float register pointers.  */
   argreg = A0_REGNUM;
@@ -3786,7 +3780,7 @@
 	      argreg += FP_REGISTER_DOUBLE ? 1 : 2;
 	    }
 	  /* Reserve space for the FP register.  */
-	  stack_offset += ROUND_UP (len, MIPS_STACK_ARGSIZE);
+	  stack_offset += align_up (len, MIPS_STACK_ARGSIZE);
 	}
       else
 	{
@@ -3930,7 +3924,7 @@
 		 refered to as their "home".  Consequently, space is
 		 always allocated.  */
 
-	      stack_offset += ROUND_UP (partial_len, MIPS_STACK_ARGSIZE);
+	      stack_offset += align_up (partial_len, MIPS_STACK_ARGSIZE);
 	    }
 	}
       if (mips_debug)
@@ -6404,10 +6398,6 @@
 		      RA_REGNUM);
   fprintf_unfiltered (file,
 		      "mips_dump_tdep: REGISTER_NAMES = delete?\n");
-  fprintf_unfiltered (file,
-		      "mips_dump_tdep: ROUND_DOWN = function?\n");
-  fprintf_unfiltered (file,
-		      "mips_dump_tdep: ROUND_UP = function?\n");
 #ifdef SAVED_BYTES
   fprintf_unfiltered (file,
 		      "mips_dump_tdep: SAVED_BYTES = %d\n",
Index: ppc-sysv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-sysv-tdep.c,v
retrieving revision 1.10
diff -u -r1.10 ppc-sysv-tdep.c
--- ppc-sysv-tdep.c	12 Sep 2003 18:55:24 -0000	1.10
+++ ppc-sysv-tdep.c	12 Sep 2003 19:55:44 -0000
@@ -29,21 +29,6 @@
 
 #include "ppc-tdep.h"
 
-/* Ensure that X is aligned to an S byte boundary (assuming that S is
-   a power of 2) rounding up/down where necessary.  */
-
-static ULONGEST
-align_up (ULONGEST x, int s)
-{
-  return (x + s - 1) & -s;
-}
-
-static ULONGEST
-align_down (ULONGEST x, int s)
-{
-  return (x & -s);
-}
-
 /* Pass the arguments in either registers, or in the stack. Using the
    ppc sysv ABI, the first eight words of the argument list (that might
    be less than eight parameters if some parameters occupy more than one
Index: s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.112
diff -u -r1.112 s390-tdep.c
--- s390-tdep.c	9 Sep 2003 04:41:32 -0000	1.112
+++ s390-tdep.c	12 Sep 2003 19:55:45 -0000
@@ -2207,28 +2207,6 @@
 }
 
 
-/* Round ADDR up to the next N-byte boundary.  N must be a power of
-   two.  */
-static CORE_ADDR
-round_up (CORE_ADDR addr, int n)
-{
-  /* Check that N is really a power of two.  */
-  gdb_assert (n && (n & (n-1)) == 0);
-  return ((addr + n - 1) & -n);
-}
-
-
-/* Round ADDR down to the next N-byte boundary.  N must be a power of
-   two.  */
-static CORE_ADDR
-round_down (CORE_ADDR addr, int n)
-{
-  /* Check that N is really a power of two.  */
-  gdb_assert (n && (n & (n-1)) == 0);
-  return (addr & -n);
-}
-
-
 /* Return the alignment required by TYPE.  */
 static int
 alignment_of (struct type *type)
@@ -2305,7 +2283,7 @@
           && pass_by_copy_ref (type))
         {
           sp -= length;
-          sp = round_down (sp, alignment_of (type));
+          sp = align_down (sp, alignment_of (type));
           write_memory (sp, VALUE_CONTENTS (arg), length);
           copy_addr[i] = sp;
           num_copies++;
@@ -2324,7 +2302,7 @@
         struct type *type = VALUE_TYPE (arg);
         int length = TYPE_LENGTH (type);
         
-        sp = round_down (sp, alignment_of (type));
+        sp = align_down (sp, alignment_of (type));
 
         /* SIMPLE_ARG values get extended to DEPRECATED_REGISTER_SIZE bytes. 
            Assume every argument is.  */
@@ -2334,12 +2312,12 @@
   }
 
   /* Include space for any reference-to-copy pointers.  */
-  sp = round_down (sp, pointer_size);
+  sp = align_down (sp, pointer_size);
   sp -= num_copies * pointer_size;
     
   /* After all that, make sure it's still aligned on an eight-byte
      boundary.  */
-  sp = round_down (sp, 8);
+  sp = align_down (sp, 8);
 
   /* Finally, place the actual parameters, working from SP towards
      higher addresses.  The code above is supposed to reserve enough
@@ -2404,7 +2382,7 @@
               {
                 /* Simple args are always extended to 
                    DEPRECATED_REGISTER_SIZE bytes.  */
-                starg = round_up (starg, DEPRECATED_REGISTER_SIZE);
+                starg = align_up (starg, DEPRECATED_REGISTER_SIZE);
 
                 /* Do we need to pass a pointer to our copy of this
                    argument?  */
@@ -2421,10 +2399,10 @@
             else
               {
                 /* You'd think we should say:
-                   starg = round_up (starg, alignment_of (type));
+                   starg = align_up (starg, alignment_of (type));
                    Unfortunately, GCC seems to simply align the stack on
                    a four/eight-byte boundary, even when passing doubles. */
-                starg = round_up (starg, S390_STACK_PARAMETER_ALIGNMENT);
+                starg = align_up (starg, S390_STACK_PARAMETER_ALIGNMENT);
                 write_memory (starg, VALUE_CONTENTS (arg), length);
                 starg += length;
               }
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.106
diff -u -r1.106 utils.c
--- utils.c	22 Aug 2003 20:23:15 -0000	1.106
+++ utils.c	12 Sep 2003 19:55:55 -0000
@@ -2929,3 +2929,19 @@
     crc = crc32_table[(crc ^ *buf) & 0xff] ^ (crc >> 8);
   return ~crc & 0xffffffff;;
 }
+
+ULONGEST
+align_up (ULONGEST v, int n)
+{
+  /* Check that N is really a power of two.  */
+  gdb_assert (n && (n & (n-1)) == 0);
+  return (v + n - 1) & -n;
+}
+
+ULONGEST
+align_down (ULONGEST v, int n)
+{
+  /* Check that N is really a power of two.  */
+  gdb_assert (n && (n & (n-1)) == 0);
+  return (v & -n);
+}

Re: [patch/rfc] align_up, align_down

[Kevin Buettner]

On Sep 12,  4:08pm, Andrew Cagney wrote:

> This patch introduces two utility functions:
> 
> 	align_up (v, n);
> 	align_down (v, n);
> 
> for [re]aligning addresses vis:
> 
> +   addr = align_up (addr, 8); -- VALUE needs 8 byte alignment
> +   write_memory (addr, value, len);
> +   addr += len;
> 
> +   sp = align_down (sp - len, 16); -- Keep SP 16 byte aligned
> +   write_memory (sp, value, len);
> 
> It then goes through and replaces all occurances of round_up / 
> round_down and align_up / align_down with these globals.
> 
> You'll notice that I chose align_XXX rather than round_XXX.  I think 
> this better reflects the intended usage.  I've noticed a lot of code doing:
> 
> 	write_memory (addr, data, len);
> 	addr += round_up (len, 16);
> 
> instead of:
> 
> 	addr = align_up (addr, 16);
> 	write_memory (addr, data, len);
> 	addr += len;
> 
> as the former may not result in ADDR having the required alignment.  The 
> PPC SVr4 Altivec ABI, for instance, switches between 8 and 16 byte 
> alignment making the former code very wrong.
> 
> anyway, thoughts?

Looks good to me...

Kevin

Re: [patch/rfc] align_up, align_down

[Jim Blandy]

Absolutely.

Andrew Cagney <ac131313@redhat.com> writes:
> 2003-09-12  Andrew Cagney  <cagney@redhat.com>
> 
> 	* utils.c (align_up, align_down): New functions.
> 	* defs.h (align_up, align_down): Declare.
> 	* ppc-sysv-tdep.c (align_up, align_down): Delete functions.
> 	* s390-tdep.c: Replace "round_up" and "round_down" with "align_up"
> 	and "align_down".
> 	(round_up, round_down): Delete functions.
> 	* mips-tdep.c: Replace ROUND_UP and ROUND_DOWN with "align_up" and
> 	"align_down".
> 	(ROUND_DOWN, ROUND_UP): Delete macros.
> 	(mips_dump_tdep): Do not print "ROUND_UP" or "ROUND_DOWN".
> 	* h8300-tdep.c: Replace "round_up" and "round_down" with
> 	"align_up" and "align_down".
> 	(round_up, round_down): Delete macros.
> 	* frv-tdep.c: Replace ROUND_UP and ROUND_DOWN with "align_up" and
> 	"align_down".
> 	(ROUND_UP, ROUND_DOWN): Delete macros.

Re: [patch/rfc] align_up, align_down

[Andrew Cagney]

> Hello,
> 
> This patch introduces two utility functions:
> 
>     align_up (v, n);
>     align_down (v, n);
> 
> for [re]aligning addresses vis:
> 
> +   addr = align_up (addr, 8); -- VALUE needs 8 byte alignment
> +   write_memory (addr, value, len);
> +   addr += len;
> 
> +   sp = align_down (sp - len, 16); -- Keep SP 16 byte aligned
> +   write_memory (sp, value, len);
> 
> It then goes through and replaces all occurances of round_up / round_down and align_up / align_down with these globals.
> 
> You'll notice that I chose align_XXX rather than round_XXX.  I think this better reflects the intended usage.  I've noticed a lot of code doing:
> 
>     write_memory (addr, data, len);
>     addr += round_up (len, 16);
> 
> instead of:
> 
>     addr = align_up (addr, 16);
>     write_memory (addr, data, len);
>     addr += len;
> 
> as the former may not result in ADDR having the required alignment.  The PPC SVr4 Altivec ABI, for instance, switches between 8 and 16 byte alignment making the former code very wrong.
> 
> anyway, thoughts?

All positive.

> I'll pick it up in a week,

I've committed it.

Andrew

> 2003-09-12  Andrew Cagney  <cagney@redhat.com>
> 
> 	* utils.c (align_up, align_down): New functions.
> 	* defs.h (align_up, align_down): Declare.
> 	* ppc-sysv-tdep.c (align_up, align_down): Delete functions.
> 	* s390-tdep.c: Replace "round_up" and "round_down" with "align_up"
> 	and "align_down".
> 	(round_up, round_down): Delete functions.
> 	* mips-tdep.c: Replace ROUND_UP and ROUND_DOWN with "align_up" and
> 	"align_down".
> 	(ROUND_DOWN, ROUND_UP): Delete macros.
> 	(mips_dump_tdep): Do not print "ROUND_UP" or "ROUND_DOWN".
> 	* h8300-tdep.c: Replace "round_up" and "round_down" with
> 	"align_up" and "align_down".
> 	(round_up, round_down): Delete macros.
> 	* frv-tdep.c: Replace ROUND_UP and ROUND_DOWN with "align_up" and
> 	"align_down".
> 	(ROUND_UP, ROUND_DOWN): Delete macros.