* [Patch Darwin 2/2] fix PPC64 ABI
@ 2010-07-24 8:36 IainS
2010-07-24 9:46 ` Mike Stump
2010-07-24 10:13 ` Nathan Froyd
0 siblings, 2 replies; 10+ messages in thread
From: IainS @ 2010-07-24 8:36 UTC (permalink / raw)
To: GCC Patches; +Cc: mrs, David Edelsohn
Hi,
We need to deal with the fact that the darwin ppc64 ABI is defined by an
earlier version of gcc, with the property that it always applied
alignment
adjustments to the va-args (even for zero-sized types). The cheapest
way
to deal with this is to replicate the effect of the part of
std_gimplify_va_arg_expr ()
that carries out the align adjust, for the case of relevance.
I've made it such that there is no impact at all for non-darwin.
The impact for darwin is restricted to the single m64 ABI case.
OK for trunk and 4.5 when it re-opens?
Iain
gcc/
PR target/29090
* config/rs6000/rs6000.c (rs6000_gimplify_va_arg): Special-case the
Darwin64 ABI, for zero-sized objects.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 162457)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -8931,6 +8995,51 @@ rs6000_gimplify_va_arg (tree valist, tree type,
gi
return build_va_arg_indirect_ref (t);
}
+#if TARGET_MACHO
+ /* We need to deal with the fact that the darwin ppc64 ABI is
defined by an
+ earlier version of gcc, with the property that it always applied
alignment
+ adjustments to the va-args (even for zero-sized types). The
cheapest way
+ to deal with this is to replicate the effect of the part of
+ std_gimplify_va_arg_expr that carries out the align adjust, for
the case
+ of relevance.
+ We don't need to check for pass-by-reference because of the test
above.
+ We can return a simplifed answer, since we know there's no
offset to add. */
+
+ if (rs6000_darwin64_abi
+ && integer_zerop (TYPE_SIZE (type)))
+ {
+ unsigned HOST_WIDE_INT align, boundary;
+ tree valist_tmp = get_initialized_tmp_var (valist, pre_p, NULL);
+ align = PARM_BOUNDARY / BITS_PER_UNIT;
+ boundary = FUNCTION_ARG_BOUNDARY (TYPE_MODE (type), type);
+ if (boundary > MAX_SUPPORTED_STACK_ALIGNMENT)
+ boundary = MAX_SUPPORTED_STACK_ALIGNMENT;
+ boundary /= BITS_PER_UNIT;
+ if (boundary > align)
+ {
+ tree t ;
+ /* This updates arg ptr by the amount that would be necessary
+ to align the zero-sized (but not zero-alignment) item. */
+ t = build2 (MODIFY_EXPR, TREE_TYPE (valist), valist_tmp,
+ fold_build2 (POINTER_PLUS_EXPR,
+ TREE_TYPE (valist),
+ valist_tmp, size_int (boundary - 1)));
+ gimplify_and_add (t, pre_p);
+
+ t = fold_convert (sizetype, valist_tmp);
+ t = build2 (MODIFY_EXPR, TREE_TYPE (valist), valist_tmp,
+ fold_convert (TREE_TYPE (valist),
+ fold_build2 (BIT_AND_EXPR, sizetype, t,
+ size_int (-boundary))));
+ t = build2 (MODIFY_EXPR, TREE_TYPE (valist), valist, t);
+ gimplify_and_add (t, pre_p);
+ }
+ /* Since it is zero-sized there's no increment for the item
itself. */
+ valist_tmp = fold_convert (build_pointer_type (type),
valist_tmp);
+ return build_va_arg_indirect_ref (valist_tmp);
+ }
+#endif
+
if (DEFAULT_ABI != ABI_V4)
{
if (targetm.calls.split_complex_arg && TREE_CODE (type) ==
COMPLEX_TYPE)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 8:36 [Patch Darwin 2/2] fix PPC64 ABI IainS
@ 2010-07-24 9:46 ` Mike Stump
2010-07-24 10:04 ` IainS
2010-07-24 15:06 ` IainS
2010-07-24 10:13 ` Nathan Froyd
1 sibling, 2 replies; 10+ messages in thread
From: Mike Stump @ 2010-07-24 9:46 UTC (permalink / raw)
To: IainS; +Cc: GCC Patches, mrs, David Edelsohn
On Jul 24, 2010, at 1:36 AM, IainS wrote:
> We need to deal with the fact that the darwin ppc64 ABI is defined by an
> earlier version of gcc, with the property that it always applied alignment
> adjustments to the va-args (even for zero-sized types).
> OK for trunk and 4.5 when it re-opens?
I'm fine with this, David, any objections?
Also, as memory serves there is one more fix from last year that fixes the abi so that it is self consistent from last year. I think it fixes a failing struct abi testcase as I recall.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 9:46 ` Mike Stump
@ 2010-07-24 10:04 ` IainS
2010-07-24 15:06 ` IainS
1 sibling, 0 replies; 10+ messages in thread
From: IainS @ 2010-07-24 10:04 UTC (permalink / raw)
To: mrs; +Cc: GCC Patches, David Edelsohn
On 24 Jul 2010, at 10:45, Mike Stump wrote:
> On Jul 24, 2010, at 1:36 AM, IainS wrote:
>> We need to deal with the fact that the darwin ppc64 ABI is defined
>> by an
>> earlier version of gcc, with the property that it always applied
>> alignment
>> adjustments to the va-args (even for zero-sized types).
>
>> OK for trunk and 4.5 when it re-opens?
>
> I'm fine with this, David, any objections?
>
>
> Also, as memory serves there is one more fix from last year that
> fixes the abi so that it is self consistent from last year. I think
> it fixes a failing struct abi testcase as I recall.
hm.
All the test-cases from the PRs and gxx.dg/compat and gxx.dg/struct-
layout-1
now pass ... (less breakage to struct-by-value-1.c caused last month,
which I'm trying to track down).
The "RS6000_STRUCT_HACK" no longer seems necessary (but I might be
mistaken) ..
can you identify the PR, or Radar?
cheers,
Iain
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 9:46 ` Mike Stump
2010-07-24 10:04 ` IainS
@ 2010-07-24 15:06 ` IainS
2010-07-24 15:10 ` David Edelsohn
1 sibling, 1 reply; 10+ messages in thread
From: IainS @ 2010-07-24 15:06 UTC (permalink / raw)
To: mrs; +Cc: GCC Patches, David Edelsohn
Mike, David,
On 24 Jul 2010, at 10:45, Mike Stump wrote:
> Also, as memory serves there is one more fix from last year that
> fixes the abi so that it is self consistent from last year. I think
> it fixes a failing struct abi testcase as I recall.
OK, I found it (the struct hack is needed after all, in fact).
FWIW, The fail of gcc.target/powerpc/ppc64-abi-1.c is misleading - I
think that is a situation where the test-case needs some tweaking.
The darwin64-abi.c check from 4.2.1 (or LLVM) is the right thing to
use...
... but I have no idea what license it is covered by and, therefore,
whether we can import it... so for now I'm using it outside the gcc
tree.
====
There will follow a part #3 (I can hack a fix... now I need to find
the Right Way).
Part 3 will apply on top of parts 1 & 2 .. so those patches stand.
cheers
Iain
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 15:06 ` IainS
@ 2010-07-24 15:10 ` David Edelsohn
2010-07-27 13:35 ` IainS
0 siblings, 1 reply; 10+ messages in thread
From: David Edelsohn @ 2010-07-24 15:10 UTC (permalink / raw)
To: IainS; +Cc: mrs, GCC Patches
On Sat, Jul 24, 2010 at 11:06 AM, IainS
<developer@sandoe-acoustics.co.uk> wrote:
> Mike, David,
>
> On 24 Jul 2010, at 10:45, Mike Stump wrote:
>>
>> Also, as memory serves there is one more fix from last year that fixes the
>> abi so that it is self consistent from last year. I think it fixes a
>> failing struct abi testcase as I recall.
>
> OK, I found it (the struct hack is needed after all, in fact).
>
> FWIW, The fail of gcc.target/powerpc/ppc64-abi-1.c is misleading - I think
> that is a situation where the test-case needs some tweaking.
>
> The darwin64-abi.c check from 4.2.1 (or LLVM) is the right thing to use...
> ... but I have no idea what license it is covered by and, therefore,
> whether we can import it... so for now I'm using it outside the gcc tree.
>
> ====
>
> There will follow a part #3 (I can hack a fix... now I need to find the
> Right Way).
>
> Part 3 will apply on top of parts 1 & 2 .. so those patches stand.
Darwin-specific patches approved by Mike are fine with me.
Thanks, David
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 15:10 ` David Edelsohn
@ 2010-07-27 13:35 ` IainS
0 siblings, 0 replies; 10+ messages in thread
From: IainS @ 2010-07-27 13:35 UTC (permalink / raw)
To: David Edelsohn; +Cc: mrs, gcc patches, Nathan Froyd
On 24 Jul 2010, at 16:10, David Edelsohn wrote:
> On Sat, Jul 24, 2010 at 11:06 AM, IainS
> <developer@sandoe-acoustics.co.uk> wrote:
>> Mike, David,
>>
>> On 24 Jul 2010, at 10:45, Mike Stump wrote:
>>>
>>> Also, as memory serves there is one more fix from last year that
>>> fixes the
>>> abi so that it is self consistent from last year. I think it
>>> fixes a
>>> failing struct abi testcase as I recall.
>>
>> OK, I found it (the struct hack is needed after all, in fact).
>>
>> FWIW, The fail of gcc.target/powerpc/ppc64-abi-1.c is misleading -
>> I think
>> that is a situation where the test-case needs some tweaking.
>>
>> The darwin64-abi.c check from 4.2.1 (or LLVM) is the right thing to
>> use...
>> ... but I have no idea what license it is covered by and,
>> therefore,
>> whether we can import it... so for now I'm using it outside the gcc
>> tree.
>>
>> ====
>>
>> There will follow a part #3 (I can hack a fix... now I need to find
>> the
>> Right Way).
>>
>> Part 3 will apply on top of parts 1 & 2 .. so those patches stand.
>
> Darwin-specific patches approved by Mike are fine with me
part 1 is r162567
part 2 (as amended with Nathan's suggestion, copied below) is r162568
(changelog for #2 is 162569)
cheers,
Iain
amended part 2/2
/* We need to deal with the fact that the darwin ppc64 ABI is
defined by an
earlier version of gcc, with the property that it always applied
alignment
adjustments to the va-args (even for zero-sized types). The
cheapest way
to deal with this is to replicate the effect of the part of
std_gimplify_va_arg_expr that carries out the align adjust, for
the case
of relevance.
We don't need to check for pass-by-reference because of the test
above.
We can return a simplifed answer, since we know there's no
offset to add. */
if (TARGET_MACHO
&& rs6000_darwin64_abi
&& integer_zerop (TYPE_SIZE (type)))
{
unsigned HOST_WIDE_INT align, boundary;
tree valist_tmp = get_initialized_tmp_var (valist, pre_p, NULL);
align = PARM_BOUNDARY / BITS_PER_UNIT;
boundary = FUNCTION_ARG_BOUNDARY (TYPE_MODE (type), type);
if (boundary > MAX_SUPPORTED_STACK_ALIGNMENT)
boundary = MAX_SUPPORTED_STACK_ALIGNMENT;
boundary /= BITS_PER_UNIT;
if (boundary > align)
{
tree t ;
/* This updates arg ptr by the amount that would be necessary
to align the zero-sized (but not zero-alignment) item. */
t = build2 (MODIFY_EXPR, TREE_TYPE (valist), valist_tmp,
fold_build2 (POINTER_PLUS_EXPR,
TREE_TYPE (valist),
valist_tmp, size_int (boundary - 1)));
gimplify_and_add (t, pre_p);
t = fold_convert (sizetype, valist_tmp);
t = build2 (MODIFY_EXPR, TREE_TYPE (valist), valist_tmp,
fold_convert (TREE_TYPE (valist),
fold_build2 (BIT_AND_EXPR, sizetype, t,
size_int (-boundary))));
t = build2 (MODIFY_EXPR, TREE_TYPE (valist), valist, t);
gimplify_and_add (t, pre_p);
}
/* Since it is zero-sized there's no increment for the item
itself. */
valist_tmp = fold_convert (build_pointer_type (type),
valist_tmp);
return build_va_arg_indirect_ref (valist_tmp);
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 8:36 [Patch Darwin 2/2] fix PPC64 ABI IainS
2010-07-24 9:46 ` Mike Stump
@ 2010-07-24 10:13 ` Nathan Froyd
2010-07-24 11:44 ` IainS
1 sibling, 1 reply; 10+ messages in thread
From: Nathan Froyd @ 2010-07-24 10:13 UTC (permalink / raw)
To: IainS; +Cc: GCC Patches, mrs, David Edelsohn
On Sat, Jul 24, 2010 at 09:36:14AM +0100, IainS wrote:
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 162457)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -8931,6 +8995,51 @@ rs6000_gimplify_va_arg (tree valist, tree type,
> gi
> return build_va_arg_indirect_ref (t);
> }
>
> +#if TARGET_MACHO
Could we avoid introducing more #if TARGET_MACHO where it's not
necessary? It should be sufficient to say something like:
if (TARGET_MACHO
&& rs6000_darwin_abi
&& integer_zerop (TYPE_SIZE (type)))
...
and the compiler will DTRT and fold out that code for non-Darwin
targets. (This comment applies equally to your other darwin64 patch.)
-Nathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 10:13 ` Nathan Froyd
@ 2010-07-24 11:44 ` IainS
2010-07-26 13:33 ` Nathan Froyd
0 siblings, 1 reply; 10+ messages in thread
From: IainS @ 2010-07-24 11:44 UTC (permalink / raw)
To: Nathan Froyd; +Cc: GCC Patches, mrs, David Edelsohn
On 24 Jul 2010, at 11:13, Nathan Froyd wrote:
> On Sat, Jul 24, 2010 at 09:36:14AM +0100, IainS wrote:
>> Index: gcc/config/rs6000/rs6000.c
>> ===================================================================
>> --- gcc/config/rs6000/rs6000.c (revision 162457)
>> +++ gcc/config/rs6000/rs6000.c (working copy)
>> @@ -8931,6 +8995,51 @@ rs6000_gimplify_va_arg (tree valist, tree
>> type,
>> gi
>> return build_va_arg_indirect_ref (t);
>> }
>>
>> +#if TARGET_MACHO
>
> Could we avoid introducing more #if TARGET_MACHO where it's not
> necessary? It should be sufficient to say something like:
>
> if (TARGET_MACHO
> && rs6000_darwin_abi
> && integer_zerop (TYPE_SIZE (type)))
> ...
>
> and the compiler will DTRT and fold out that code for non-Darwin
> targets. (This comment applies equally to your other darwin64 patch.)
sure, if that's the preference, no problem.
I guess I was thinking that it would be better to eliminate the code
at pre-processing
cheers,
Iain
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-24 11:44 ` IainS
@ 2010-07-26 13:33 ` Nathan Froyd
2010-07-26 15:19 ` IainS
0 siblings, 1 reply; 10+ messages in thread
From: Nathan Froyd @ 2010-07-26 13:33 UTC (permalink / raw)
To: IainS; +Cc: GCC Patches, mrs, David Edelsohn
On Sat, Jul 24, 2010 at 12:43:52PM +0100, IainS wrote:
> On 24 Jul 2010, at 11:13, Nathan Froyd wrote:
>> Could we avoid introducing more #if TARGET_MACHO where it's not
>> necessary? It should be sufficient to say something like:
>>
>> if (TARGET_MACHO
>> && rs6000_darwin_abi
>> && integer_zerop (TYPE_SIZE (type)))
>> ...
>>
>> and the compiler will DTRT and fold out that code for non-Darwin
>> targets. (This comment applies equally to your other darwin64 patch.)
>
> sure, if that's the preference, no problem.
> I guess I was thinking that it would be better to eliminate the code at
> pre-processing
It does slightly speed things up (I doubt that you'd notice the speedup,
really). The rationale for using:
if (TARGET_MACHO)
versus
#if TARGET_MACHO
is that with the former, the compiler will validate that the code
syntax-checks and type-checks even when compiling for non-Darwin
targets. This extra checking makes it slightly harder to inadvertently
break things.
-Nathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch Darwin 2/2] fix PPC64 ABI
2010-07-26 13:33 ` Nathan Froyd
@ 2010-07-26 15:19 ` IainS
0 siblings, 0 replies; 10+ messages in thread
From: IainS @ 2010-07-26 15:19 UTC (permalink / raw)
To: Nathan Froyd; +Cc: gcc patches, mrs
On 26 Jul 2010, at 14:33, Nathan Froyd wrote:
> On Sat, Jul 24, 2010 at 12:43:52PM +0100, IainS wrote:
>> On 24 Jul 2010, at 11:13, Nathan Froyd wrote:
>>> Could we avoid introducing more #if TARGET_MACHO where it's not
>>> necessary? It should be sufficient to say something like:
>>>
>>> if (TARGET_MACHO
>>> && rs6000_darwin_abi
>>> && integer_zerop (TYPE_SIZE (type)))
>>> ...
>>>
>>> and the compiler will DTRT and fold out that code for non-Darwin
>>> targets. (This comment applies equally to your other darwin64
>>> patch.)
>>
>> sure, if that's the preference, no problem.
>> I guess I was thinking that it would be better to eliminate the
>> code at
>> pre-processing
>
> It does slightly speed things up (I doubt that you'd notice the
> speedup,
> really). The rationale for using:
>
> if (TARGET_MACHO)
>
> versus
>
> #if TARGET_MACHO
>
> is that with the former, the compiler will validate that the code
> syntax-checks and type-checks even when compiling for non-Darwin
> targets. This extra checking makes it slightly harder to
> inadvertently
> break things.
OK. I'll do a second pass through (after sorting out part #3 of the
ABI fixes).
Apropos extending this to its logical conclusion:
I suspect that the only viable 'end-game' at present is to macro-ize
the machopic* calls,
exposing the whole of the machopic workings would probably carry too
much weight.
(that comment would apply equally to i386.c, which also uses that
machopic common code).
Iain
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-07-27 13:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-24 8:36 [Patch Darwin 2/2] fix PPC64 ABI IainS
2010-07-24 9:46 ` Mike Stump
2010-07-24 10:04 ` IainS
2010-07-24 15:06 ` IainS
2010-07-24 15:10 ` David Edelsohn
2010-07-27 13:35 ` IainS
2010-07-24 10:13 ` Nathan Froyd
2010-07-24 11:44 ` IainS
2010-07-26 13:33 ` Nathan Froyd
2010-07-26 15:19 ` IainS
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).