This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][i386]Fix PR 57756


On Sat, Oct 12, 2013 at 12:10 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sat, Oct 12, 2013 at 2:16 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>
>> Ping.
>
>>>>> This looks nice.  I suppose you grepped for effected targets and rs6000
>>>>> was the only one besides i386.
>>>>>
>>>>> This is ok given target maintainers do not object within 24h and the test
>>>>> successfully bootstrapped and passed regression tests.
>>>>
>>>> I changed this patch  quite a bit after I realized I missed many other
>>>> places where global_options field was accessed. The two specific
>>>> changes are:
>>>>
>>>> * Particularly, ix86_function_specific_save and
>>>> ix86_function_specific_restore had to be changed to copy to a specific
>>>> gcc_options structure than to global_options.
>>>> * Function ix86_option_override_internal accesses global_options via
>>>> macros, like TARGET_64BIT etc. This had to be changed. So, I created
>>>> new macros to accept a parameter and named them like TARGET_64BIT_P
>>>> and replaced all the accesses in i386.c
>>>>
>>>> I also marked as TODO a copy in function ix86_function_specific_save.
>>>> Here, the cl_target_option structure has a ix86_target_flags_explicit
>>>> field whereas the global_options structure does not have one.
>>>> Previously, this used to get saved to the global_options target_flags
>>>> field but this did not make sense to me. I have commented this line
>>>> out. Please let me know if a new field needs to be added.
>>>>
>>>> This patch passes bootstrap and all tests. Please take another look.
>
> AFAICS, the patch was approved by Richard on 23. September. None of
> the target maintainers have had any further objections to it.
>
> Regarding the TODO in i386.c: some options depend on and enable other
> options, for example -msse3 enables SSE2, etc, although user didn't
> explicitly add -msse2 to the options. This is not the case with global
> options. Since this field is used extensively through i386.c, I'd say
> to play safe and save it.
>
> So, x86 part is OK with the above change. Middle end is already
> approved and rs6000 maintainer didn't object the approval, so please
> go ahead and commit the patch.

I committed this patch after making the above change.

Thanks
Sri


>
> Thanks,
> Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]