diff mbox

Disable accumulate-outgoing-args for Generic and Buldozers

Message ID 20140123231639.GA6783@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Jan. 23, 2014, 11:16 p.m. UTC 
> > 	* config/i38/x86-tune.def: Disable X86_TUNE_ACCUMULATE_OUTGOING_ARGS
> > 	for generic and recent AMD chips
> 
> The ChangeLog reads:
> 
> 2014-01-22  Jan Hubicka  <jh@suse.cz>
> 
>         * config/i386/x86-tune.def (X86_TUNE_ACCUMULATE_OUTGOING_ARGS):
>         Enable for generic and recent AMD targets.
> 
> Very confusing...
Sorry, will fix it.

I looked into the movups.  The problem here is that ix86_expand_push is
not given a memory destination argument (because push expander is not)
and it builds it by itself.  The alignment then ends up being 8.
I am not 100% sure if the following code is safe (if I change alignment
in the testcase we end up using DImode push so it works). Perhaps we want
to remove the push expanders and make ix86_expand_move to handle this case
so we preserve other mem info too?

Honza

	* i386.c (ix86_expand_push): Set alignment based on argument
	boundary.

Comments

H.J. Lu March 24, 2014, 5:04 p.m. UTC | #1 
On Thu, Jan 23, 2014 at 3:16 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >     * config/i38/x86-tune.def: Disable X86_TUNE_ACCUMULATE_OUTGOING_ARGS
>> >     for generic and recent AMD chips
>>
>> The ChangeLog reads:
>>
>> 2014-01-22  Jan Hubicka  <jh@suse.cz>
>>
>>         * config/i386/x86-tune.def (X86_TUNE_ACCUMULATE_OUTGOING_ARGS):
>>         Enable for generic and recent AMD targets.
>>
>> Very confusing...
> Sorry, will fix it.
>

I still see:

2014-01-22  Jan Hubicka  <hubicka@ucw.cz>

        * config/i386/x86-tune.def (X86_TUNE_ACCUMULATE_OUTGOING_ARGS):
        Enable for generic and recent AMD targets.

We should also update comments:

   FIXME: the flags is incorrectly enabled for amdfam10, Bulldozer,
   Bobcat and Generic.  This is because disabling it causes large
   regression on mgrid due to IRA limitation leading to unecessary
   use of the frame pointer in 32bit mode.  */
diff mbox

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 206946)
+++ config/i386/i386.c	(working copy)
@@ -17230,6 +17230,9 @@ 
 ix86_expand_push (enum machine_mode mode, rtx x)
 {
   rtx tmp;
+  unsigned int align = ix86_function_arg_boundary (mode, NULL);
+  if (align > MAX_SUPPORTED_STACK_ALIGNMENT)
+    align = MAX_SUPPORTED_STACK_ALIGNMENT;
 
   tmp = expand_simple_binop (Pmode, PLUS, stack_pointer_rtx,
 			     GEN_INT (-GET_MODE_SIZE (mode)),
@@ -17239,6 +17242,11 @@ 
 
   tmp = gen_rtx_MEM (mode, stack_pointer_rtx);
 
+  /* We use push of non-integer parameters only to store function
+     arguments.  */
+  if (MEM_ALIGN (tmp) < align)
+    set_mem_align (tmp, align);
+
   /* When we push an operand onto stack, it has to be aligned at least
      at the function argument boundary.  However since we don't have
      the argument type, we can't determine the actual argument