This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Sriraman Tallam <tmsriram at google dot com>
- Cc: Bernd Edlinger <bernd dot edlinger at hotmail dot de>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 15 Nov 2013 10:09:02 +0100
- Subject: Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4Z1GqxgJAYU9r9iadMPVE+nBCJw6pp8qPo9O_i9ExzHoQ at mail dot gmail dot com> <CAFULd4aU3DEPUGeN=bLkArGW2S=iHtxQTGKTgD9-eYGvVK1z0A at mail dot gmail dot com> <CAAs8Hmygdb12ULdBYXhSKons5RKAEZgBn0zdu45=sL1gMSm+hg at mail dot gmail dot com> <DUB122-W28C9D0D0ECEBE59BFBF9FE4FE0 at phx dot gbl> <CAAs8HmyTuwVW8GB=KjkQks6Dvpm5Z=0oOmz-Z-_gkGVCP+iKeA at mail dot gmail dot com> <CAAs8HmwyO9usuHVdxN-T75yGgngThhmGSobD9kodiJ9D0cdEJA at mail dot gmail dot com> <DUB122-W286028EA3B1AE0F218D774E4F80 at phx dot gbl> <CAAs8HmzMaKxko_bQ98RUWBR1TNE3oABKDpgW5qztkGzoQ-YuAg at mail dot gmail dot com>
On Fri, Nov 15, 2013 at 4:54 AM, Sriraman Tallam <email@example.com> wrote:
>>>>>>>>> Currently on trunk the option -mpreferred-stack-boundary does not work
>>>>>>>>> together with #pragma GCC target("sse") or __attribute__((target("sse"))).
>>>>>>>>> There is already a test case that detects this: gcc.target/i386/fastcall-sseregparm.c
>>>>>>>>> The attached patch fixes this test case under i686-pc-linux-gnu.
>>>>>>>>> Boot-strapped and regression-tested under i686-pc-linux-gnu.
>>>>>>>>> OK for trunk?
>>>> No, this is not what I had in mind. This is simply reverting my
>>>> refactoring work which was to make ix86_option_override_internal get
>>>> rid of the global_options dependency. Here is the problem:
>>>> global_options gets some flags set after command-line options are read
>>>> (ix86_preferred_stack_boundary_arg in this case). But, this does not
>>>> get saved into target_option_default_node because there is no
>>>> corresponding field in cl_target_option for
>>>> ix86_preferred_stack_boundary_arg. So, when you restore
>>>> target_option_default_node to func_options in
>>>> ix86_valid_target_attribute_p, this particular flag does not get
>>>> copied. So, you can either copy this explicitly to func_options which
>>>> was your first patch or you could extend cl_target_option to include
>>>> this field too which is done by making
>>>> ix86_preferred_stack_boundary_arg a Variable in i386.opt. The latter
>>>> is cleaner because it always saves the default flags into
>>> I quickly hacked up what I had in mind and attached the patch. Can you
>>> check if this fixes your problem?
>> Well, this way it could be fixed too.
>> But opts->x_ix86_preferred_stack_bounary_arg is not dependent on any
>> pragma or target attribute. Correct me if that is wrong.
> That seems correct.
>> And this code
>> if (opts_set->x_ix86_preferred_stack_boundary_arg)
>> int min = (TARGET_64BIT_P (opts->x_ix86_isa_flags)
>> ? (TARGET_SSE_P (opts->x_ix86_isa_flags) ? 4 : 3) : 2);
>> int max = (TARGET_SEH ? 4 : 12);
>> if (opts->x_ix86_preferred_stack_boundary_arg < min
>> || opts->x_ix86_preferred_stack_boundary_arg> max)
>> checks func_options against global_options_set:
>> new_target = ix86_valid_target_attribute_tree (args, &func_options,
>> So this code as it is will fail if this option was ever made target specific.
> This is correct. But, right now global_options_set is capturing only
> the command line options that are set and does not seem to be
> modified. If this option were to be made target specific we should not
> access this field off global_options_set. We should add a MASK to
> target flags and get it from there just like any other target flag
> that is function specific does it.
>> There is still a reason why this check needs to be executed each time
>> the opts->x_ix86_isa_flags changes.
>> Because of this I still would prefer my second attempt of fixing this issue,
>> because it is simple and it removes the different handling between
>> -mpreferred-stack-boundary and -mincoming-stack-boundary.
> I understand your problem better now. I still do not think we should
> make ix86_option_override_internal should read global_options flags
> directly. That is overriding opts passed in as a parameter. I am fine
> with Patch 1 which is explicitly copying global_options
> preferred_stack_boundary_arg fields onto func_options. FYI, I cannot
> approve any patches and you still have to get it approved by the
> maintainers. I will sweep these copies and save it into
> cl_target_option as a cleanup if more of these emerge.
I'd prefer the proposed general solution. It seems to me that Patch 1
is somehow a hack that will "solve" only one particular option out of