public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Olivier Hainque <hainque@adacore.com>
To: Alexandre Oliva <aoliva@redhat.com>
Cc: Olivier Hainque <hainque@adacore.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
	Nicolas Roche <roche@adacore.com>
Subject: Re: fix libcc1 dependencies in toplevel Makefile
Date: Mon, 26 Jun 2017 07:41:00 -0000	[thread overview]
Message-ID: <F36A0A1A-EC61-46D5-B343-FA839C4DF287@adacore.com> (raw)
In-Reply-To: <orzid0rzk1.fsf@lxoliva.fsfla.org>

Hello Alex,

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

> On Jun 22, 2017, at 14:12 , Alexandre Oliva <aoliva@redhat.com> wrote:
> 
> On Jun 13, 2017, Olivier Hainque <hainque@adacore.com> wrote:
> 
>> 2017-06-13  Olivier Hainque  <hainque@adacore.com>
> 
>> 	* 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

  reply	other threads:[~2017-06-26  7:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-13 12:58 Olivier Hainque
2017-06-14 11:39 ` Nathan Sidwell
2017-06-14 21:11   ` Olivier Hainque
2017-06-15 12:03     ` Nathan Sidwell
2017-06-15 12:29       ` Olivier Hainque
2017-06-22 12:13 ` Alexandre Oliva
2017-06-26  7:41   ` Olivier Hainque [this message]
2017-06-27 16:32     ` Olivier Hainque
2017-06-27 19:53     ` Alexandre Oliva
2017-07-03 21:05       ` Olivier Hainque
2018-06-03 19:13       ` Alexandre Oliva
2018-06-12  2:50         ` Alexandre Oliva
2018-06-12  8:57           ` Olivier Hainque
2018-06-12 15:31           ` Jeff Law
2018-06-26  5:39           ` Alexandre Oliva
2018-06-27 19:53             ` Olivier Hainque

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F36A0A1A-EC61-46D5-B343-FA839C4DF287@adacore.com \
    --to=hainque@adacore.com \
    --cc=aoliva@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=roche@adacore.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).