This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: Sriraman Tallam <tmsriram at google dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 15 Nov 2013 11:06:14 +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> <CAFULd4ZN_=H+XiL4rRZg=m+S6t8_GWhtR-5mUaT_xccoxO5qwQ at mail dot gmail dot com> <DUB122-W46CD0D6028AB0C8BA7A050E4FB0 at phx dot gbl>
On Fri, Nov 15, 2013 at 10:58 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
>>>> 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,
>>>> &global_options_set);
>>>>
>>>> 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
>> many similar.
>>
>> Thanks,
>> Uros.
>
> I agree, if you want, I can apply patch #2 today, (which at least does not
> look so hacky as my first approach), to buy us some time.
>
> And I think Sriram should prepare a followup-patch that removes
> this dependencies on global_options_set or makes that data structure
> also target specific.
Do we have a pressing issue here that needs immediate, but incomplete fix?
If this is not the case, I'd rather wait for a correct fix.
Thanks,
Uros.
- References:
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- RE: [PATCH, i386] Fix -mpreferred-stack-boundary
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- RE: [PATCH, i386] Fix -mpreferred-stack-boundary
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- Re: [PATCH, i386] Fix -mpreferred-stack-boundary
- RE: [PATCH, i386] Fix -mpreferred-stack-boundary