Re: HP-UX IA64 structure passing patch

[Steve Ellcey <sje at cup dot hp dot com>]

> A few things you may not have considered...
> 1) What about varargs/stdarg functions?  Do they know that sub-word structures
>    are left-aligned?  You probably need to modify the ia64_va_arg function
>    in the config/ia64/ia64.c file.

I did forget about varargs but I just tested it and everything seems to
work just fine with my patch (plus a couple of macros I have but forgot
to include in the patch).  I passed various size structures (1 to 9
bytes) in various parameter locations (1st to 12th) and it all seems to
work correctly.

> 2) What about structures on the stack?  If there are 8 or more words of
>    arguments before the sub-word structure, it will end up being passed on the
>    stack.  Presumably this also needs to be left aligned, but I doubt that
>    your patch handles this.  There is a macro FUNCTION_ARG_PADDING that you
>    can set to control this.  See the config/mips/abi64.h file for instance.
>    There may be some other macros you need to set to get this right.

I did test structures on the stack, it seems to work fine.  It looks
like I modified FUNCTION_ARG_PADDING but forgot to include that in my
earlier patch along with PAD_VARARGS_DOWN.  Here is a resubmition that
includes the new FUNCTION_ARG_PADDING (implemented as a function) and
PAD_VARARGS_DOWN.

> There is a possible alternative solution to your patch.  Instead of modifying
> the existing argument/return code, you can use a PARALLEL.  This takes you into
> the emit_group_{load,store} code, which by default does left alignment for big
> endian targets and right-alignment for little endian targets.  We should not
> assume that this is correct, but at least it is the assumption that you need.
> So if you modify FUNCTION_ARG and FUNCTION_VALUE to use a PARALLEL for
> subword structures, then you should get the right alignment without needing
> to modify any machine independent code.  You still need to fix the other two
> things I mentioned above though.

This sounds interesting but I don't know if I could implement it in time
for 3.1 (if at all) and since this affects the compilers ABI compliance
on IA64 HP-UX I would like to get it into 3.1 if at all possible.

> Long term, I think the argument/return stuff needs a rewrite.  Currently
> we have lots of different macros that control argument passing/return, which
> all interact in different confusing ways.  It might be better to have the
> middle end pass all of the arguments to target dependent code, and have the
> target dependent code pass back a structure detailing where they all go.
> This is essentially what the PARALLEL stuff does now, except it only
> handles one argument at a time, and it doesn't support all possible arguments.
> For instance, there is no way to specify aligment for sub-word structures.

I agree that the argument/return stuff is messy but this was the
cleanest way I could find to change it to do what I needed for ABI
compliance.  I would be willing to revisit this code if someone was
rewriting the argument/return code but I don't think that I could do the
rewrite myself.

In the meantime I would like to have this checked in so that GCC / HP C
can correctly interact on IA64.

Here is another copy of the patch with FUNCTION_ARG_PADDING and
PAD_VARARGS_DOWN included:


2001-11-28  Steve Ellcey  <sje@cup.hp.com>

	* doc/tm.texi (TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN): New.
	* target.h (targetm.struct_arg_reg_little_endian): New.
	* target-def.h (TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN): New.
	* calls.c (store_unaligned_arguments_into_pseudos): Check
	targetm.struct_arg_reg_little_endian to see how structures
	are passed/returned.
	* expr.c (move_block_from_reg): Ditto.
	(move_block_from_reg): Ditto.
	(copy_blkmode_from_reg): Ditto.
	* stmt.c (expand_return): Ditto.
	* stor-layout.c (compute_record_mode): If
	targetm.struct_arg_reg_little_endian is set then check 
	MEMBER_TYPE_FORCES_BLK even if mode == VOIDmode.
	* config/ia64/hpux.h (MEMBER_TYPE_FORCES_BLK): Set to true
	so that Structures of one field are still treated as structures.
	(TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN): New, set it to true.
	(FUNCTION_ARG_PADDING): Set to ia64_hpux_function_arg_padding().
	(PAD_VARARGS_DOWN): Modify from default to not pad structures down.
	* ia64-protos.h (ia64_hpux_function_arg_padding): New.
	* ia64.c (ia64_hpux_function_arg_padding): New function to special
	case handling of structure padding.


--- orig/gcc/gcc/doc/tm.texi	Wed Nov 28 16:08:32 2001
+++ gcc/gcc/doc/tm.texi	Wed Nov 28 16:06:38 2001
@@ -6785,6 +6785,14 @@ collecting constructors and destructors 
 It is false if we must use @command{collect2}.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN
+This value is true if the target is big-endian and uses a non-standard
+(little-endian) method of passing and returning structures in registers.
+On the HP-UX IA64 and PA64 platforms structures are aligned differently
+then integral values and setting this value to true will allow for the 
+special handling of structure arguments and return values.
+@end deftypefn
+
 @deftypefn {Target Hook} void TARGET_ASM_CONSTRUCTOR (rtx @var{symbol}, int @var{priority})
 If defined, a function that outputs assembler code to arrange to call
 the function referenced by @var{symbol} at initialization time.
--- orig/gcc/gcc/target.h	Wed Nov 28 12:29:17 2001
+++ gcc/gcc/target.h	Wed Nov 28 11:19:42 2001
@@ -163,6 +163,10 @@ struct gcc_target
   /* True if "native" constructors and destructors are supported,
      false if we're using collect2 for the job.  */
   bool have_ctors_dtors;
+
+  /* True if big-endian system passes/returns structures in a non-standard
+     (little-endian) manner, this is needed for HP-UX IA64 and PA64.  */
+  bool struct_arg_reg_little_endian;
 };
 
 extern struct gcc_target targetm;
--- orig/gcc/gcc/target-def.h	Wed Nov 28 12:29:17 2001
+++ gcc/gcc/target-def.h	Wed Nov 28 11:18:56 2001
@@ -136,6 +136,10 @@ Foundation, 59 Temple Place - Suite 330,
 #define TARGET_SECTION_TYPE_FLAGS default_section_type_flags
 #endif
 
+#ifndef TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN
+#define TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN false
+#endif
+
 /* The whole shebang.  */
 #define TARGET_INITIALIZER			\
 {						\
@@ -152,5 +156,6 @@ Foundation, 59 Temple Place - Suite 330,
   TARGET_EXPAND_BUILTIN,			\
   TARGET_SECTION_TYPE_FLAGS,			\
   TARGET_HAVE_NAMED_SECTIONS,			\
-  TARGET_HAVE_CTORS_DTORS			\
+  TARGET_HAVE_CTORS_DTORS,			\
+  TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN		\
 }
--- orig/gcc/gcc/calls.c	Wed Nov 28 12:29:17 2001
+++ gcc/gcc/calls.c	Wed Nov 28 11:04:44 2001
@@ -33,6 +33,7 @@ Software Foundation, 59 Temple Place - S
 #include "tm_p.h"
 #include "timevar.h"
 #include "sbitmap.h"
+#include "target.h"
 
 #if !defined FUNCTION_OK_FOR_SIBCALL
 #define FUNCTION_OK_FOR_SIBCALL(DECL) 1
@@ -1029,7 +1030,9 @@ store_unaligned_arguments_into_pseudos (
 	   significant byte (to the right).  On a BYTES_BIG_ENDIAN machine,
 	   this means we must skip the empty high order bytes when
 	   calculating the bit offset.  */
-	if (BYTES_BIG_ENDIAN && bytes < UNITS_PER_WORD)
+	if (BYTES_BIG_ENDIAN
+	    && !targetm.struct_arg_reg_little_endian
+	    && bytes < UNITS_PER_WORD)
 	  big_endian_correction = (BITS_PER_WORD  - (bytes * BITS_PER_UNIT));
 
 	for (j = 0; j < args[i].n_aligned_regs; j++)
--- orig/gcc/gcc/expr.c	Wed Nov 28 12:29:17 2001
+++ gcc/gcc/expr.c	Wed Nov 28 16:07:43 2001
@@ -45,6 +45,7 @@ Software Foundation, 59 Temple Place - S
 #include "langhooks.h"
 #include "intl.h"
 #include "tm_p.h"
+#include "target.h"
 
 /* Decide whether a function's arguments should be processed
    from first to last or from last to first.
@@ -1877,7 +1878,8 @@ move_block_from_reg (regno, x, nregs, si
   /* If SIZE is that of a mode no bigger than a word, just use that
      mode's store operation.  */
   if (size <= UNITS_PER_WORD
-      && (mode = mode_for_size (size * BITS_PER_UNIT, MODE_INT, 0)) != BLKmode)
+      && (mode = mode_for_size (size * BITS_PER_UNIT, MODE_INT, 0)) != BLKmode
+      && !targetm.struct_arg_reg_little_endian)
     {
       emit_move_insn (adjust_address (x, mode, 0), gen_rtx_REG (mode, regno));
       return;
@@ -1886,7 +1888,9 @@ move_block_from_reg (regno, x, nregs, si
   /* Blocks smaller than a word on a BYTES_BIG_ENDIAN machine must be aligned
      to the left before storing to memory.  Note that the previous test
      doesn't handle all cases (e.g. SIZE == 3).  */
-  if (size < UNITS_PER_WORD && BYTES_BIG_ENDIAN)
+  if (size < UNITS_PER_WORD
+      && BYTES_BIG_ENDIAN
+      && !targetm.struct_arg_reg_little_endian)
     {
       rtx tem = operand_subword (x, 0, 1, BLKmode);
       rtx shift;
@@ -2171,15 +2175,25 @@ copy_blkmode_from_reg (tgtblk, srcreg, t
 
   /* This code assumes srcreg is at least a full word.  If it isn't,
      copy it into a new pseudo which is a full word.  */
+
+  /* If struct_arg_reg_little_endian is set and convert_to_mode does
+     a copy, the wrong part of the register gets copied so we fake
+     a type conversion in place.  */
+     
   if (GET_MODE (srcreg) != BLKmode
       && GET_MODE_SIZE (GET_MODE (srcreg)) < UNITS_PER_WORD)
-    srcreg = convert_to_mode (word_mode, srcreg, TREE_UNSIGNED (type));
+    if (targetm.struct_arg_reg_little_endian)
+       srcreg = simplify_gen_subreg (word_mode, srcreg, GET_MODE (srcreg), 0);
+    else
+       srcreg = convert_to_mode (word_mode, srcreg, TREE_UNSIGNED (type));
 
   /* Structures whose size is not a multiple of a word are aligned
      to the least significant byte (to the right).  On a BYTES_BIG_ENDIAN
      machine, this means we must skip the empty high order bytes when
      calculating the bit offset.  */
-  if (BYTES_BIG_ENDIAN && bytes % UNITS_PER_WORD)
+  if (BYTES_BIG_ENDIAN
+      && !targetm.struct_arg_reg_little_endian
+      && bytes % UNITS_PER_WORD)
     big_endian_correction
       = (BITS_PER_WORD - ((bytes % UNITS_PER_WORD) * BITS_PER_UNIT));
 
--- orig/gcc/gcc/stmt.c	Wed Nov 28 12:29:17 2001
+++ gcc/gcc/stmt.c	Wed Nov 28 11:18:07 2001
@@ -53,6 +53,7 @@ Software Foundation, 59 Temple Place - S
 #include "toplev.h"
 #include "output.h"
 #include "ggc.h"
+#include "target.h"
 
 #define obstack_chunk_alloc xmalloc
 #define obstack_chunk_free free
@@ -3214,7 +3215,9 @@ expand_return (retval)
 	 to the least significant byte (to the right).  On a BYTES_BIG_ENDIAN
 	 machine, this means we must skip the empty high order bytes when
 	 calculating the bit offset.  */
-      if (BYTES_BIG_ENDIAN && bytes % UNITS_PER_WORD)
+      if (BYTES_BIG_ENDIAN
+	  && !targetm.struct_arg_reg_little_endian
+	  && bytes % UNITS_PER_WORD)
 	big_endian_correction = (BITS_PER_WORD - ((bytes % UNITS_PER_WORD)
 						  * BITS_PER_UNIT));
 
--- orig/gcc/gcc/stor-layout.c	Wed Nov 28 12:29:17 2001
+++ gcc/gcc/stor-layout.c	Wed Nov 28 11:02:59 2001
@@ -30,6 +30,7 @@ Software Foundation, 59 Temple Place - S
 #include "expr.h"
 #include "toplev.h"
 #include "ggc.h"
+#include "target.h"
 
 /* Set to one when set_sizetype has been called.  */
 static int sizetype_set;
@@ -1145,7 +1146,17 @@ compute_record_mode (type)
 #ifdef MEMBER_TYPE_FORCES_BLK
       /* With some targets, eg. c4x, it is sub-optimal
 	 to access an aligned BLKmode structure as a scalar.  */
-      if (mode == VOIDmode && MEMBER_TYPE_FORCES_BLK (field))
+
+      /* On ia64-*-hpux we need to ensure that we don't change the
+	 mode of a structure containing a single field or else we
+	 will pass it incorrectly.  Since a structure with a single
+	 field causes mode to get set above we can't allow the
+	 check for mode == VOIDmode in this case.  Perhaps
+	 MEMBER_TYPE_FORCES_BLK should be extended to include mode
+	 as an argument and the check could be put in there for c4x.  */
+
+      if ((mode == VOIDmode || targetm.struct_arg_reg_little_endian)
+	  && MEMBER_TYPE_FORCES_BLK (field))
 	return;
 #endif /* MEMBER_TYPE_FORCES_BLK  */
     }
--- orig/gcc/gcc/config/ia64/hpux.h	Wed Nov 28 12:28:14 2001
+++ gcc/gcc/config/ia64/hpux.h	Thu Nov 29 09:14:58 2001
@@ -95,3 +95,30 @@ Boston, MA 02111-1307, USA.  */
 
 #undef TARGET_DEFAULT
 #define TARGET_DEFAULT (MASK_DWARF2_ASM | MASK_BIG_ENDIAN)
+
+/* This needs to be set to force structure arguments with a single
+   field to be treated as structures and not as the type of their
+   field.  Without this a structure with a single char will be
+   returned just like a char variable and that is wrong on HP-UX
+   IA64.  TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN triggers the special
+   structure handling, this macro simply ensures that single field
+   structures are always treated like structures.  */
+
+#define MEMBER_TYPE_FORCES_BLK(FIELD) 1
+
+/* Override the setting of targetm.struct_arg_reg_little_endian in
+   target-def.h.  Setting this to true means that we are not passing
+   structures in registers in the "normal" big-endian way.  See
+   See section 8.5 of the "Itanium Software Conventions and Runtime
+   Architecture", specifically Table 8-1 and the explanation of Byte 0
+   alignment and LSB alignment and a description of how structures
+   are passed.  */
+
+#define TARGET_STRUCT_ARG_REG_LITTLE_ENDIAN true
+
+#undef FUNCTION_ARG_PADDING
+#define FUNCTION_ARG_PADDING(MODE, TYPE) \
+	ia64_hpux_function_arg_padding ((MODE), (TYPE))
+
+#undef PAD_VARARGS_DOWN
+#define PAD_VARARGS_DOWN (!AGGREGATE_TYPE_P (type))
--- orig/gcc/gcc/config/ia64/ia64-protos.h	Thu Nov 29 09:12:00 2001
+++ gcc/gcc/config/ia64/ia64-protos.h	Thu Nov 29 09:13:06 2001
@@ -134,3 +134,8 @@ extern void sdata_section PARAMS ((void)
 #ifdef SBSS_SECTION_ASM_OP
 extern void sbss_section PARAMS ((void));
 #endif
+
+#ifdef ARGS_SIZE_RTX
+/* expr.h defines ARGS_SIZE_RTX and `enum direction'.  */
+extern enum direction ia64_hpux_function_arg_padding PARAMS ((enum machine_mode, tree));
+#endif /* ARGS_SIZE_RTX */
--- orig/gcc/gcc/config/ia64/ia64.c	Thu Nov 29 09:10:19 2001
+++ gcc/gcc/config/ia64/ia64.c	Thu Nov 29 09:11:19 2001
@@ -7726,3 +7726,27 @@ ia64_expand_builtin (exp, target, subtar
 
   return NULL_RTX;
 }
+
+/* For the HP-UX IA64 aggregate parameters are passed stored in the
+   most significant bits of the stack slot.  */
+
+enum direction
+ia64_hpux_function_arg_padding (mode, type)
+     enum machine_mode mode;
+     tree type;
+{
+   /* Exception to normal case for structures/unions/etc. */
+
+   if (type && AGGREGATE_TYPE_P (type)
+       && int_size_in_bytes (type) < UNITS_PER_WORD)
+     return upward;
+
+   /* This is the standard FUNCTION_ARG_PADDING with !BYTES_BIG_ENDIAN
+      hardwired to be true. */
+
+   return((mode == BLKmode
+       ? (type && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
+          && int_size_in_bytes (type) < (PARM_BOUNDARY / BITS_PER_UNIT))
+       : GET_MODE_BITSIZE (mode) < PARM_BOUNDARY)
+      ? downward : upward);
+}