[patch][plugins] Provide unique names to all compiler passes

[Richard Guenther]

On Fri, Nov 6, 2009 at 11:53 AM, Joern Rennecke <amylaar@spamcop.net> wrote:
> Quoting Richard Guenther <richard.guenther@gmail.com>:
>
>> On Thu, Nov 5, 2009 at 11:32 PM, Joern Rennecke <amylaar@spamcop.net>
>> wrote:
>
> ...
>>>
>>> This exposed a problem with the patch: the renaming of passes changes
>>> the dump file names and options.  The -fdump-postreload option was
>>> suddenly missing.
>>>
>>> To rectify this, I have backed out the name changes where there was no
>>> actual pass name conflict, and where an actual name clash had to be
>>> resolved, I have used a prefix followed with ASCII STX, which a bit of
>>> new code in register_one_dump_file skips, so that the old dump file and
>>> option names continue to be used.
>>
>> Ick.  Why use STX when you could have used for example '@'?
>
> I would have thought '@' to be in the dump file name / options namespace.
> Also, STX indeed marks the 'start of text' with regards to that purpose.
>
> That being said, although not my first choice, I think '@' would work as
> well, so if you think that would be better, I can prepare & test a patch
> with '@'.  Or whatever other character people might prefer.
>
>>> There might be a case to disambiguate some dump options, but I don't want
>>> to bundle such user interface changes and the test suite changes that
>>> would
>>> then be required with the plugin prerequisites.
>>
>> Fair enough... OTOH the disambiguator seems to be
>> GIMPLE_PASS vs. RTL_PASS in the cases where there were
>> dump options available.
>> When there were no dump options
>> available you could have simply renamed the pass.
>
> I did use "*all-postreload" for the postreload pass to avoid a clash
> with the "postreload" of postreload_cse (technically "*postreload"
> and "postreload" do not clash at the moment, but it seemed like a bug
> waiting to happen to rely on that).
>
>> So, is using the pass kind enough to resolve ambiguities?
>
> It would work for pre and eh passes, but not for dce - all of the dce passes
> are rtl passes.
> Also, if we ever get to the point of putting the mudflap and the nomudflap
> passes in the same executable, we can't distinguish them by the type either.
>
> Moreover, it would make identifying passes more complicated if we couldn't
> simply identify them by a string.
> You'd either have to pass around a tuple of multiple identifying data items,
> or some pointer to a struct.  If you pass a pointer to the pass as such, the
> plugins would have to know about the structure of that struct, which may be
> subject to change.
>
> I could put type and name in their own struct, but then all the code that
> uses pass structs would need changing.  Plus, the plugins would need to
> know about enum opt_pass_type, and we might want to change that too at
> some point in time.
>
> Also, a lot of the ICI pass handling would have to be rewritten if we can't
> identify a pass by a name.
> For debug purposes, it would also be more convenient if we can output
> a single identifying string rather than have to look at multiple fields.

Ok.  The patch is ok after waiting another 24h to let others
the time to comment.

Thanks,
Richard.