On Thu, Nov 12, 2015 at 12:37 PM, Tom de Vries <tom_devr...@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
