On Thu, Jan 28, 2016 at 04:42:02AM -0800, H.J. Lu wrote:
> >>>>>> 2016-01-27 Jakub Jelinek <ja...@redhat.com>
> >>>>>> Ilya Enkovich <enkovich....@gmail.com>
> >>>>>>
> >>>>>> PR target/69454
> >>>>>> * config/i386/i386.c (convert_scalars_to_vector): Remove
> >>>>>> stack alignment fixes.
> >>>>>> (ix86_option_override_internal): Disable TARGET_STV if stack
> >>>>>> is not properly aligned.
> >>>>>>
> >>>>>> gcc/testsuite/
> >>>>>>
> >>>>>> 2016-01-27 Ilya Enkovich <enkovich....@gmail.com>
> >>>>>>
> >>>>>> PR target/69454
> >>>>>> * gcc.target/i386/pr69454-1.c: New test.
> >>>>>> * gcc.target/i386/pr69454-2.c: New test.
> >>>>>>
> >>>>>>
> >>>>>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> >>>>>> index 34b57a4..9fb8db8 100644
> >>>>>> --- a/gcc/config/i386/i386.c
> >>>>>> +++ b/gcc/config/i386/i386.c
> >>>>>> @@ -3588,16 +3588,6 @@ convert_scalars_to_vector ()
> >>>>>> bitmap_obstack_release (NULL);
> >>>>>> df_process_deferred_rescans ();
> >>>>>>
> >>>>>> - /* Conversion means we may have 128bit register spills/fills
> >>>>>> - which require aligned stack. */
> >>>>>> - if (converted_insns)
> >>>>>> - {
> >>>>>> - if (crtl->stack_alignment_needed < 128)
> >>>>>> - crtl->stack_alignment_needed = 128;
> >>>>>> - if (crtl->stack_alignment_estimated < 128)
> >>>>>> - crtl->stack_alignment_estimated = 128;
> >>>>>> - }
> >>>>>> -
> >>>>>> return 0;
> >>>>>> }
> >>>>>>
> >>>>>> @@ -5453,6 +5443,11 @@ ix86_option_override_internal (bool main_args_p,
> >>>>>> opts->x_target_flags |= MASK_VZEROUPPER;
> >>>>>> if (!(opts_set->x_target_flags & MASK_STV))
> >>>>>> opts->x_target_flags |= MASK_STV;
> >>>>>> + /* Disable STV if -mpreferred-stack-boundary=2 - the needed
> >>>>>> + stack realignment will be extra cost the pass doesn't take into
> >>>>>> + account and the pass can't realign the stack. */
> >>>>>> + if (ix86_preferred_stack_boundary < 64)
> >>>>>> + opts->x_target_flags &= ~MASK_STV;
> >>
> >> This won't work for 32-bit incoming stack boundary and 64-bit preferred
> >> stack boundary. In this case, STV won't be off. When LRA needs 64-bit
> >> aligned stack slot, stack must be realigned. But for leaf function, we may
> >> not realign stack if ix86_minimum_alignment returns 32 for DImode. You
> >> must either add assert (!TARGET_STV) before returning 32 for DImode or
> >> return 64 for DImode if STV is on in ix86_minimum_alignment.
> >
> > TARGET_STV doesn't mean STV pass will run. We can check alignment in STV
> > pass gate and this assert would be wrong. If we decide STV to be dependent
> > on
> > stack alignment then we shouldn't make alignment be dependent on STV. I can
> > add
> > assert into convert_scalars_to_vector to check
> > crtl->stack_alignment_estimated >= 64
> > by that moment.
> >
>
> The bottom line is ix86_minimum_alignment must return the correct
> number for DImode or you can just turn off STV. My suggestion is
> to use my patch.
Uros, any preferences here? I mean, it is possible to use
e.g. the ix86_option_override_internal and have H.J's ix86_minimum_alignment
change as a safety net, in the usual case for -mpreferred-stack-boundary=2
we'll just disable TARGET_STV and ix86_minimum_alignment change won't do
anything, as TARGET_STV will be false, and if for whatever case it gets
through (target attribute, -mincoming-stack-boundary=, ...)
ix86_minimum_alignment will be there to ensure enough stack alignment.
Most of the smaller -mpreferred-stack-boundary= uses are -mno-sse anyway,
and that is something we don't want to affect.
Jakub
