This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC] Remove first_pass_instance from pass_vrp
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Tom de Vries <Tom_deVries at mentor dot com>
- Cc: "gcc-patches at gnu dot org" <gcc-patches at gnu dot org>
- Date: Thu, 12 Nov 2015 13:26:27 +0100
- Subject: Re: [RFC] Remove first_pass_instance from pass_vrp
- Authentication-results: sourceware.org; auth=none
- References: <56447A09 dot 4070608 at mentor dot com>
On Thu, Nov 12, 2015 at 12:37 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Hi,
>
> [ See also related discussion at
> https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00452.html ]
>
> this patch removes the usage of first_pass_instance from pass_vrp.
>
> the patch:
> - limits itself to pass_vrp, but my intention is to remove all
> usage of first_pass_instance
> - lacks an update to gdbhooks.py
>
> Modifying the pass behaviour depending on the instance number, as
> first_pass_instance does, break compositionality of the pass list. In other
> words, adding a pass instance in a pass list may change the behaviour of
> another instance of that pass in the pass list. Which obviously makes it
> harder to understand and change the pass list. [ I've filed this issue as
> PR68247 - Remove pass_first_instance ]
>
> The solution is to make the difference in behaviour explicit in the pass
> list, and no longer change behaviour depending on instance number.
>
> One obvious possible fix is to create a duplicate pass with a different
> name, say 'pass_vrp_warn_array_bounds':
> ...
> NEXT_PASS (pass_vrp_warn_array_bounds);
> ...
> NEXT_PASS (pass_vrp);
> ...
>
> But, AFAIU that requires us to choose a different dump-file name for each
> pass. And choosing vrp1 and vrp2 as new dump-file names still means that
> -fdump-tree-vrp no longer works (which was mentioned as drawback here:
> https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00453.html ).
>
> This patch instead makes pass creation parameterizable. So in the pass list,
> we use:
> ...
> NEXT_PASS_WITH_ARG (pass_vrp, true /* warn_array_bounds_p */);
> ...
> NEXT_PASS_WITH_ARG (pass_vrp, false /* warn_array_bounds_p */);
> ...
>
> This approach gives us clarity in the pass list, similar to using a
> duplicate pass 'pass_vrp_warn_array_bounds'.
>
> But it also means -fdump-tree-vrp still works as before.
>
> Good idea? Other comments?
It's good to get rid of the first_pass_instance hack.
I can't comment on the AWK, leaving that to others. Syntax-wise I'd hoped
we can just use NEXT_PASS with the extra argument being optional...
I don't see the need for giving clone_with_args a new name, just use an overload
of clone ()? [ideally C++ would allow us to say that only one overload may be
implemented]
Thanks,
Richard.
> Thanks,
> - Tom