* [PATCH] Fix struct passing on targets that rely on TYPE in BLOCK_REG_PADDING if mode is not BLKmode (PR middle-end/37316)
@ 2008-10-23 13:19 Jakub Jelinek
2008-10-27 7:58 ` [PATCH] Fix struct passing on targets that rely on TYPE in John David Anglin
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2008-10-23 13:19 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, John David Anglin
Hi!
Daniel's 2008-07-0{1,7} function.c changes added
assign_parm_remove_parallels call that replaced some code in
assign_parm_setup_block. Previously, emit_group_store in
assign_parm_setup_block was being passed 3 different types
in different places.
For non-BLKmode parallels with more than one hard reg containing
the parameter and use_register_for_decl (parm) it used:
if (data->nominal_mode != data->passed_mode)
{
rtx t = gen_reg_rtx (GET_MODE (entry_parm));
emit_group_store (t, entry_parm, NULL_TREE,
GET_MODE_SIZE (GET_MODE (entry_parm)));
convert_move (parmreg, t, 0);
}
else
emit_group_store (parmreg, entry_parm, data->nominal_type,
int_size_in_bytes (data->nominal_type));
and otherwise:
emit_group_store (mem, entry_parm, data->passed_type, size);
The last call remains in assign_parm_setup_block and
assign_parm_remove_parallels handles the non-BLKmode cases only,
but it passes NULL_TREE as type to emit_group_store.
On targets that look at type in BLOCK_REG_PADDING macro
even for non-BLKmode mode this means (or could mean) an ABI change.
On the calls.c side, emit_group_load* is always passed
TREE_TYPE (arg->tree_value), except for libcalls where NULL_TREE
is passed. I believe this corresponds to passed_type, so I'd say
we should use data->passed_type always (and it certainly fixes the
testcase referenced in the PR). After all, we should care about
the type in which the arguments is passed, what type is used in the
rest of the function should be irrelevant for how it is padded in the
hard registers.
I've bootstrapped/regtested this on x86_64-linux, which doesn't say
much, given that emit_group_store doesn't care about type if
BLOCK_REG_PADDING isn't defined. BLOCK_REG_PADDING is
defined on arm, mips, pa and some powerpc variants, but
apparently on powerpc64-linux I have access to type is only looked
at if mode is BLKmode. On TARGET_AAPCS_BASED big endian ARM
type is dereferenced unconditionally, so if it works at all, it
can't hit this case (i.e. can't use partial regs in PARALLELs),
on other ARMs it is ignored. So the only places where this
patch could make a difference is:
1) hppa*-*
2) powerpc*-aix* (AGGREGATES_PAD_UPWARD_ALWAYS 1)
3) maybe mips*-*
Unfortunately, I have access to none of these, could somebody please
bootstrap/regtest this there? It can affect only the callee
side, not caller, so if there is a problem, it would show up as
an ABI incompatibility between caller and callee.
2008-10-23 Jakub Jelinek <jakub@redhat.com>
PR middle-end/37316
* function.c (assign_parm_remove_parallels): Pass
data->passed_type as third argument to emit_group_store.
--- gcc/function.c.jj 2008-10-23 09:07:04.000000000 +0200
+++ gcc/function.c 2008-10-23 10:52:08.000000000 +0200
@@ -2436,7 +2436,7 @@ assign_parm_remove_parallels (struct ass
if (GET_CODE (entry_parm) == PARALLEL && GET_MODE (entry_parm) != BLKmode)
{
rtx parmreg = gen_reg_rtx (GET_MODE (entry_parm));
- emit_group_store (parmreg, entry_parm, NULL_TREE,
+ emit_group_store (parmreg, entry_parm, data->passed_type,
GET_MODE_SIZE (GET_MODE (entry_parm)));
entry_parm = parmreg;
}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix struct passing on targets that rely on TYPE in
2008-10-23 13:19 [PATCH] Fix struct passing on targets that rely on TYPE in BLOCK_REG_PADDING if mode is not BLKmode (PR middle-end/37316) Jakub Jelinek
@ 2008-10-27 7:58 ` John David Anglin
2008-10-27 8:43 ` Daniel Jacobowitz
2008-10-27 9:57 ` Jakub Jelinek
0 siblings, 2 replies; 4+ messages in thread
From: John David Anglin @ 2008-10-27 7:58 UTC (permalink / raw)
To: jakub; +Cc: ebotcazou, gcc-patches
I have committed the following change to resolve PR 37316. The change avoids
the problematic call to emit_group_store in assign_parm_remove_parallels by
using BLKmode for the PARALLEL used for structs, aggregates and other
similar objects. It also fixes the incorrect justification noted in
debugging this problem for complex and vector types. The change doesn't
fix the middle-end regression.
Tested on hppa64-hp-hpux11.11. Committed to trunk.
Dave
--
J. David Anglin dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
2008-10-26 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>
PR middle-end/37316
* pa.c (function_arg_padding): Pad complex and vector types upward in
64-bit runtime.
(function_arg): Use BLKmode for PARALLEL in 64-bit runtime.
Index: config/pa/pa.c
===================================================================
--- config/pa/pa.c (revision 141361)
+++ config/pa/pa.c (working copy)
@@ -5888,7 +5888,11 @@
function_arg_padding (enum machine_mode mode, const_tree type)
{
if (mode == BLKmode
- || (TARGET_64BIT && type && AGGREGATE_TYPE_P (type)))
+ || (TARGET_64BIT
+ && type
+ && (AGGREGATE_TYPE_P (type)
+ || TREE_CODE (type) == COMPLEX_TYPE
+ || TREE_CODE (type) == VECTOR_TYPE)))
{
/* Return none if justification is not required. */
if (type
@@ -9277,7 +9281,7 @@
offset += 8;
}
- return gen_rtx_PARALLEL (mode, gen_rtvec_v (ub, loc));
+ return gen_rtx_PARALLEL (BLKmode, gen_rtvec_v (ub, loc));
}
}
else
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix struct passing on targets that rely on TYPE in
2008-10-27 7:58 ` [PATCH] Fix struct passing on targets that rely on TYPE in John David Anglin
@ 2008-10-27 8:43 ` Daniel Jacobowitz
2008-10-27 9:57 ` Jakub Jelinek
1 sibling, 0 replies; 4+ messages in thread
From: Daniel Jacobowitz @ 2008-10-27 8:43 UTC (permalink / raw)
To: John David Anglin; +Cc: jakub, ebotcazou, gcc-patches
On Sun, Oct 26, 2008 at 09:26:25PM -0400, John David Anglin wrote:
> I have committed the following change to resolve PR 37316. The change avoids
> the problematic call to emit_group_store in assign_parm_remove_parallels by
> using BLKmode for the PARALLEL used for structs, aggregates and other
> similar objects. It also fixes the incorrect justification noted in
> debugging this problem for complex and vector types. The change doesn't
> fix the middle-end regression.
>
> Tested on hppa64-hp-hpux11.11. Committed to trunk.
Thank you (and Jakub) for your persistence with this.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix struct passing on targets that rely on TYPE in
2008-10-27 7:58 ` [PATCH] Fix struct passing on targets that rely on TYPE in John David Anglin
2008-10-27 8:43 ` Daniel Jacobowitz
@ 2008-10-27 9:57 ` Jakub Jelinek
1 sibling, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2008-10-27 9:57 UTC (permalink / raw)
To: John David Anglin; +Cc: ebotcazou, gcc-patches
On Sun, Oct 26, 2008 at 09:26:25PM -0400, John David Anglin wrote:
> I have committed the following change to resolve PR 37316. The change avoids
> the problematic call to emit_group_store in assign_parm_remove_parallels by
> using BLKmode for the PARALLEL used for structs, aggregates and other
> similar objects. It also fixes the incorrect justification noted in
> debugging this problem for complex and vector types. The change doesn't
> fix the middle-end regression.
>
> Tested on hppa64-hp-hpux11.11. Committed to trunk.
>
> Dave
> --
> J. David Anglin dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
>
> 2008-10-26 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>
>
> PR middle-end/37316
> * pa.c (function_arg_padding): Pad complex and vector types upward in
> 64-bit runtime.
> (function_arg): Use BLKmode for PARALLEL in 64-bit runtime.
>
> Index: config/pa/pa.c
> ===================================================================
> --- config/pa/pa.c (revision 141361)
> +++ config/pa/pa.c (working copy)
> @@ -5888,7 +5888,11 @@
> function_arg_padding (enum machine_mode mode, const_tree type)
> {
> if (mode == BLKmode
> - || (TARGET_64BIT && type && AGGREGATE_TYPE_P (type)))
> + || (TARGET_64BIT
> + && type
> + && (AGGREGATE_TYPE_P (type)
> + || TREE_CODE (type) == COMPLEX_TYPE
> + || TREE_CODE (type) == VECTOR_TYPE)))
> {
> /* Return none if justification is not required. */
> if (type
> @@ -9277,7 +9281,7 @@
> offset += 8;
> }
>
> - return gen_rtx_PARALLEL (mode, gen_rtvec_v (ub, loc));
> + return gen_rtx_PARALLEL (BLKmode, gen_rtvec_v (ub, loc));
I think this unnecessarily penalizes the code, forces going through memory
always. To work around the emit_group_store bug, if the
assign_parm_remove_parallels bug is fixed, you could IMHO just use BLKmode
for COMPLEX_TYPE.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-10-27 7:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-23 13:19 [PATCH] Fix struct passing on targets that rely on TYPE in BLOCK_REG_PADDING if mode is not BLKmode (PR middle-end/37316) Jakub Jelinek
2008-10-27 7:58 ` [PATCH] Fix struct passing on targets that rely on TYPE in John David Anglin
2008-10-27 8:43 ` Daniel Jacobowitz
2008-10-27 9:57 ` Jakub Jelinek
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).