Re: -mtune-ctrl=.... support, round-2

[Xinliang David Li <davidxl at google dot com>]

Fixed some formatting issues and typos. There are no regressions with
the attached patch. Ok for trunk?

thanks,

David

On Wed, Aug 7, 2013 at 10:12 PM, Xinliang David Li <davidxl@google.com> wrote:
> Hi, the attached is a follow up patch. The patch includes the following changes:
>
> 1. new option -mno-default -- suggested by H.J. It can be combined
> with -mtune-ctrl= to clear the default settings.
> 2. new option -mdump-tune-features -- this option can be used to dump
> all feature names and their default settings.
> 3. Added documentation to the 3 new options
>
> 4. Move the contents of inital_x86_tune_features into x86-tune.def to
> make sure they are always in sync
> 5. Some refactoring and handle the the tune features properly during
> target option restore.
>
> 2013-08-07  Xinliang David Li  <davidxl@google.com>
>
>         * config/i386/i386.h: Adjust macro definition.
>         * config/i386/i386.opt: Define two new options.
>         * config/i386/x86-tune.def: Add arch selector field in macros.
>         * config/i386/i386.h: include x86-tune.def.
>         * config/i386/i386.c (ix86_option_override_internal):
>         Refactor the code.
>         (parse_mtune_ctrl_str): New function.
>         (set_ix86_tune_features): New function.
>         (ix86_function_specific_restore): Call the new helper function.
>
> Ok for trunk after testing?
>
> thanks,
>
> David
>
>
> On Wed, Aug 7, 2013 at 5:22 PM, Xinliang David Li <davidxl@google.com> wrote:
>> On Wed, Aug 7, 2013 at 4:54 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>>> On Sun, 4 Aug 2013, Andi Kleen wrote:
>>>
>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> >
>>>> > The patch fails to add documentation.
>>>>
>>>> That seems like a feature, it's likely not useful for the general
>>>> public. More for specialized tools that automatically search
>>>> for the best tuning.
>>>
>>> If such a tool is not part of GCC itself, then it is a user of GCC and
>>> documentation should be provided for those writing such a tool.
>>>
>>> Options should be undocumented in very limited cases such as multiple
>>> variant spellings where only one should be used but others are accepted
>>> because they were historically, or whether the user interface to an option
>>> is separate from the internal option passed to cc1 (the documentation is
>>> of options as passed to the driver, but the .opt file may describe options
>>> passed to cc1 as a result of other options being passed to the driver), or
>>> the option really is only for use within the build of GCC itself.
>>> Otherwise, the strong presumption is that all options should be
>>> documented, with appropriate caveats as applicable about the cases where
>>> an option is useful.
>>>
>>
>> Fair enough. I will add the documentation in a followup patch which
>> also implements H.J's proposal -mno-default and cleans up the initial
>> tuning array to be always in sync with new enums.
>>
>>
>>>> > And I am nervous about testing
>>>> > coverage - is this considered a development option only or are random
>>>> > combinations expected to work in all situations? I expect not, thus
>>>> > this looks like a dangerous option?
>>>>
>>>> When it's undocumented it doesn't need to work in every situation?
>>>
>>> No input files and command-line arguments whatsoever to the driver should
>>> result in ICEs, including undocumented options.
>>
>> True -- if ICEs are found, we should fix them.
>>
>> thanks,
>>
>> David
>>
>>>
>>> --
>>> Joseph S. Myers
>>> joseph@codesourcery.com

Attachment: mtune-ctrl_follow.txt
Description: Text document