Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

> On 26 Jun 2018, at 07:38, Alexandre Oliva  wrote:
> 
> Here's the patch I'll install if nobody objects in the next few days.
> Tested on x86_64-linux-gnu with a gcc bootstrap tree, a gcc
> non-bootstrap tree, and a binutils+gdb tree.

Thanks a lot for this Alex!

Re: fix libcc1 dependencies in toplevel Makefile

[Alexandre Oliva]

On Jun 11, 2018, Alexandre Oliva  wrote:

> On Jun  3, 2018, Alexandre Oliva  wrote:
>> On Jun 27, 2017, Alexandre Oliva  wrote:

> 1. address the previously-mentioned fragility in the patch I posted, to
> catch all cases of postbootstrap targets and their deps on
> non-postbootstrap targets.

This turned out to just require some thinking to convince myself it
wouldn't come up.

There was a major problem in the earlier patch, however: @if/@endif
gcc-no-bootstrap wasn't quite what we needed to enclose the preexisting
deps, because that works for cases in which gcc is built but not
bootstrapped, but not cases in which gcc is not built.  I had to
introduce @unless/@endunless to express the desired semantics.


Here's the patch I'll install if nobody objects in the next few days.
Tested on x86_64-linux-gnu with a gcc bootstrap tree, a gcc
non-bootstrap tree, and a binutils+gdb tree.

In the patch below, I've omitted hunks with only whitespace changes to
Makefile.in, so that people can more easily identify how rules are
changing.



Introduce @unless/@endunless and postbootstrap Makefile targets

From: Alexandre Oliva 

This patch turns dependencies of non-bootstrap targets on bootstrap
targets for bootstrap builds into dependencies on stage_last.  This
arrangement gets stage1-bubble to run from stage_last if we haven't
started a bootstrap yet, and to use the current stage otherwise.  This
was already the case of target libs, just not of non-bootstrapped host
modules.

In order to retain preexisting dependencies in non-bootstrap builds,
or in gcc-less builds, this introduces support for @unless/@endunless
pairs in Makefile.in.

There is a remaining possibility of problem if activating, in a tree
configured for bootstrap, a parallel build of two or more modules, at
least one bootstrapped and one not.  In this case, make might decide
to build stage_current and stage_last in parallel, the latter will
start a submake to build stage1 while the initial make, having
satisfied stage_current, proceeds to build the bootstrapped module in
non-bootstrapped configurations.  The two builds will overlap and will
likely conflict.  This situation does NOT arise in normal settings,
however: a post-bootstrap build of all-host all-target will indeed
activate such targets concurrently, but only after building all
bootstrapped modules successfully, and it will have both stage_last
and stage_current targets already satisfied, so the potential race
between builds will not arise.

Another remaining problem, that is slightly expanded with this patch,
is that of an interrupted build in a tree configured for bootstrap,
continued with a non-bootstrapped target.  Target modules that were
not bootstrapped would already fail to complete the current stage when
activated explicitly in the command line for a retry; host modules,
however, would attempt to build their bootstrapped dependencies, which
is what led to the problem of concurrent builds addressed with this
patch.  An interrupted or failed build might still recover correctly,
if the non-bootstrapped target is activated in both builds, because
then make will remove stage_last when its build command is
interrupted, so that it will attempt to recreate it with stage1-bubble
in the second try.  A bootstrap build, however, will not be attempting
to build stage_last, so the file will remain and the retry won't go
through stage1-bubble.  We have lived with that for target modules, so
we can probably live with that for host modules too.

Another undesirable consequence of this change is that non-boostrapped
host modules, in a tree configured for bootstrap, when activated as
make all-, will build all of stage1 instead of only the
module's usual dependencies.  This is intentional and necessary to fix
the parallel-build problem.  If it's not desirable, disabling the
unnecessary bootstrap configuration will suffice to restore the
original set of dependencies.


for  ChangeLog

* configure.ac: Introduce support for @unless/@endunless.
* Makefile.tpl (dep-kind): Rewrite with cond; return
postbootstrap in some cases.
(make-postboot-dep, postboot-targets): New.
(dependencies): Do not output postbootstrap dependencies at
first.  Output non-target ones changed for configure to depend
on stage_last @if gcc-bootstrap, and the original deps @unless
gcc-bootstrap.
* configure.in, Makefile.in: Rebuilt.
---
 Makefile.in  |  181 +-
 Makefile.tpl |   78 +
 configure|   20 +-
 configure.ac |   20 +-
 4 files changed, 134 insertions(+), 165 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 32a92a6bcd17..e0dfad337a6c 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -56060,9 +56018,7 @@ all-stagefeedback-fixincludes: 
maybe-all-stagefeedback-libiberty
 all-stageautoprofile-fixincludes: maybe-all-stageautoprofile-libiberty
 

Re: fix libcc1 dependencies in toplevel Makefile

[Jeff Law]

On 06/11/2018 08:50 PM, Alexandre Oliva wrote:
> So I see two possible ways to go from now:
> 
> 1. address the previously-mentioned fragility in the patch I posted, to
> catch all cases of postbootstrap targets and their deps on
> non-postbootstrap targets.
> 
> 
> 2. revamp the bootstrap/non-bootstrap dependencies, using GNU make
> conditionals rather than configure-time enable/disable-bootstrap, so
> that we can have a different set of dependencies while running the
> bootstrap proper, having non-stage dependencies activated by default
> when any of the all-* targets are named in the command line, and also
> when building post-bootstrap all-host all-target.  This might seem to
> bring the problem back, but rather by having the full dependency set,
> we'd avoid the race not by refraining from reentering dirs, but rather
> by having them entered or reentered according to the full dependencies,
> without mixing stage and non-stage dependencies.  I'm not yet sure this
> is actually doable, but it seems to me that if it is, it would be more
> robust than what we have now.
Your call.  I've wanted the build system revamped for 20+ years, but
it's nontrivial and the most serious problems were addressed as we
continued to pull the runtime bits out of gcc/

Jeff

Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

Hi Alex,

Thanks for your feedback and help looking into this.

> On 12 Jun 2018, at 04:50, Alexandre Oliva  wrote:
> 
> I was missing one possibility: that the problem occurred during the
> post-bootstrap all-host all-target build.  As far as I can tell from
> Nicolas' analysis, this was indeed the case.

Yes, indeed. I intended to convey this in the
opening message of this thread by referring to concurrency
between libcc1 and libquadmath. That was admittedly
too implicit :)


> I still don't see how any
> staging or unstaging might have taken place, but I can now see that we
> do reenter the gcc dir before building all-libcc1.  If that reentering
> rebuilds anything, particularly headers, that may be enough to explain
> the reported symptoms.

Right.

> Now, I do vaguely recall build output within the gcc subdir that
> possibly recreated the gcc/include subtree, which might explain the
> observed errors.

That's consistent at least, as the problem we had was the compilation
of a libquadmath source not finding limits.h.
 
> The good news is that the patch I posted the other day actually
> addresses this problem: the dep on stage_last is not enough to trigger a
> rebuild of gcc, so a post-bootstrap all-host all-target build will not
> reenter the bootstrapped dirs,

Nice :-)

> but that dep does trigger an initial
> build of gcc if one has not gone through bootstrapping yet.
> 
> So I see two possible ways to go from now:
> 
> 1. address the previously-mentioned fragility in the patch I posted, to
> catch all cases of postbootstrap targets and their deps on
> non-postbootstrap targets.
> 
> 2. revamp the bootstrap/non-bootstrap dependencies, using GNU make
> conditionals rather than configure-time enable/disable-bootstrap, so
> that we can have a different set of dependencies while running the
> bootstrap proper, having non-stage dependencies activated by default
> when any of the all-* targets are named in the command line, and also
> when building post-bootstrap all-host all-target.  This might seem to
> bring the problem back, but rather by having the full dependency set,
> we'd avoid the race not by refraining from reentering dirs, but rather
> by having them entered or reentered according to the full dependencies,
> without mixing stage and non-stage dependencies.  I'm not yet sure this
> is actually doable, but it seems to me that if it is, it would be more
> robust than what we have now.

I'm really not familiar enough with the dependencies organization
to provide informed input here.

Maybe a reasonable effort on 1 would be good enough in practice and
we can get to 2 only as a second step if we still observe failures.

Or start with a reasonable effort on 2 to evaluate feasibility and 
get a rough guesstimate of the effort it would take to get there, then
reassess.

For sure, I'm happy to try any patch in our development (!production)
builds and see where this leads.

Thanks again for your help on this!

With Kind Regards,

Olivier

Re: fix libcc1 dependencies in toplevel Makefile

[Alexandre Oliva]

On Jun  3, 2018, Alexandre Oliva  wrote:

> On Jun 27, 2017, Alexandre Oliva  wrote:

>> configuration, because the current Makefile would only do that with
>> all-host, after bootstrap is complete.

> I have extensively studied the dependencies, and I still don't see how
> all-libcc1, that is only activated as a target during the post-bootstrap
> all-host build, might have been activated concurrently with
> staging/unstaging.  By the time we get to all-host, we've sequentially
> completed bootstrap, compare, and unstage.

> The only possibilities I see of something going wrong as described is a
> parallel build that has bootstrap and postbootstrap targets in the
> command line, or some patch that changes the dependencies so that such
> targets are considered in parallel.

> I could definitely use the build logs from back then, if still
> available, to try to make sense of the problem.

I was missing one possibility: that the problem occurred during the
post-bootstrap all-host all-target build.  As far as I can tell from
Nicolas' analysis, this was indeed the case.  I still don't see how any
staging or unstaging might have taken place, but I can now see that we
do reenter the gcc dir before building all-libcc1.  If that reentering
rebuilds anything, particularly headers, that may be enough to explain
the reported symptoms.

Now, I do vaguely recall build output within the gcc subdir that
possibly recreated the gcc/include subtree, which might explain the
observed errors.

The good news is that the patch I posted the other day actually
addresses this problem: the dep on stage_last is not enough to trigger a
rebuild of gcc, so a post-bootstrap all-host all-target build will not
reenter the bootstrapped dirs, but that dep does trigger an initial
build of gcc if one has not gone through bootstrapping yet.


So I see two possible ways to go from now:

1. address the previously-mentioned fragility in the patch I posted, to
catch all cases of postbootstrap targets and their deps on
non-postbootstrap targets.


2. revamp the bootstrap/non-bootstrap dependencies, using GNU make
conditionals rather than configure-time enable/disable-bootstrap, so
that we can have a different set of dependencies while running the
bootstrap proper, having non-stage dependencies activated by default
when any of the all-* targets are named in the command line, and also
when building post-bootstrap all-host all-target.  This might seem to
bring the problem back, but rather by having the full dependency set,
we'd avoid the race not by refraining from reentering dirs, but rather
by having them entered or reentered according to the full dependencies,
without mixing stage and non-stage dependencies.  I'm not yet sure this
is actually doable, but it seems to me that if it is, it would be more
robust than what we have now.

-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist

Re: fix libcc1 dependencies in toplevel Makefile

[Alexandre Oliva]

On Jun 27, 2017, Alexandre Oliva  wrote:

> On Jun 26, 2017, Olivier Hainque  wrote:
>> On Jun 22, 2017, at 14:12 , Alexandre Oliva  wrote:
>>> Your patch takes care of the build dependencies of libcc1, which should
>>> avoid some scenarios that might lead to concurrency between staged and
>>> non-staged builds.  However, I don't see that it ensures libcc1 will be
>>> built after GCC in bootstrap scenarios; it might do so under 'make
>>> bootstrap', but probably not under 'make all-libcc1'.  I think we may
>>> need some additional bootstrap-only explicit dependency for that to work
>>> properly.


> On Jun 27, 2017, Olivier Hainque  wrote:

>>> On Jun 26, 2017, at 09:16 , Olivier Hainque  wrote:

>> make -j 32 BOOT_LDFLAGS=-Wl,--stack=0x200 CC=gcc 'ADAFLAGS=-W
>> -Wall -gnatpg -gnata -gnatws -gnatU -gnatyN' CXXFLAGS=-O2
>> BOOT_CFLAGS=-O2 CFLAGS=-O2 'LN_S=cp -p' 'BOOT_ADAFLAGS=-gnatpgn
>> -gnatU' 'STAGE1_CFLAGS=-O2 -O0 -g' bootstrap

> Thanks.  Given that 'bootstrap' is the only requested make target, we
> can be assured that something iffy took place.  What I can't figure out
> is how we even tried to build libcc1 during bootstrap, under that
> configuration, because the current Makefile would only do that with
> all-host, after bootstrap is complete.

I have extensively studied the dependencies, and I still don't see how
all-libcc1, that is only activated as a target during the post-bootstrap
all-host build, might have been activated concurrently with
staging/unstaging.  By the time we get to all-host, we've sequentially
completed bootstrap, compare, and unstage.

The only possibilities I see of something going wrong as described is a
parallel build that has bootstrap and postbootstrap targets in the
command line, or some patch that changes the dependencies so that such
targets are considered in parallel.

I could definitely use the build logs from back then, if still
available, to try to make sense of the problem.

> Yeah.  Running all-gcc While unstage does its directory-moving dance
> can't be good.  We can't have them both.

But then, again, I don't see anything that would ever attempt to build
these in parallel.

We do have other non-bootstrap targets that depend on bootstrap targets,
and that would presumably trigger the same problem, if there is one
indeed.


> So, would you like to give the automatic figuring out of
> non-bootstrap-on-bootstrap deps in dependencies, and guard them between
> @if gcc-no-bootstrap and @endif (then both configure- and all- libcc1
> deps would be adjusted this way)?

The patch below introduces this, but...  without understanding how the
problem comes about, it's hard to tell whether it would actually fix
anything.  Really, I don't really think it would fix anything: if there
is something that activates all-libcc1 (or any other non-bootstrap
target) in parallel with bootstrap, we will likely get parallel build
errors, with or without the patch.  The patch might make such failures
more visible, which might be a good thing, but I'm not sure it's worth
the trouble: it will annoy maintainers of other tools who use uberbaum
trees, because if they don't --disable-bootstrap at configure time,
their tools (e.g. gdb) will build all of GCC stage1 before starting the
GDB build.

It is a bit fragile, too.  For example, it could be improved (and
perhaps actually reduce the likelihood of triggering parallel builds of
postbootstrap and bootstrap targets) by first detecting all
postbootstrap targets, and then mapping *all* of their dependencies on
non-postbootstrap targets to stage_last.  As it is, there is a
possibility of direct dependencies of postbootstrap on prebootstrap
targets, which makes room for the prebootstrap targets to be activated
in parallel with (rather than just as part of) the bootstrap build.

It wouldn't be hard to do that, and I'd get to it if I thought it would
actually improve something, which would require a better understanding
of the observed failures.  As it is, I'm posting this just for the
record, because I don't think it's really of any use :-(


The sad news is that there really isn't any way (AFAICT) to catch
situations that are known to get us in trouble, namely, when parallel
builds are started with both bootstrap and postbootstrap targets.  I
think we'd need all of the bootstrap logic to be encoded in a
single-level Makefile (i.e., not recursive makes) to avoid problems with
such builds, and that in turn would require postbootstrap targets to
depend on the last stage.  That would require a lot of Makefile
reworking, for very little benefit.



Introduce postbootstrap Makefile targets

This patch turns dependencies of non-bootstrap targets on bootstrap
targets for bootstrap builds into dependencies on stage_last.


for  ChangeLog

* Makefile.tpl (dep-kind): Return postbootstrap in some cases.
(make-postboot-dep, postboot-targets): New.
(dependencies): Do not output postbootstrap dependencies at
first.  Output them 

Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

Hi Alex,

(Back from a few days away)

> On 27 Jun 2017, at 21:50, Alexandre Oliva  wrote:
> 
>> I don't quite understand this: we're using the same prerequisite as target
>> libraries, e.g. all-target-libstdc++-v3 or all-target-libbacktrace
> 
> Not quite.  Target libraries have deps on e.g. target-libgcc, look below
> the following comments in Makefile.in:
> 
> # Dependencies for target modules on other target modules are
> # described by lang_env_dependencies; the defaults apply to anything
> # not mentioned there.
> 
> plus, maybe-configure*-target-libgcc depend on maybe-all*-gcc (see above
> those comments).  The precise deps vary per bootstrap level, or
> non-bootstrap.
> 
> But after the proposed patch there are no such deps for libcc1 in the
> bootstrap case, so we might very well attempt to build libcc1 in
> parallel with gcc.  We shouldn't do that.
> 
> But then, it all works out because we only build all-host after
> bootstrap is complete; all-stage* doesn't depend on libcc1 at all.

I think I see.

[...]

> So, would you like to give the automatic figuring out of
> non-bootstrap-on-bootstrap deps in dependencies, and guard them between
> @if gcc-no-bootstrap and @endif (then both configure- and all- libcc1
> deps would be adjusted this way)?  (I'm not saying it should be trivial
> to do or anything like that; I'm not all that familiar with it and I'd
> have to figure it out myself if I were to do it, but I think that would
> be better than adding yet another means of introducing dependencies,
> while leaving another risky dep in place)


I'm willing to study this more and see what can be done
to improve things further. There are still a few details I don't
quite grasp so it'll just take a bit of time.

Thanks a lot for the additional set of extensive comments!

With Kind Regards,

Olivier

Re: fix libcc1 dependencies in toplevel Makefile

[Alexandre Oliva]

On Jun 26, 2017, Olivier Hainque  wrote:

> On Jun 22, 2017, at 14:12 , Alexandre Oliva  wrote:
>> Your patch takes care of the build dependencies of libcc1, which should
>> avoid some scenarios that might lead to concurrency between staged and
>> non-staged builds.  However, I don't see that it ensures libcc1 will be
>> built after GCC in bootstrap scenarios; it might do so under 'make
>> bootstrap', but probably not under 'make all-libcc1'.  I think we may
>> need some additional bootstrap-only explicit dependency for that to work
>> properly.

> I don't quite understand this: we're using the same prerequisite as target
> libraries, e.g. all-target-libstdc++-v3 or all-target-libbacktrace

Not quite.  Target libraries have deps on e.g. target-libgcc, look below
the following comments in Makefile.in:

# Dependencies for target modules on other target modules are
# described by lang_env_dependencies; the defaults apply to anything
# not mentioned there.

plus, maybe-configure*-target-libgcc depend on maybe-all*-gcc (see above
those comments).  The precise deps vary per bootstrap level, or
non-bootstrap.

But after the proposed patch there are no such deps for libcc1 in the
bootstrap case, so we might very well attempt to build libcc1 in
parallel with gcc.  We shouldn't do that.

But then, it all works out because we only build all-host after
bootstrap is complete; all-stage* doesn't depend on libcc1 at all.


> and I don't see other deps for these either.

> I don't see why the sequencing constraints for libcc1 should be tighter
> than those for the target libraries.

It was not about making them tighter, just about making them present.
Right now, in the bootstrap case, they're entirely implicit, by the fact
that we complete bootstrap first, then proceed to build all-host
all-target.  This deserves at least a comment somewhere, perhaps next to
libcc1 in Makefile.def, or next to depgcc.

Something to the effect that depgcc brings in a necessary dependency
that is implicit in the bootstrap case by the fact that we firt
bootstrap, then proceed to build all-host all-target.

Perhaps instead of depgcc=true, we should have a new flag in
dependencies that indicates the dep should be non-bootstrap only.  Or
maybe the code that implements dependencies could figure it out on its
own, when it sees a dep between a non-bootstrap module and a bootstrap
one, and generate the deps within @if gcc-no-bootstrap/@endif.

I think this would get us the behavior we want in both bootstrap and
non-bootstrap cases, including the libcc1 configure dep that, as it is,
might cause GCC to be configured in parallel given the right (or rather
wrong) conditions.


On Jun 27, 2017, Olivier Hainque  wrote:

>> On Jun 26, 2017, at 09:16 , Olivier Hainque  wrote:

> make -j 32 BOOT_LDFLAGS=-Wl,--stack=0x200 CC=gcc 'ADAFLAGS=-W
> -Wall -gnatpg -gnata -gnatws -gnatU -gnatyN' CXXFLAGS=-O2
> BOOT_CFLAGS=-O2 CFLAGS=-O2 'LN_S=cp -p' 'BOOT_ADAFLAGS=-gnatpgn
> -gnatU' 'STAGE1_CFLAGS=-O2 -O0 -g' bootstrap

Thanks.  Given that 'bootstrap' is the only requested make target, we
can be assured that something iffy took place.  What I can't figure out
is how we even tried to build libcc1 during bootstrap, under that
configuration, because the current Makefile would only do that with
all-host, after bootstrap is complete.

> From the logs of discussions we tracked, the understanding
> of the dependency issue was that we *had* (before the patch),
> possibilities to have stage_current and maybe-all-gcc targets
> built concurrently, via

>> configure-target-libquadmath: stage_current
>> all-target-libquadmath: configure-target-libquadmath
>> maybe-all-target-libquadmath: all-target-libquadmath

>> all-target: maybe-all-target-libquadmath

> on the one hand,

>> all-libcc1: maybe-all-gcc

>> maybe-all-libcc1: all-libcc1

>> all-host: maybe-all-libcc1

> on the other hand.

> Does that make sense ?

Yeah.  Running all-gcc While unstage does its directory-moving dance
can't be good.  We can't have them both.


So, would you like to give the automatic figuring out of
non-bootstrap-on-bootstrap deps in dependencies, and guard them between
@if gcc-no-bootstrap and @endif (then both configure- and all- libcc1
deps would be adjusted this way)?  (I'm not saying it should be trivial
to do or anything like that; I'm not all that familiar with it and I'd
have to figure it out myself if I were to do it, but I think that would
be better than adding yet another means of introducing dependencies,
while leaving another risky dep in place)

Thanks,

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

Hi Alex,

> On Jun 26, 2017, at 09:16 , Olivier Hainque  wrote:
> 
>> I'd like to understand better what the concurrency problem is with the
>> current build machinery, before we proceed with this change.  If you
>> manage to trigger the problem again, could you try to further analyze
>> build logs to check for e.g. concurrent activation of all-gcc in both
>> the top-level Makefile and the recursed-into-for-stage1 Makefile, or
>> somesuch?  Something else worth considering is what the make targets
>> specified in the command line were.
> 
> The problems were showing pretty rarely, only on certain hosts, in
> certain load conditions. We should still have the logs around and I'll
> look into this. They are regular logs, without -d. I can almost for sure
> fetch the exact "make" command line involved.

This was:

make -j 32 BOOT_LDFLAGS=-Wl,--stack=0x200 CC=gcc 'ADAFLAGS=-W -Wall -gnatpg 
-gnata -gnatws -gnatU -gnatyN' CXXFLAGS=-O2 BOOT_CFLAGS=-O2 CFLAGS=-O2 'LN_S=cp 
-p' 'BOOT_ADAFLAGS=-gnatpgn -gnatU' 'STAGE1_CFLAGS=-O2 -O0 -g' bootstrap

From the logs of discussions we tracked, the understanding
of the dependency issue was that we *had* (before the patch),
possibilities to have stage_current and maybe-all-gcc targets
built concurrently, via

> configure-target-libquadmath: stage_current
> all-target-libquadmath: configure-target-libquadmath
> maybe-all-target-libquadmath: all-target-libquadmath

> all-target: maybe-all-target-libquadmath

on the one hand,

> all-libcc1: maybe-all-gcc

> maybe-all-libcc1: all-libcc1

> all-host: maybe-all-libcc1

on the other hand.

Does that make sense ?

Thanks for your feedback!

(Note that I'll be away from tomorrow to Monday)

Olivier

Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

Hello Alex,

Thanks for the review and for the extensive comments on this,
much appreciated :)

> On Jun 22, 2017, at 14:12 , Alexandre Oliva  wrote:
> 
> On Jun 13, 2017, Olivier Hainque  wrote:
> 
>> 2017-06-13  Olivier Hainque  
> 
>>  * Makefile.def (host_modules): Set depgcc to true for libcc1,
>>  meaning need of a dep on stage_current if gcc-bootstrap and on
>>  maybe-all-gcc otherwise.
>>  (dependencies) Remove unconditional dependency on all-gcc.
> 
>>  * Makefile.tpl ("all" targets): Handle depgcc.
>>  * Makefile.in: Regenerate
> 
> This looks reasonable to me.  libcc1 is weird.  It's not a target
> library, it doesn't use the current stage tools for building.  It might
> as well not have any deps on the current stage's gcc, if it weren't for
> the fact that it includes headers from the current stage's gcc and links
> with current stage's host libraries, and even its configure reads from
> files created in current stage's gcc configuration.
> 
> So, it needs to be built after gcc and its host deps are built, and it
> needs to be configured after gcc is configured.  However, it is not part
> of the bootstrap, and we avoid building it more than once even in a
> bootstrap build.  That's what makes it special and tricky.

OK, thanks for summarizing the areas of intricacy.

> Your patch takes care of the build dependencies of libcc1, which should
> avoid some scenarios that might lead to concurrency between staged and
> non-staged builds.  However, I don't see that it ensures libcc1 will be
> built after GCC in bootstrap scenarios; it might do so under 'make
> bootstrap', but probably not under 'make all-libcc1'.  I think we may
> need some additional bootstrap-only explicit dependency for that to work
> properly.

I don't quite understand this: we're using the same prerequisite as target
libraries, e.g. all-target-libstdc++-v3 or all-target-libbacktrace, and I
don't see other deps for these either.

I don't see why the sequencing constraints for libcc1 should be tighter
than those for the target libraries.

I certainly don't grasp all the ramifications of the particularities you
outlined above, though.

> Furthermore, the patch does not take care of the configure dependencies
> of libcc1, so I think there might still be room for trouble, depending
> on what make targets are concurrently requested.  I'm not entirely sure
> this is true, though.

To my knowledge, we have never observed a problem in this area, assuming
our understanding of the problems we saw was correct :)

> I'd like to understand better what the concurrency problem is with the
> current build machinery, before we proceed with this change.  If you
> manage to trigger the problem again, could you try to further analyze
> build logs to check for e.g. concurrent activation of all-gcc in both
> the top-level Makefile and the recursed-into-for-stage1 Makefile, or
> somesuch?  Something else worth considering is what the make targets
> specified in the command line were.

The problems were showing pretty rarely, only on certain hosts, in
certain load conditions. We should still have the logs around and I'll
look into this. They are regular logs, without -d. I can almost for sure
fetch the exact "make" command line involved.

We had performed some analysis of what was happening, to our understanding.
I'll dig this out as well.

> All this said, I do agree that explicit deps on maybe-all-gcc are a
> likely source of trouble;

OK

> AFAICT all other host modules that are to be
> built after gcc depend on some target lib too.  Perhaps that brings some
> dep that libcc1 should have too...

This relates to the comments above. I guess I don't understand
what libcc1 might need that target libs wouldn't need.

Olivier

Re: fix libcc1 dependencies in toplevel Makefile

[Alexandre Oliva]

On Jun 13, 2017, Olivier Hainque  wrote:

> 2017-06-13  Olivier Hainque  

>   * Makefile.def (host_modules): Set depgcc to true for libcc1,
>   meaning need of a dep on stage_current if gcc-bootstrap and on
>   maybe-all-gcc otherwise.
>   (dependencies) Remove unconditional dependency on all-gcc.

>   * Makefile.tpl ("all" targets): Handle depgcc.
>   * Makefile.in: Regenerate
 
This looks reasonable to me.  libcc1 is weird.  It's not a target
library, it doesn't use the current stage tools for building.  It might
as well not have any deps on the current stage's gcc, if it weren't for
the fact that it includes headers from the current stage's gcc and links
with current stage's host libraries, and even its configure reads from
files created in current stage's gcc configuration.

So, it needs to be built after gcc and its host deps are built, and it
needs to be configured after gcc is configured.  However, it is not part
of the bootstrap, and we avoid building it more than once even in a
bootstrap build.  That's what makes it special and tricky.

Your patch takes care of the build dependencies of libcc1, which should
avoid some scenarios that might lead to concurrency between staged and
non-staged builds.  However, I don't see that it ensures libcc1 will be
built after GCC in bootstrap scenarios; it might do so under 'make
bootstrap', but probably not under 'make all-libcc1'.  I think we may
need some additional bootstrap-only explicit dependency for that to work
properly.

Furthermore, the patch does not take care of the configure dependencies
of libcc1, so I think there might still be room for trouble, depending
on what make targets are concurrently requested.  I'm not entirely sure
this is true, though.

I'd like to understand better what the concurrency problem is with the
current build machinery, before we proceed with this change.  If you
manage to trigger the problem again, could you try to further analyze
build logs to check for e.g. concurrent activation of all-gcc in both
the top-level Makefile and the recursed-into-for-stage1 Makefile, or
somesuch?  Something else worth considering is what the make targets
specified in the command line were.

All this said, I do agree that explicit deps on maybe-all-gcc are a
likely source of trouble; AFAICT all other host modules that are to be
built after gcc depend on some target lib too.  Perhaps that brings some
dep that libcc1 should have too...

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

> On 15 Jun 2017, at 14:03, Nathan Sidwell  wrote:
> 
> On 06/14/2017 01:24 PM, Olivier Hainque wrote:
> 
>>> I'm happy to try the patch.
>> That would bring useful extra datapoints, Thanks!
> 
> I now seem unable to trigger the race with an unpatched source. Sorry

No pb. Thanks for trying.

We were seeing the failures only in pretty specific circumstances (particular
machine & load conditions etc)

The patch remains worth considering IMO. At least, it would be interesting to
get feedback from a build-system maintainer on the correctness of the current
dependencies.

Our understanding is that it is incorrect to have libcc1 depend on
maybe-all-gcc in a bootstrapping setup because it can trigger rebuilds of gcc/
components in parallel with ongoing processing of target components (with
pre-requisites on stage_current only).

Re: fix libcc1 dependencies in toplevel Makefile

[Nathan Sidwell]

On 06/14/2017 01:24 PM, Olivier Hainque wrote:


I'm happy to try the patch.


That would bring useful extra datapoints, Thanks!


I now seem unable to trigger the race with an unpatched source. Sorry


--
Nathan Sidwell

Re: fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

> On Jun 14, 2017, at 13:39 , Nathan Sidwell  wrote:
> 
> Olivier,
>> During highly parallel builds on fast hosts, we have experienced
>> sporadic bootstrap failures on libquadmath like
> 
> I have encountered such a bootstrap problem too.  I guessed dependency race 
> condition, but -j21 was a simpler fix :)

I see :)

> I'm happy to try the patch.

That would bring useful extra datapoints, Thanks!

The patch might be more complex than it needs to be. 

The logic is very simple: I wasn't sure whether I could
add 

 @if gcc-no-bootstrap
 all-[+prefix+][+module+]: maybe-all-gcc
 @endif gcc-no-bootstrap

to all "all" targets after the dependency to stage_current
@if gcc-bootstrap (in Makefile.tpl).

The "depgcc" boolean is simply a mechanism to do that
only for cases where we were adding a extra explicit dependency
to maybe-all-gcc before, that is, only for libcc1.

Olivier

Re: fix libcc1 dependencies in toplevel Makefile

[Nathan Sidwell]

Olivier,

During highly parallel builds on fast hosts, we have experienced
sporadic bootstrap failures on libquadmath like


I have encountered such a bootstrap problem too.  I guessed dependency 
race condition, but -j21 was a simpler fix :)


I'm happy to try the patch.

nathan

--
Nathan Sidwell

fix libcc1 dependencies in toplevel Makefile

[Olivier Hainque]

Hello,

During highly parallel builds on fast hosts, we have experienced
sporadic bootstrap failures on libquadmath like

  In file included from ../../../src/libquadmath/printf/printf_fp.c:39:0:
  ../../../src/libquadmath/printf/quadmath-printf.h:24:20: fatal error: 
.../build/./gcc/include-fixed/limits.h: No such file or directory
  #include 

A pretty clear sign of a race condition caused by some inaccuracy in the
dependency statements.

Investigation led us to suspect this piece in the toplevel Makefile.in:

  all-libcc1: maybe-all-gcc

which differs from all the other dependencies on maybe-all-gcc in that it's
unconditional whereas the other ones are conditioned on @if gcc-no-bootstrap.

(Thanks to Nico Roche, cc'ed for the worked involved in finding this out)

Our understanding is that it's incorrect to have dependencies on maybe-all-gcc
in the bootstrap case; that this should be a dependency on stage_current
instead.

This patch is a proposal to address this by first removing the following
statement in Makefile.def:

  dependencies = { module=all-libcc1; on=all-gcc; };

(which emits the dependency unconditionally), then refining the expansion
of "all" targets in Makefile.tpl so they include a possible dep conditioned by
gcc-no-bootstrap, on demand for "host_module"s that ask for it by way of a new
"depgcc" parameter.

We have been using this in-house for months now. The sporadic failures
have disappeared since then and we haven't observed any related fallout
so far.

Bootstrapped and regression tested on x86_64-linux.

OK to commit ?

Thanks in advance for your feedback,

With Kind Regards,

Olivier

2017-06-13  Olivier Hainque  

* Makefile.def (host_modules): Set depgcc to true for libcc1,
meaning need of a dep on stage_current if gcc-bootstrap and on
maybe-all-gcc otherwise.
(dependencies) Remove unconditional dependency on all-gcc.

* Makefile.tpl ("all" targets): Handle depgcc.
* Makefile.in: Regenerate
 


libcc1-deps.diff
Description: Binary data