[PATCH] Improve target pass registration

Jakub Jelinek jakub@redhat.com
Tue Oct 4 14:54:00 GMT 2016


On Tue, Oct 04, 2016 at 05:48:15PM +0300, Alexander Monakov wrote:
> On Fri, 30 Sep 2016, Jakub Jelinek wrote:
> > This patch allows backends to provide their *-passes.def file with
> > instructions how to ammend passes.def, which then can be inspected in
> > pass-instances.def the script generates.
> 
> A few minor comments:
> 
> > --- gcc/gen-pass-instances.awk.jj	2016-09-29 22:53:10.264776158 +0200
> > +++ gcc/gen-pass-instances.awk	2016-09-30 13:22:53.745373889 +0200
> > @@ -30,81 +32,201 @@
> >  # through:
> >  #   NEXT_PASS (pass_copy_prop, 8);
> >  # (currently there are 8 instances of that pass)
> > +#
> > +#     INSERT_PASS_AFTER (pass_copy_prop, 1, pass_stv);
> 
> I think it would be nice to mention that a 4th argument can be supplied and if
> so, it will be passed to the pass at run time (although I see the existing
> comment doesn't mention that either).

Yeah, I've followed the earlier comments.  The 4th argument is quite rare
thing.

> > +# will insert
> > +#     NEXT_PASS (pass_stv, 1);
> > +# after immediately after the NEXT_PASS (pass_copy_prop, 1) line,
> 
> Typo in the comment: duplicated 'after'.

Will fix.

> > --- gcc/config/i386/i386-passes.def.jj	2016-09-30 12:54:29.959793738 +0200
> > +++ gcc/config/i386/i386-passes.def	2016-09-30 14:01:31.237200032 +0200
> > @@ -0,0 +1,31 @@
> [snip]
> > +
> > +/*
> > + Macros that should be defined used in this file:
> > +   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
> > +   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
> > +   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
> > + */
> 
> REPLACE_PASS isn't actually used in this file.

I guess it should be
  Macros that can be used in this file:

> > --- gcc/config/i386/i386-protos.h.jj	2016-06-24 12:59:29.000000000 +0200
> > +++ gcc/config/i386/i386-protos.h	2016-09-30 14:00:54.759659671 +0200
> 
> > @@ -4105,13 +4105,15 @@ class pass_stv : public rtl_opt_pass
> [snip]
> >  
> >    /* opt_pass methods: */
> >    virtual bool gate (function *)
> >      {
> > -      return TARGET_STV && TARGET_SSE2 && optimize > 1;
> > +      return (timode_p ^ (TARGET_64BIT == 0))
> > +	     && TARGET_STV && TARGET_SSE2 && optimize > 1;
> 
> The first line could also be 'timode_p == !!TARGET_64BIT'.

Yeah, that is probably more readable.

> Also I believe this needs parens around the whole expression.

Ok, will do.

	Jakub



More information about the Gcc-patches mailing list