RE: [PATCH] Makefile.in: clean up match.pd-related dependencies

[Tamar Christina <Tamar.Christina@arm.com>]

> -----Original Message-----
> From: Alexander Monakov <amonakov@ispras.ru>
> Sent: Friday, May 5, 2023 7:22 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: Richard Biener <richard.guenther@gmail.com>; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Makefile.in: clean up match.pd-related dependencies
> 
> 
> On Fri, 5 May 2023, Tamar Christina wrote:
> 
> > > -----Original Message-----
> > > From: Alexander Monakov <amonakov@ispras.ru>
> > > Sent: Friday, May 5, 2023 6:59 PM
> > > To: Tamar Christina <Tamar.Christina@arm.com>
> > > Cc: Richard Biener <richard.guenther@gmail.com>;
> > > gcc-patches@gcc.gnu.org
> > > Subject: RE: [PATCH] Makefile.in: clean up match.pd-related
> > > dependencies
> > >
> > >
> > > On Fri, 5 May 2023, Tamar Christina wrote:
> > >
> > > > > > Am 05.05.2023 um 19:03 schrieb Alexander Monakov via
> > > > > > Gcc-patches
> > > > > > <gcc-
> > > > > patches@gcc.gnu.org>:
> > > > > >
> > > > > > Clean up confusing changes from the recent refactoring for
> > > > > > parallel match.pd build.
> > > > > >
> > > > > > gimple-match-head.o is not built. Remove related flags adjustment.
> > > > > >
> > > > > > Autogenerated gimple-match-N.o files do not depend on
> > > > > > gimple-match-exports.cc.
> > > > > >
> > > > > > {gimple,generic)-match-auto.h only depend on the prerequisites
> > > > > > of the corresponding s-{gimple,generic}-match stamp file, not any .cc
> file.
> > > > >
> > > > > LGTM
> > > > >
> > > > > > gcc/ChangeLog:
> > > > > >
> > > > > >    * Makefile.in: (gimple-match-head.o-warn): Remove.
> > > > > >    (GIMPLE_MATCH_PD_SEQ_SRC): Do not depend on
> > > > > >    gimple-match-exports.cc.
> > > > > >    (gimple-match-auto.h): Only depend on s-gimple-match.
> > > > > >    (generic-match-auto.h): Likewise.
> > > > > > ---
> > > > > >
> > > > > > Tamar, do I understand correctly that you do not have more
> > > > > > plans for match.pd and I won't collide with you if I attempt
> > > > > > more cleanups in this
> > > > > area? Thanks!
> > > >
> > > > No, but I'm also not sure why this change.
> > > > The idea here was that if gimple-head-export.cc changes you must
> > > > have changed genmatch.cc and so you need to regenerate the
> > > > gimple-match-*
> > > which could change the header.
> > >
> > > gimple-head-export.cc does not exist.
> > >
> > > gimple-match-exports.cc is not a generated file. It's under source
> > > control and edited independently from genmatch.cc. It is compiled
> > > separately, producing gimple-match-exports.o.
> > >
> > > gimple-match-head.cc is also not a generated file, also under source
> control.
> > > It is transitively included into gimple-match-N.o files. If it
> > > changes, they will be rebuilt. This is not changed by my patch.
> > >
> > > gimple-match-auto.h is a generated file. It depends on
> > > s-gimple-match stamp file, which in turn depends on genmatch and
> > > match.pd. If either changes, the rule for the stamp file triggers.
> > > gimple-match-N.o files also depend on the stamp file, so they will be
> rebuilt as well.
> >
> > s-gimple-match does not depend on gimple-match-head.cc. if it changes
> > the stamp is not invalidated.
> 
> Right, this is correct: there's no need to rerun the recipe for the stamp,
> because contents of gimple-match-head.cc do not affect it.
> 
> > This happens to work because gimple-match-N.cc does depend on
> > gimple-match-head.cc, but if the gimple-match-N.cc already exists then
> nothing changes.
> 
> No, if gimple-match-N.cc already exist, make notices they are out-of-date via
> 
> $(GIMPLE_MATCH_PD_SEQ_SRC): s-gimple-match gimple-match-head.cc;
> @true
> 
> and this triggers rebuilding gimple-match-N.o.
> 
> I tested this. After 'touch gimple-match-head.cc' all ten gimple-match-N.o files
> are rebuilt.
> 
> > So I don't think this changes anything. If anything I would say the
> > stamp file needs to depend on gimple-match-head.cc.
> 
> Is my explanation above satisfactory?

Sure,

Thanks,
Tamar

> 
> Thanks.
> Alexander
> 
> >
> > Thanks,
> > Tamar
> >
> > >
> > > Is there some problem I'm not seeing?
> > >
> > > Thanks.
> > > Alexander
> > >
> > > > So not sure I agree with this.
> > > >
> > > > Thanks,
> > > > Tamar
> > > >
> > > > > >
> > > > > > gcc/Makefile.in | 9 +++------
> > > > > > 1 file changed, 3 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in index
> > > > > > 7e7ac078c5..0cc13c37d0 100644
> > > > > > --- a/gcc/Makefile.in
> > > > > > +++ b/gcc/Makefile.in
> > > > > > @@ -230,7 +230,6 @@ gengtype-lex.o-warn = -Wno-error
> > > > > > libgcov-util.o-warn = -Wno-error libgcov-driver-tool.o-warn =
> > > > > > -Wno-error libgcov-merge-tool.o-warn = -Wno-error
> > > > > > -gimple-match-head.o-warn = -Wno-unused
> > > > > > gimple-match-exports.o-warn
> > > > > =
> > > > > > -Wno-unused dfp.o-warn = -Wno-strict-aliasing
> > > > > >
> > > > > > @@ -2674,12 +2673,10 @@ s-tm-texi:
> > > > > > build/genhooks$(build_exeext)
> > > > > $(srcdir)/doc/tm.texi.in
> > > > > >      false; \
> > > > > >    fi
> > > > > >
> > > > > > -$(GIMPLE_MATCH_PD_SEQ_SRC): s-gimple-match gimple-match-
> > > head.cc \
> > > > > > -            gimple-match-exports.cc; @true
> > > > > > -gimple-match-auto.h: s-gimple-match gimple-match-head.cc \
> > > > > > -            gimple-match-exports.cc; @true
> > > > > > +$(GIMPLE_MATCH_PD_SEQ_SRC): s-gimple-match gimple-match-
> > > head.cc;
> > > > > > +@true
> > > > > > +gimple-match-auto.h: s-gimple-match; @true
> > > > > > $(GENERIC_MATCH_PD_SEQ_SRC): s-generic-match
> > > > > > generic-match-head.cc; @true
> > > > > > -generic-match-auto.h: s-generic-match generic-match-head.cc;
> > > > > > @true
> > > > > > +generic-match-auto.h: s-generic-match; @true
> > > > > >
> > > > > > s-gimple-match: build/genmatch$(build_exeext) \
> > > > > >        $(srcdir)/match.pd cfn-operators.pd
> > > > > > --
> > > > > > 2.39.2
> > > > > >
> > > >
> >