On Mon, Apr 8, 2019 at 3:53 PM Martin Liška <mli...@suse.cz> wrote:
>
> On 4/8/19 3:50 PM, Martin Liška wrote:
> > On 4/8/19 2:42 PM, Richard Biener wrote:
> >> On Mon, Apr 8, 2019 at 2:26 PM Martin Liška <mli...@suse.cz> wrote:
> >>>
> >>> On 4/8/19 2:18 PM, Richard Biener wrote:
> >>>> On Mon, Apr 8, 2019 at 1:30 PM Martin Liška <mli...@suse.cz> wrote:
> >>>>>
> >>>>> On 4/8/19 12:08 PM, Richard Biener wrote:
> >>>>>> On Fri, Apr 5, 2019 at 12:42 PM Martin Liška <mli...@suse.cz> wrote:
> >>>>>>>
> >>>>>>> Hi.
> >>>>>>>
> >>>>>>> The patch adds a new config that makes LTO+PGO bootstrap faster by
> >>>>>>> using LTO only in stage4. In stage3, generators are build with LTO
> >>>>>>> in order to collect a reasonable profile for LTO FE.
> >>>>>>>
> >>>>>>> Ready for trunk?
> >>>>>>
> >>>>>> I wonder if you need the
> >>>>>>
> >>>>>> +AC_SUBST(GENERATOR_CFLAGS)
> >>>>>>
> >>>>>> at all, can't you just use
> >>>>>>
> >>>>>> +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >>>>>> +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >>>>>>
> >>>>>> ?
> >>>>>
> >>>>> I've just tested that and it does not work for me.
> >>>>
> >>>> Ah, you probably need to move the
> >>>>
> >>>> @@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
> >>>> CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
> >>>> LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
> >>>> prefix +] \
> >>>> CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
> >>>> + GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
> >>>> GENERATOR_CFLAGS; \
> >>>> CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev
> >>>> +] \
> >>>>
> >>>> change to the respective build targets.
> >>>
> >>> Can you please point me to a location in Makefile.tpl where
> >>> is the target? It's all Greek to me :)
> >>
> >> I think it's
> >>
> >> all-stage[+id+]-[+prefix+][+module+]:
> >> configure-stage[+id+]-[+prefix+][+module+]
> >> @[ $(current_stage) = stage[+id+] ] || $(MAKE) stage[+id+]-start
> >> @r=`${PWD_COMMAND}`; export r; \
> >> s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> >> TFLAGS="$(STAGE[+id+]_TFLAGS)"; \
> >> [+exports+][+ IF prev +] \
> >> [+poststage1_exports+][+ ENDIF prev +] [+extra_exports+] \
> >> cd [+subdir+]/[+module+] && \
> >> [+autoprofile+] \
> >> $(MAKE) $(BASE_FLAGS_TO_PASS)[+ IF prefix +] \
> >> CFLAGS="$(CFLAGS_FOR_TARGET)" \
> >> CXXFLAGS="$(CXXFLAGS_FOR_TARGET)" \
> >> LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"[+ ELSE prefix +] \
> >> CFLAGS="$(STAGE[+id+]_CFLAGS)" \
> >> CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"[+ IF prev +] \
> >> LIBCFLAGS="$(STAGE[+id+]_CFLAGS)"[+ ELSE prev +] \
> >> LIBCFLAGS="$(LIBCFLAGS)"[+ ENDIF prev +][+ ENDIF prefix +]
> >> \
> >> CFLAGS_FOR_TARGET="$(CFLAGS_FOR_TARGET)" \
> >> CXXFLAGS_FOR_TARGET="$(CXXFLAGS_FOR_TARGET)" \
> >> LIBCFLAGS_FOR_TARGET="$(LIBCFLAGS_FOR_TARGET)" \
> >> [+args+] [+IF prev +][+poststage1_args+][+ ELSE prev +] \
> >> [+stage1_args+][+ ENDIF prev +] [+extra_make_flags+] \
> >> TFLAGS="$(STAGE[+id+]_TFLAGS)" [+profile_data+] \
> >> $(TARGET-stage[+id+]-[+prefix+][+module+])
> >>
> >> where you maybe can simply add a
> >> GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)
> >>
> >> Richard.
> >>
> >>> Martin
> >>>
> >>>>
> >>>>>>
> >>>>>> Please mention in both bootstrap-lto-lean.mk and the documentation
> >>>>>> that the intended make target for this config is profiledbootstrap
> >>>>>> since for non-profiledbootstrap it ends up not using LTO at all. A
> >>>>>> "lean"
> >>>>>> mode for non-profiledbootstrap would need to set up things to
> >>>>>> use LTO only for stage3 which means not doing a bootstrap comparison
> >>>>>> which means we could "skip" stage2 as well here. So partial support
> >>>>>> for that would be to have
> >>>>>>
> >>>>>> STAGE2_CFLAGS += -frandom-seed=1
> >>>>>> STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
> >>>>>> ...
> >>>>>> do-compare = true
> >>>>>
> >>>>> Changed to tihs.
> >>>>>
> >>>>>>
> >>>>>> So if this works for non-profiledbootstrap the docs could be
> >>>>>> changed to say "but is intended for faster build by only
> >>>>>> using LTO in the final bootstrap stage. With @samp{...}
> >>>>>> the LTO frontend is trained only on generator files."
> >>>>>
> >>>>> Likewise.
> >>>>>
> >>>>>>
> >>>>>> (why do we need -frandom-seed=1? IIRC that was only for the
> >>>>>> comparison step which is elided in all cases for -lean.mk).
> >>>>>
> >>>>> Yep, it's not neded now.
> >>>>
> >>>> I see it still on STAGE[23]_CFLAGS? I think you can drop
> >>>> STAGE2_CFLAGS adjustment completely.
> >>>>
> >>>> Otherwise looks OK - mind trying one more time with the
> >>>> above suggestion for GENERATOR_CFLAGS?
> >>>>
> >>>> Thanks,
> >>>> Richard.
> >>>>
> >>>>> Martin
> >>>>>
> >>>>>>
> >>>>>> Richard.
> >>>>>>
> >>>>>>> Thanks,
> >>>>>>> Martin
> >>>>>>>
> >>>>>>> ChangeLog:
> >>>>>>>
> >>>>>>> 2019-04-05 Martin Liska <mli...@suse.cz>
> >>>>>>>
> >>>>>>> * Makefile.in: Regenerate.
> >>>>>>> * Makefile.tpl: Pass GENERATOR_CFLAGS
> >>>>>>> in all stages.
> >>>>>>>
> >>>>>>> config/ChangeLog:
> >>>>>>>
> >>>>>>> 2019-04-05 Martin Liska <mli...@suse.cz>
> >>>>>>>
> >>>>>>> * bootstrap-lto-lean.mk: New file.
> >>>>>>>
> >>>>>>> gcc/ChangeLog:
> >>>>>>>
> >>>>>>> 2019-04-05 Martin Liska <mli...@suse.cz>
> >>>>>>>
> >>>>>>> * Makefile.in: Use GENERATOR_CFLAGS for all generators.
> >>>>>>> * configure: Regenerate.
> >>>>>>> * configure.ac: Pass GENERATOR_CFLAGS.
> >>>>>>> * doc/install.texi: Document the new config.
> >>>>>>> ---
> >>>>>>> Makefile.in | 207
> >>>>>>> +++++++++++++++++++++++++++++++++++
> >>>>>>> Makefile.tpl | 2 +
> >>>>>>> config/bootstrap-lto-lean.mk | 19 ++++
> >>>>>>> gcc/Makefile.in | 4 +-
> >>>>>>> gcc/configure | 6 +-
> >>>>>>> gcc/configure.ac | 1 +
> >>>>>>> gcc/doc/install.texi | 6 +
> >>>>>>> 7 files changed, 241 insertions(+), 4 deletions(-)
> >>>>>>> create mode 100644 config/bootstrap-lto-lean.mk
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>
> >
> > Works for me. There's the update patch that also removes the usage
> > of 'frandom-seed=1'.
> >
> > Ready for trunk?
> > Thanks,
> > Martin
> >
>
> ENOPATCH fix.
@@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
prefix +] \
CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
+ GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
GENERATOR_CFLAGS; \
CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev +] \
LIBCFLAGS="$(STAGE[+id+]_CFLAGS)"[+ ELSE prev +] \
LIBCFLAGS="$(LIBCFLAGS)"[+ ENDIF prev +]; export LIBCFLAGS;[+
you can drop this hunk now.
@@ -1193,6 +1195,7 @@ all-stage[+id+]-[+prefix+][+module+]:
configure-stage[+id+]-[+prefix+][+module+]
CXXFLAGS="$(CXXFLAGS_FOR_TARGET)" \
LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"[+ ELSE prefix +] \
CFLAGS="$(STAGE[+id+]_CFLAGS)" \
+ GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)" \
CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"[+ IF prev +] \
LIBCFLAGS="$(STAGE[+id+]_CFLAGS)"[+ ELSE prev +] \
LIBCFLAGS="$(LIBCFLAGS)"[+ ENDIF prev +][+ ENDIF prefix +] \
please use tabs according to surrounding cases.
OK with that changes.
Richard.
> Martin
