On Mon, Jun 6, 2011 at 4:22 AM, Richard Guenther
<richard.guent...@gmail.com> wrote:
> On Wed, Jun 1, 2011 at 7:24 PM, Xinliang David Li <davi...@google.com> wrote:
>> The attached is the split #1 patch that enhances -fenable/disable.
>>
>> Ok after testing?
>
> I expect the testcases will be quite fragile, so while I appreciate
> test coverage for new options I think we should go without those
> that involve any kind of UID. Those which use assembler names
> also will fail randomly dependent on how targets mangle their
> functions - so I think we have to drop all testcases.
Ok -- how about keeping tests with large uid range, and assembler name
for x86? A feature without testing is just to easy to break without
being noticed.
>
> Also
>
> +/* A helper function to determine if an identifier is valid to
> + be an assembler name (better to use target specific hook). */
> +
> +static bool
> +is_valid_assembler_name (const char *str)
> +{
> + const char *p = str;
> + char c;
> +
> + c = *p;
> + if (!((c >= 'a' && c <= 'z')
> + || (c >= 'A' && c <= 'Z')
> + || *p == '_'))
> + return false;
> +
> + p++;
> + while ((c = *p))
> + {
> + if (!((c >= 'a' && c <= 'z')
> + || (c >= 'A' && c <= 'Z')
> + || (c >= '0' && c <= '9')
> + || *p == '_'))
> + return false;
> + p++;
> + }
> +
> + return true;
> +}
>
> why all that complicated checks? Why not just check for p[0]
> in [^0-9] and re-structure the range parsing to switch between
> UIDs and assembler-names that way?
Ok.
David
>
> Thanks,
> Richard.
>
>> Thanks,
>> David
>>
>> On Wed, Jun 1, 2011 at 9:16 AM, Xinliang David Li <davi...@google.com> wrote:
>>> On Wed, Jun 1, 2011 at 1:51 AM, Richard Guenther
>>> <richard.guent...@gmail.com> wrote:
>>>> On Wed, Jun 1, 2011 at 1:34 AM, Xinliang David Li <davi...@google.com>
>>>> wrote:
>>>>> The following patch implements the a new option that dumps gcc PASS
>>>>> configuration. The sample output is attached. There is one
>>>>> limitation: some placeholder passes that are named with '*xxx' are
>>>>> note registered thus they are not listed. They are not important as
>>>>> they can not be turned on/off anyway.
>>>>>
>>>>> The patch also enhanced -fenable-xxx and -fdisable-xx to allow a list
>>>>> of function assembler names to be specified.
>>>>>
>>>>> Ok for trunk?
>>>>
>>>> Please split the patch.
>>>>
>>>> I'm not too happy how you dump the pass configuration. Why not simply,
>>>> at a _single_ place, walk the pass tree? Instead of doing pieces of it
>>>> at pass execution time when it's not already dumped - that really looks
>>>> gross.
>>>
>>> Yes, that was the original plan -- but it has problems
>>> 1) the dumper needs to know the root pass lists -- which can change
>>> frequently -- it can be a long term maintanance burden;
>>> 2) the centralized dumper needs to be done after option processing
>>> 3) not sure if gate functions have any side effects or have dependencies on
>>> cfun
>>>
>>> The proposed solutions IMHO is not that intrusive -- just three hooks
>>> to do the dumping and tracking indentation.
>>>
>>>>
>>>> The documentation should also link this option to the -fenable/disable
>>>> options as obviously the pass names in that dump are those to be
>>>> used for those flags (and not readily available anywhere else).
>>>
>>> Ok.
>>>
>>>>
>>>> I also think that it would be way more useful to note in the individual
>>>> dump files the functions (at the place they would usually appear) that
>>>> have the pass explicitly enabled/disabled.
>>>
>>> Ok -- for ipa passes or tree/rtl passes where all functions are
>>> explicitly disabled.
>>>
>>> Thanks,
>>>
>>> David
>>>
>>>>
>>>> Richard.
>>>>
>>>>> Thanks,
>>>>>
>>>>> David
>>>>>
>>>>
>>>
>>
>
