* Applying FUNCTION_ARG_PADDING to register arguments
@ 1997-10-23 12:49 Joern Rennecke
1997-10-23 17:28 ` Jim Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 1997-10-23 12:49 UTC (permalink / raw)
To: egcs
While tracking down a varargs bug, I found that calls.c doesn't respect
FUNCTION_ARG_PADDING for register arguments. I think that is a bug.
I have appended a fix. One loop needed rewriting to support
FUNCTION_ARG_PADDING properly, therefore I have left the old test in
#if 0 so that you can more easily see what the actual change is and
what is just necessery code to implement it.
Thu Oct 23 18:11:33 1997 J"orn Rennecke <amylaar@cygnus.co.uk>
* calls.c (expand_call): Use FUNCTION_ARG_PADDING also for
register parameters.
Index: calls.c
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/calls.c,v
retrieving revision 1.2
diff -p -r1.2 calls.c
*** calls.c 1997/08/11 20:07:08 1.2
--- calls.c 1997/10/23 17:21:04
*************** expand_call (exp, target, ignore)
*** 1773,1778 ****
--- 1773,1781 ----
{
int bytes = int_size_in_bytes (TREE_TYPE (args[i].tree_value));
int big_endian_correction = 0;
+ int bitsize = TYPE_ALIGN (TREE_TYPE (args[i].tree_value));
+ rtx reg, word;
+ int k, bitpos, xbitpos;
args[i].n_aligned_regs
= args[i].partial ? args[i].partial
*************** expand_call (exp, target, ignore)
*** 1781,1829 ****
args[i].aligned_regs = (rtx *) alloca (sizeof (rtx)
* args[i].n_aligned_regs);
/* Structures smaller than 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)
big_endian_correction = (BITS_PER_WORD - (bytes * BITS_PER_UNIT));
!
! for (j = 0; j < args[i].n_aligned_regs; j++)
{
! rtx reg = gen_reg_rtx (word_mode);
! rtx word = operand_subword_force (args[i].value, j, BLKmode);
! int bitsize = TYPE_ALIGN (TREE_TYPE (args[i].tree_value));
! int bitpos;
!
! args[i].aligned_regs[j] = reg;
!
! /* Clobber REG and move each partword into it. Ensure we don't
! go past the end of the structure. Note that the loop below
! works because we've already verified that padding
! and endianness are compatible.
!
! We use to emit a clobber here but that doesn't let later
! passes optimize the instructions we emit. By storing 0 into
! the register later passes know the first AND to zero out the
! bitfield being set in the register is unnecessary. The store
! of 0 will be deleted as will at least the first AND. */
!
! emit_move_insn (reg, const0_rtx);
!
! for (bitpos = 0;
! bitpos < BITS_PER_WORD && bytes > 0;
! bitpos += bitsize, bytes -= bitsize / BITS_PER_UNIT)
{
! int xbitpos = bitpos + big_endian_correction;
!
! store_bit_field (reg, bitsize, xbitpos, word_mode,
! extract_bit_field (word, bitsize, bitpos, 1,
! NULL_RTX, word_mode,
! word_mode,
! bitsize / BITS_PER_UNIT,
! BITS_PER_WORD),
! bitsize / BITS_PER_UNIT, BITS_PER_WORD);
}
}
}
--- 1784,1841 ----
args[i].aligned_regs = (rtx *) alloca (sizeof (rtx)
* args[i].n_aligned_regs);
+ #if 0
/* Structures smaller than 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)
big_endian_correction = (BITS_PER_WORD - (bytes * BITS_PER_UNIT));
! #else
! /* Do alignment padding according to FUNCTION_ARG_PADDING. */
! if (FUNCTION_ARG_PADDING (args[i].mode,
! TREE_TYPE (args[i].tree_value))
! == downward)
! big_endian_correction
! = - (unsigned) (bytes * BITS_PER_UNIT) % PARM_BOUNDARY;
! #endif
!
! /* Clobber each REG and move each partword into it. Ensure we
! don't go past the end of the structure.
!
! We used to emit a clobber for each REG here but that doesn't
! let later passes optimize the instructions we emit. By storing
! 0 into the register later passes know the first AND to zero out
! the bitfield being set in the register is unnecessary. The
! store of 0 will be deleted as will at least the first AND. */
!
! for (j = 0, k = 0, bitpos = BITS_PER_WORD,
! xbitpos = BITS_PER_WORD + big_endian_correction;
! bytes > 0;
! bitpos += bitsize, xbitpos += bitsize,
! bytes -= bitsize / BITS_PER_UNIT)
{
! if (bitpos >= BITS_PER_WORD)
{
! word = operand_subword_force (args[i].value, k++, BLKmode);
! bitpos = 0;
! }
! if (xbitpos >= BITS_PER_WORD)
! {
! if (j >= args[i].n_aligned_regs)
! break;
! reg = gen_reg_rtx (word_mode);
! args[i].aligned_regs[j++] = reg;
! emit_move_insn (reg, const0_rtx);
! xbitpos -= BITS_PER_WORD;
}
+ store_bit_field (reg, bitsize, xbitpos, word_mode,
+ extract_bit_field (word, bitsize, bitpos, 1,
+ NULL_RTX, word_mode,
+ word_mode,
+ bitsize / BITS_PER_UNIT,
+ BITS_PER_WORD),
+ bitsize / BITS_PER_UNIT, BITS_PER_WORD);
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Applying FUNCTION_ARG_PADDING to register arguments
1997-10-23 12:49 Applying FUNCTION_ARG_PADDING to register arguments Joern Rennecke
@ 1997-10-23 17:28 ` Jim Wilson
1997-10-24 6:41 ` Joern Rennecke
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jim Wilson @ 1997-10-23 17:28 UTC (permalink / raw)
To: Joern Rennecke; +Cc: egcs
While tracking down a varargs bug, I found that calls.c doesn't respect
FUNCTION_ARG_PADDING for register arguments. I think that is a bug.
I agree. However, the problem is not quite this simple to fix, because adding
code that uses FUNCTION_ARG_PADDING may break the compiler for other targets
if FUNCTION_ARG_PADDING happens to be defined wrong for this case. This
should at least be checked. This particular code is used for the PA and
Irix6 ports. I suggest you test them with your patch.
The irix6 port has a related problem. I wrote up a description of
this once so I could discuss the problem with Kenner, but never had time
to finish the work I started. My message follows. I don't claim that
everything in it is correct, but there might be some useful info in it.
--------------------
I am trying to fix some problems with the irix6 calling convention support.
Gcc is emitting code that is incompatible with the ABI and the SGI C compiler.
The ABI specifies that structure arguments and return values are always
left aligned. This is not how gcc traditionally handles structures in
registers, and fixing it gets to be very tricky.
There is a similar problem with the standard mips ABI. However, there
are fewer cases that need to be dealt with. It handles the left alignment of
struct arguments smaller than a word by using a function_arg trick to cause
logical left shift instructions to be emitted when calling a function, and
logical right shift instructions to be emitted in function prologues. This
trick can not be easily extended to handle all irix6 problems.
I have a tentative patch, but it needs cleaning up. Due to the significant
amount of time it will take to clean it up and test it, I am looking for
general guidance as to what you are willing to accept.
The right way to fix this problem seems to be to use FUNCTION_ARG_PADDING.
Setting the padding to upward for a big-endian machine forces left alignment.
However, there are three problems here.
The first problem is that MUST_PASS_IN_STACK macro in expr.h forces BLKmode
arguments to be passed in the stack if the padding is opposite the natural
direction. This is wrong for irix6. In general, MUST_PASS_IN_STACK should
not be overriding FUNCTION_ARG if FUNCTION_ARG says that something is only
passed in registers. This seems to be a relic left over from GCC 1.x. I
suspect that other ports may also be generating incorrect (non ABI compliant)
code because of this. For instance, I noticed that the standard MIPS ABI
support suffers from this bug also. However, at this point, it seems unsafe to
just delete this macro, because that will introduce unexpected and unnecessary
incompatibilites for targets where gcc is the only compiler. Providing a
way for tm.h files to override MUST_PASS_IN_STACK seems the safest way for
now. For now I have just modified expr.h but I don't think this is safe.
The second problem is that there are many places which do not use
FUNCTION_ARG_PADDING when loading arguments. It is not used when loading
non-BLKmode arguments into registers. It is not used when loading structure
arguments with insufficient alignment into registers. The first case exists
because gcc will change structure modes from BLKmode to an integer mode
if it is sufficiently aligned. This problem is worked around in the standard
mips ABI via the function_arg trick. It may make sense to add code to
emit_calls which checks the padding, and then emits a left-shift if necessary.
However, I am concerned that an attempt at an elegant solution may break
other ports that don't define FUNCTION_ARG_PADDING correctly.
For now, I am handling this via the function_arg hack.
The second case concerns code that is only used by the PA and MIPS ports,
so it should be safe to change it to use FUNCTION_ARG_PADDING. I have
an example of this in my patch.
The third problem is that FUNCTION_ARG_PADDING is not used when returning
values. It seems a bit odd to try to use it here, but it will do the
right thing for the Irix 6 port. Perhaps we should create a new macro?
As above, there are two cases to be dealt with, the `unaligned' structure
return case, and the regular case. I have a change for the `unaligned' return
case in my patch. The regular case I am handling via a function_epilogue
hack. As above, I have concerns about breaking other ports
that don't define FUNCTION_ARG_PADDING correctly, but this is not a problem
if we create a new macro, which would then only be used by the Irix 6 port.
The following patch seems to work, but is not very elegant. I have tested
it for irix6 and for the PA (for the `unaligned' structure arguments in
registers case) against a number of small testcases, but it needs more
extensive testing.
diff -pr clean-ss-961219/calls.c ss-961219/calls.c
*** clean-ss-961219/calls.c Thu Dec 12 16:46:42 1996
--- ss-961219/calls.c Fri Dec 27 15:06:25 1996
*************** expand_call (exp, target, ignore)
*** 1763,1773 ****
args[i].aligned_regs = (rtx *) alloca (sizeof (rtx)
* args[i].n_aligned_regs);
! /* Structures smaller than 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)
big_endian_correction = (BITS_PER_WORD - (bytes * BITS_PER_UNIT));
for (j = 0; j < args[i].n_aligned_regs; j++)
--- 1763,1776 ----
args[i].aligned_regs = (rtx *) alloca (sizeof (rtx)
* args[i].n_aligned_regs);
! /* 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 arguments are padded downwards. */
! if (BYTES_BIG_ENDIAN && (bytes % UNITS_PER_WORD)
! && (FUNCTION_ARG_PADDING (TYPE_MODE (TREE_TYPE (args[i].tree_value)),
! TREE_TYPE (args[i].tree_value))
! == downward))
big_endian_correction = (BITS_PER_WORD - (bytes * BITS_PER_UNIT));
for (j = 0; j < args[i].n_aligned_regs; j++)
diff -pr clean-ss-961219/config/mips/abi64.h ss-961219/config/mips/abi64.h
*** clean-ss-961219/config/mips/abi64.h Tue Oct 22 15:23:24 1996
--- ss-961219/config/mips/abi64.h Fri Dec 27 12:54:15 1996
*************** Boston, MA 02111-1307, USA. */
*** 77,88 ****
#define FUNCTION_ARG_PADDING(MODE, TYPE) \
(! BYTES_BIG_ENDIAN \
? upward \
! : (((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 \
! && (mips_abi == ABI_32 || mips_abi == ABI_EABI \
! || GET_MODE_CLASS (MODE) == MODE_INT))) \
? downward : upward))
#undef RETURN_IN_MEMORY
--- 77,103 ----
#define FUNCTION_ARG_PADDING(MODE, TYPE) \
(! BYTES_BIG_ENDIAN \
? upward \
! : (mips_abi == ABI_32 || mips_abi == ABI_EABI) \
! /* This is the default definition. */ \
! ? (((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) \
! /* This is the ABI_N32/ABI_64 definition. Structures are always \
! left aligned. This only matters for structures which are not \
! a multiple of 64 bits. Integers smaller than 64 bits are right \
! aligned. Floats smaller than 64 bits are left aligned. Otherwise \
! align the same way as the default. See the comment in function_arg \
! about left-adjusting of parameters for non-BLKmode structures. */ \
! : ((((MODE) == BLKmode \
! || ((TYPE) && (TREE_CODE (TYPE) == RECORD_TYPE \
! || TREE_CODE (TYPE) == UNION_TYPE))) \
! ? ((TYPE) && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
! && (int_size_in_bytes (TYPE) \
! % (PARM_BOUNDARY / BITS_PER_UNIT) == 0)) \
: (GET_MODE_BITSIZE (MODE) < PARM_BOUNDARY \
! && GET_MODE_CLASS (MODE) != MODE_FLOAT)) \
? downward : upward))
#undef RETURN_IN_MEMORY
diff -pr clean-ss-961219/config/mips/mips.c ss-961219/config/mips/mips.c
*** clean-ss-961219/config/mips/mips.c Thu Dec 12 17:27:46 1996
--- ss-961219/config/mips/mips.c Fri Dec 27 15:54:58 1996
*************** function_arg (cum, mode, type, named)
*** 3267,3273 ****
calling convention for now. */
if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
! && ! TARGET_64BIT && mips_abi != ABI_EABI)
{
rtx amount = GEN_INT (BITS_PER_WORD
- int_size_in_bytes (type) * BITS_PER_UNIT);
--- 3267,3274 ----
calling convention for now. */
if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
! && (mips_abi != ABI_32 || ! TARGET_64BIT) && mips_abi != ABI_EABI
! && (mips_abi == ABI_32 || mode != BLKmode))
{
rtx amount = GEN_INT (BITS_PER_WORD
- int_size_in_bytes (type) * BITS_PER_UNIT);
*************** function_epilogue (file, size)
*** 5547,5552 ****
--- 5548,5563 ----
if (!noepilogue)
{
+ /* Must left shift the return value if it is a structure with an integral
+ mode, and we are emitting code for the N32 or N64 ABIs. */
+ tree retval_type = TREE_TYPE (TREE_TYPE (current_function_decl));
+ if (TYPE_MODE (retval_type) != BLKmode
+ && AGGREGATE_TYPE_P (retval_type)
+ && int_size_in_bytes (retval_type) < UNITS_PER_WORD
+ && mips_abi != ABI_32 && mips_abi != ABI_EABI)
+ fprintf (file, "\tdsll\t$2,$2,%d\n",
+ BITS_PER_WORD - int_size_in_bytes (retval_type) * BITS_PER_UNIT);
+
/* In the reload sequence, we don't need to fill the load delay
slots for most of the loads, also see if we can fill the final
delay slot if not otherwise filled by the reload sequence. */
*************** mips_expand_epilogue ()
*** 5732,5737 ****
--- 5743,5764 ----
long tsize = current_frame_info.total_size;
rtx tsize_rtx = GEN_INT (tsize);
rtx tmp_rtx = (rtx)0;
+ tree retval_type;
+
+ /* Must left shift the return value if it is a structure with an integral
+ mode, and we are emitting code for the N32 or N64 ABIs. */
+ retval_type = TREE_TYPE (TREE_TYPE (current_function_decl));
+ if (TYPE_MODE (retval_type) != BLKmode
+ && AGGREGATE_TYPE_P (retval_type)
+ && int_size_in_bytes (retval_type) < UNITS_PER_WORD
+ && mips_abi != ABI_32 && mips_abi != ABI_EABI)
+ {
+ /* TARGET_64BIT is assumed to be true here. */
+ rtx retval = gen_rtx (REG, DImode, 2);
+ rtx amount = GEN_INT (BITS_PER_WORD
+ - int_size_in_bytes (retval_type) * BITS_PER_UNIT);
+ emit_insn (gen_ashldi3 (retval, retval, amount));
+ }
if (tsize > 32767)
{
diff -pr clean-ss-961219/expr.h ss-961219/expr.h
*** clean-ss-961219/expr.h Tue Oct 22 21:15:54 1996
--- ss-961219/expr.h Fri Dec 27 13:09:55 1996
*************** enum direction {none, upward, downward};
*** 207,219 ****
#define MUST_PASS_IN_STACK(MODE,TYPE) \
((TYPE) != 0 \
&& (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
! || TREE_ADDRESSABLE (TYPE) \
! || ((MODE) == BLKmode \
! && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
! && 0 == (int_size_in_bytes (TYPE) \
! % (PARM_BOUNDARY / BITS_PER_UNIT))) \
! && (FUNCTION_ARG_PADDING (MODE, TYPE) \
! == (BYTES_BIG_ENDIAN ? upward : downward)))))
/* Nonzero if type TYPE should be returned in memory.
Most machines can use the following default definition. */
--- 207,213 ----
#define MUST_PASS_IN_STACK(MODE,TYPE) \
((TYPE) != 0 \
&& (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
! || TREE_ADDRESSABLE (TYPE)))
/* Nonzero if type TYPE should be returned in memory.
Most machines can use the following default definition. */
diff -pr clean-ss-961219/ginclude/va-mips.h ss-961219/ginclude/va-mips.h
*** clean-ss-961219/ginclude/va-mips.h Tue Oct 22 15:26:29 1996
--- ss-961219/ginclude/va-mips.h Fri Dec 27 14:39:20 1996
*************** void va_end (__gnuc_va_list); /* Define
*** 234,248 ****
right aligned). */
#ifdef __mips64
#ifdef __MIPSEB__
#define va_arg(__AP, __type) \
((__type *) (void *) (__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ __va_rounded_size (__type))))[-1]
! #else
#define va_arg(__AP, __type) \
((__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ __va_rounded_size (__type))), \
*(__type *) (void *) (__AP - __va_rounded_size (__type)))
! #endif
#else /* not __mips64 */
--- 234,271 ----
right aligned). */
#ifdef __mips64
#ifdef __MIPSEB__
+
+ #if defined(_MIPS_SIM) && (_MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32)
+ /* For ABI_N32 and ABI_64, structures are left aligned, integers smaller
+ than a word are right aligned, and floats smaller than a word are left
+ aligned. We can ignore the float case for now, since floats must always
+ be promoted to double. */
+ #define va_arg(__AP, __type) \
+ ((__builtin_classify_type (* (__type *) 0) == __record_type_class \
+ || __builtin_classify_type (* (__type *) 0) == __union_type_class) \
+ ? ((__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ + __va_rounded_size (__type))), \
+ *(__type *) (void *) (__AP - __va_rounded_size (__type))) \
+ : ((__type *) (void *) \
+ (__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ + __va_rounded_size (__type))))[-1])
+ #else /* ! N32/64 ABI */
#define va_arg(__AP, __type) \
((__type *) (void *) (__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ __va_rounded_size (__type))))[-1]
! #endif /* N32/64 ABI */
!
! #else /* ! __MIPSEB__ */
!
! #if defined(_MIPS_SIM) && (_MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32)
! #error va_arg is not defined for little-endian ABI_N32/ABI_64
! #endif
!
#define va_arg(__AP, __type) \
((__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ __va_rounded_size (__type))), \
*(__type *) (void *) (__AP - __va_rounded_size (__type)))
! #endif /* ! __MIPSEB__ */
#else /* not __mips64 */
diff -pr clean-ss-961219/stmt.c ss-961219/stmt.c
*** clean-ss-961219/stmt.c Wed Oct 30 12:50:39 1996
--- ss-961219/stmt.c Fri Dec 27 15:29:04 1996
*************** expand_return (retval)
*** 2847,2857 ****
rtx result_val = expand_expr (retval_rhs, NULL_RTX, VOIDmode, 0);
enum machine_mode tmpmode, result_reg_mode;
! /* 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)
big_endian_correction = (BITS_PER_WORD - ((bytes % UNITS_PER_WORD)
* BITS_PER_UNIT));
--- 2847,2860 ----
rtx result_val = expand_expr (retval_rhs, NULL_RTX, VOIDmode, 0);
enum machine_mode tmpmode, result_reg_mode;
! /* 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 arguments are padded downwards. */
! if (BYTES_BIG_ENDIAN && (bytes % UNITS_PER_WORD)
! && (FUNCTION_ARG_PADDING (TYPE_MODE (TREE_TYPE (retval_rhs)),
! TREE_TYPE (retval_rhs))
! == downward))
big_endian_correction = (BITS_PER_WORD - ((bytes % UNITS_PER_WORD)
* BITS_PER_UNIT));
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Applying FUNCTION_ARG_PADDING to register arguments
1997-10-23 17:28 ` Jim Wilson
@ 1997-10-24 6:41 ` Joern Rennecke
1997-10-24 7:45 ` Jeffrey A Law
1997-10-24 7:43 ` Jeffrey A Law
1997-10-28 12:35 ` Joern Rennecke
2 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 1997-10-24 6:41 UTC (permalink / raw)
To: Jim Wilson; +Cc: egcs
> The first problem is that MUST_PASS_IN_STACK macro in expr.h forces BLKmode
> arguments to be passed in the stack if the padding is opposite the natural
> direction. This is wrong for irix6. In general, MUST_PASS_IN_STACK should
> not be overriding FUNCTION_ARG if FUNCTION_ARG says that something is only
> passed in registers. This seems to be a relic left over from GCC 1.x. I
> suspect that other ports may also be generating incorrect (non ABI compliant)
> code because of this. For instance, I noticed that the standard MIPS ABI
> support suffers from this bug also. However, at this point, it seems unsafe to
> just delete this macro, because that will introduce unexpected and unnecessary
> incompatibilites for targets where gcc is the only compiler. Providing a
> way for tm.h files to override MUST_PASS_IN_STACK seems the safest way for
> now. For now I have just modified expr.h but I don't think this is safe.
Actually this modification would be sufficient to solve the problem I've
originally set out to fix.
Thus, I think the way to go is to change MUST_PASS_IN_STACK in expr.h not
to take padding into account (like in your patch), and make the definition
conditional on #ifndef MUST_PASS_IN_STACK. The targets that actually want
this double argument passing can then define MUST_PASS_IN_STACK to be
identical to the old definition.
I have gone through the 35 configurations in egcs, and found that
for most of them, this change would have no effect, due to their
endianness / definition of FUNCTION_ARG_PADDING, definition of FUNCTION_ARG,
or in the case of dsp16xx, BITS_PER_UNIT being ideintical to PARM_BOUNDARY.
For the SH, the proposed change would be a bugfix. So that leaves
nine targets where I'm not sure if we'd like to define MUST_PASS_IN_STACK
in the old way:
arc
arm
h8300 (problem only for TARGET_QUICKCALL, but still...)
m32r
mips
pa
pyr
romp
sparc
If we can't resolve the issue for one or more of these targets in a timely
manner, we can just put in the old definition of MUST_PASS_IN_STACK into
the respective tm.h file, with a comment that it is unknown if that is
actually intentional, but kept for backward compatibility.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Applying FUNCTION_ARG_PADDING to register arguments
1997-10-23 17:28 ` Jim Wilson
1997-10-24 6:41 ` Joern Rennecke
@ 1997-10-24 7:43 ` Jeffrey A Law
1997-10-28 12:35 ` Joern Rennecke
2 siblings, 0 replies; 6+ messages in thread
From: Jeffrey A Law @ 1997-10-24 7:43 UTC (permalink / raw)
To: Jim Wilson; +Cc: Joern Rennecke, egcs
In message < 199710240016.RAA27996@cygnus.com >you write:
> While tracking down a varargs bug, I found that calls.c doesn't respect
> FUNCTION_ARG_PADDING for register arguments. I think that is a bug.
>
> I agree. However, the problem is not quite this simple to fix, because adding
> code that uses FUNCTION_ARG_PADDING may break the compiler for other targets
> if FUNCTION_ARG_PADDING happens to be defined wrong for this case. This
> should at least be checked. This particular code is used for the PA and
> Irix6 ports. I suggest you test them with your patch.
>
> The irix6 port has a related problem. I wrote up a description of
> this once so I could discuss the problem with Kenner, but never had time
> to finish the work I started. My message follows. I don't claim that
> everything in it is correct, but there might be some useful info in it.
There is a long standing ABI bug on the PA with regards to padding
of structs. If I remember right, when a small struct is passed in
a reg it is incorrectly justified.
I believe this can be triggered by setting up a varargs/stdarg call
with a small structs as unnamed args. If you compile the caller
with gcc and the callee with HP C, then they'll disagree on the value
of the parameters (similarly if you switch compiler for caller/callee).
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Applying FUNCTION_ARG_PADDING to register arguments
1997-10-24 6:41 ` Joern Rennecke
@ 1997-10-24 7:45 ` Jeffrey A Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeffrey A Law @ 1997-10-24 7:45 UTC (permalink / raw)
To: Joern Rennecke; +Cc: Jim Wilson, egcs
In message < 199710241340.OAA22084@phal.cygnus.co.uk >you write:
> h8300 (problem only for TARGET_QUICKCALL, but still...)
Note, TARGET_QUICKCALL is the deafult for the H8.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Applying FUNCTION_ARG_PADDING to register arguments
1997-10-23 17:28 ` Jim Wilson
1997-10-24 6:41 ` Joern Rennecke
1997-10-24 7:43 ` Jeffrey A Law
@ 1997-10-28 12:35 ` Joern Rennecke
2 siblings, 0 replies; 6+ messages in thread
From: Joern Rennecke @ 1997-10-28 12:35 UTC (permalink / raw)
To: Jim Wilson; +Cc: egcs
> The first problem is that MUST_PASS_IN_STACK macro in expr.h forces BLKmode
> arguments to be passed in the stack if the padding is opposite the natural
> direction. This is wrong for irix6. In general, MUST_PASS_IN_STACK should
> not be overriding FUNCTION_ARG if FUNCTION_ARG says that something is only
> passed in registers. This seems to be a relic left over from GCC 1.x. I
> suspect that other ports may also be generating incorrect (non ABI compliant)
> code because of this. For instance, I noticed that the standard MIPS ABI
> support suffers from this bug also. However, at this point, it seems unsafe to
> just delete this macro, because that will introduce unexpected and unnecessary
> incompatibilites for targets where gcc is the only compiler. Providing a
> way for tm.h files to override MUST_PASS_IN_STACK seems the safest way for
> now. For now I have just modified expr.h but I don't think this is safe.
I have made a patch now that changes MUST_PASS_IN_STACK in expr.h, and
sets it back in all egcs ports where it matters, except for the SH, where
I know that the old behaviour was reason for a bug. I think this should be
safe to install; after that, the individual port maintaines can sort out
what they want to do with MUST_PASS_IN_STACK.
Tue Oct 28 17:53:05 1997 J"orn Rennecke <amylaar@cygnus.co.uk>
* expr.h (MUST_PASS_IN_STACK): Don't care about padding.
Don't define if already defined.
* a29k, arc.h, arm.h, h8300.h, m32r.h (MUST_PASS_IN_STACK): Redefine.
* mips.h, pa.h, pyr.h, romp.h, sparc.h (MUST_PASS_IN_STACK): Redefine.
Index: expr.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/expr.h,v
retrieving revision 1.4
diff -p -r1.4 expr.h
*** expr.h 1997/08/29 15:02:03 1.4
--- expr.h 1997/10/28 17:44:58
*************** enum direction {none, upward, downward};
*** 206,231 ****
- if the type has variable size
- if the type is marked as addressable (it is required to be constructed
into the stack)
! - if the padding and mode of the type is such that a copy into a register
would put it into the wrong part of the register.
! Which padding can't be supported depends on the byte endianness.
A value in a register is implicitly padded at the most significant end.
On a big-endian machine, that is the lower end in memory.
! So a value padded in memory at the upper end can't go in a register.
! For a little-endian machine, the reverse is true. */
#define MUST_PASS_IN_STACK(MODE,TYPE) \
((TYPE) != 0 \
&& (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
! || TREE_ADDRESSABLE (TYPE) \
! || ((MODE) == BLKmode \
! && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
! && 0 == (int_size_in_bytes (TYPE) \
! % (PARM_BOUNDARY / BITS_PER_UNIT))) \
! && (FUNCTION_ARG_PADDING (MODE, TYPE) \
! == (BYTES_BIG_ENDIAN ? upward : downward)))))
/* Nonzero if type TYPE should be returned in memory.
Most machines can use the following default definition. */
--- 206,234 ----
- if the type has variable size
- if the type is marked as addressable (it is required to be constructed
into the stack)
!
! There used to be also a padding constraint, but this is no longer
! true:
! - if the padding and mode of the type was such that a copy into a register
would put it into the wrong part of the register.
! Which padding couldn't be supported depended on the byte endianness.
A value in a register is implicitly padded at the most significant end.
On a big-endian machine, that is the lower end in memory.
! So a value padded in memory at the upper end couldn't go in a register.
! For a little-endian machine, the reverse was true.
!
! Now, we can store values that come in registers with inconvenient padding
! to memory at the function start. That is important, because some ABIs
! require such register argument passing. */
+ #ifndef MUST_PASS_IN_STACK
#define MUST_PASS_IN_STACK(MODE,TYPE) \
((TYPE) != 0 \
&& (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
! || TREE_ADDRESSABLE (TYPE)))
! #endif
/* Nonzero if type TYPE should be returned in memory.
Most machines can use the following default definition. */
Index: config/arc/arc.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/arc/arc.h,v
retrieving revision 1.1
diff -p -r1.1 arc.h
*** arc.h 1997/09/10 07:50:29 1.1
--- arc.h 1997/10/28 17:45:01
*************** enum arc_function_type {
*** 1641,1643 ****
--- 1641,1659 ----
((TYPE) == ARC_FUNCTION_ILINK1 || (TYPE) == ARC_FUNCTION_ILINK2)
/* Compute the type of a function from its DECL. */
enum arc_function_type arc_compute_function_type ();
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/arm/arm.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/arm/arm.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 arm.h
*** arm.h 1997/08/11 15:57:24 1.1.1.1
--- arm.h 1997/10/28 17:45:04
*************** void aof_add_import (/* char * */);
*** 2047,2049 ****
--- 2047,2065 ----
void aof_delete_import (/* char * */);
void aof_dump_imports (/* FILE * */);
#endif
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/h8300/h8300.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/h8300/h8300.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 h8300.h
*** h8300.h 1997/08/11 15:57:43 1.1.1.1
--- h8300.h 1997/10/28 17:45:07
*************** int h8300_funcvec_function_p ();
*** 1397,1399 ****
--- 1397,1414 ----
char *output_adds_subs ();
char * output_simode_bld ();
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/m32r/m32r.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/m32r/m32r.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 m32r.h
*** m32r.h 1997/08/11 15:57:52 1.1.1.1
--- m32r.h 1997/10/28 17:45:10
*************** enum m32r_function_type {
*** 1865,1867 ****
--- 1865,1883 ----
((TYPE) == M32R_FUNCTION_INTERRUPT)
/* Compute the type of a function from its DECL. */
enum m32r_function_type m32r_compute_function_type ();
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/mips/mips.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/mips/mips.h,v
retrieving revision 1.3
diff -p -r1.3 mips.h
*** mips.h 1997/09/18 00:31:51 1.3
--- mips.h 1997/10/28 17:45:16
*************** while (0)
*** 4046,4048 ****
--- 4046,4064 ----
true. */
#define DONT_ACCESS_GBLS_AFTER_EPILOGUE (TARGET_ABICALLS && mips_abi != ABI_32)
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/pa/pa.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/pa/pa.h,v
retrieving revision 1.4
diff -p -r1.4 pa.h
*** pa.h 1997/08/25 15:00:44 1.4
--- pa.h 1997/10/28 17:45:28
*************** do { \
*** 2422,2424 ****
--- 2422,2440 ----
/* The number of Pmode words for the setjmp buffer. */
#define JMP_BUF_SIZE 50
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/pyr/pyr.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/pyr/pyr.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 pyr.h
*** pyr.h 1997/08/11 15:57:38 1.1.1.1
--- pyr.h 1997/10/28 17:45:30
*************** extern int swap_operands;
*** 1503,1505 ****
--- 1503,1521 ----
output_addr_const (FILE, addr); \
} \
}
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/romp/romp.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/romp/romp.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 romp.h
*** romp.h 1997/08/11 15:57:21 1.1.1.1
--- romp.h 1997/10/28 17:45:33
*************** extern int romp_debugger_arg_correction(
*** 1634,1636 ****
--- 1634,1652 ----
extern char *output_in_line_mul ();
extern char *output_fpop ();
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/sparc/sparc.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/sparc/sparc.h,v
retrieving revision 1.5
diff -p -r1.5 sparc.h
*** sparc.h 1997/10/10 22:33:24 1.5
--- sparc.h 1997/10/28 17:45:39
*************** extern char *output_return ();
*** 3199,3201 ****
--- 3199,3217 ----
/* Defined in flags.h, but insn-emit.c does not include flags.h. */
extern int flag_pic;
+
+ /* This is defined here for strict compatibility with the previous
+ definition in expr.h. It should probably be checked by someone
+ who knows what the ABI for this target should exactly be. */
+
+ #undef MUST_PASS_IN_STACK
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
Index: config/a29k/a29k.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/a29k/a29k.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 a29k.h
*** a29k.h 1997/08/11 15:57:20 1.1.1.1
--- a29k.h 1997/10/28 17:45:42
*************** extern struct rtx_def *a29k_get_reloaded
*** 764,769 ****
--- 764,786 ----
#define FUNCTION_ARG_REGNO_P(N) ((N) <= R_LR (17) && (N) >= R_LR (2))
\f
+ /* The definition of MUST_PASS_IN_STACK here is solely for the benefit
+ of the macros in this file itself; since the a29k FUNCTION_ARG will
+ never return a register when MUST_PASS_IN_STACK is false, there is
+ no problem with possible double argument passing in registers and
+ stack. */
+
+ #define MUST_PASS_IN_STACK(MODE,TYPE) \
+ ((TYPE) != 0 \
+ && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST \
+ || TREE_ADDRESSABLE (TYPE) \
+ || ((MODE) == BLKmode \
+ && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
+ && 0 == (int_size_in_bytes (TYPE) \
+ % (PARM_BOUNDARY / BITS_PER_UNIT))) \
+ && (FUNCTION_ARG_PADDING (MODE, TYPE) \
+ == (BYTES_BIG_ENDIAN ? upward : downward)))))
+
/* Define a data type for recording info about an argument list
during the scan of that argument list. This data type should
hold all necessary information about the function itself
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~1997-10-28 12:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1997-10-23 12:49 Applying FUNCTION_ARG_PADDING to register arguments Joern Rennecke
1997-10-23 17:28 ` Jim Wilson
1997-10-24 6:41 ` Joern Rennecke
1997-10-24 7:45 ` Jeffrey A Law
1997-10-24 7:43 ` Jeffrey A Law
1997-10-28 12:35 ` Joern Rennecke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).