On Wed, Mar 27, 2024 at 10:13 AM peter0x44 <peter0...@disroot.org> wrote:
>
> On 2024-03-27 01:58, Richard Biener wrote:
> > On Wed, Mar 27, 2024 at 9:13 AM Peter0x44 <peter0...@disroot.org>
> > wrote:
> >>
> >> I accidentally replied off-list. Sorry.
> >>
> >> 27 Mar 2024 8:09:30 am Peter0x44 <peter0...@disroot.org>:
> >>
> >>
> >> 27 Mar 2024 7:51:26 am Richard Biener <richard.guent...@gmail.com>:
> >>
> >> > On Tue, Mar 26, 2024 at 11:37 PM Peter Damianov <peter0...@disroot.org>
> >> > wrote:
> >> >>
> >> >> lto-wrapper generates Makefiles that use the following:
> >> >> touch -r file file.tmp && mv file.tmp file
> >> >> to truncate files.
> >> >> If there is no suitable "touch" or "mv" available, then this errors
> >> >> with
> >> >> "The system cannot find the file specified".
> >> >>
> >> >> The solution here is to check if sh -c true works, before trying to
> >> >> use it in
> >> >> the Makefile. If it doesn't, then fall back to "copy /y nul file"
> >> >> instead.
> >> >> The fallback doesn't work exactly the same (the modified time of the
> >> >> file gets
> >> >> updated), but this doesn't seem to matter in practice.
> >> >
> >> > I suppose it doesn't matter as we (no longer?) have the input as
> >> > dependency
> >> > on the rule so make doesn't get confused to re-build it. I guess we
> >> > only truncate
> >> > the file because it's going to be deleted by another process.
> >> >
> >> > Instead of doing sth like sh_exists I would suggest to simply use
> >> > #ifdef __WIN
> >> > or something like that? Not sure if we have suitable macros to
> >> > identify the
> >> > host operating system though and whether mingw, cygwin, etc. behave the
> >> > same
> >> > here.
> >>
> >> They do, I tested. Using sh_exists is deliberate, I've had to program
> >> on
> >> school computers that had cmd.exe disabled, but had busybox-w32
> >> working,
> >> so it might be more flexible in that way. I would prefer a solution
> >> which
> >> didn't require invoking cmd.exe if there is a working shell present.
> >
> > Hmm, but then I'd expect SHELL to be appropriately set in such
> > situation? So shouldn't sh_exists at least try to look at $SHELL?
> I'm not sure it would . On windows, make searches the PATH for an sh.exe
> if
> present, and then uses it by default. The relevant code is here:
> https://git.savannah.gnu.org/cgit/make.git/tree/src/variable.c#n1628
> >
> >> I figured doing the "sh_exists" is okay because there is a basically
> >> identical check for make.
> >>
> >> >
> >> > As a stop-gap solution doing
> >> >
> >> > ( @-touch -r ... ) || true
> >> >
> >> > might also work? Or another way to note to make the command can fail
> >> > without causing a problem.
> >>
> >> I don't think it would work. cmd.exe can't run subshells like this.
> >
> > Hmm, OK. So this is all for the case where 'make' is available (as you
> > say we check for that) but no POSIX command environment is
> > (IIRC both touch and mv are part of POSIX). Testing for 'touch' and
> > 'mv' would then be another option?
> I think it would work, but keep in mind they could be placed on the PATH
> but
> still invoked from cmd.exe, so you might have to be careful of the
> redirection
> syntax and no /dev/null. It's a bit more complexity that doesn't seem
> necessary, to me.
> >
> >> >
> >> > Another way would be to have a portable solution to truncate a file
> >> > (maybe even removing it would work). I don't think we should override
> >> > SHELL.
> >>
> >> Do you mean that perhaps an special command line argument could be
> >> added
> >> to lto-wrapper to do it, and then the makefile could invoke
> >> lto-wrapper
> >> to remove or truncate files instead of a shell? I'm not sure I get the
> >> proposed suggestion.
> >
> > The point is to truncate the file at the earliest point to reduce the
> > peak disk space required for a LTO link with many LTRANS units.
> > But yes, I guess it would be possible to add a flag to gcc itself
> > so the LTRANS compile would truncate the file. Currently we
> > emit sth like
> >
> > ./libquantum.ltrans0.ltrans.o:
> > @/space/rguenther/install/trunk-r14-8925/usr/local/bin/gcc
> > '-xlto' '-c' '-fno-openmp' '-fno-openacc' '-fno-pie'
> > '-fcf-protection=none' '-g' '-mtune=generic' '-march=x86-64' '-O2'
> > '-O2' '-g' '-v' '-save-temps' '-mtune=generic' '-march=x86-64'
> > '-dumpdir' 'libquantum.' '-dumpbase' './libquantum.ltrans0.ltrans'
> > '-fltrans' '-o' './libquantum.ltrans0.ltrans.o'
> > './libquantum.ltrans0.o'
> >
> > so adding a '-truncate-input' flag for the driver or even more
> > explicit '-truncate ./libquantum.ltrans0.o'
> > might be a more elegant solution then?
>
> This is much more elegant. I like it, I can take a look at implementing
> it.
> Would I be looking at gcc.cc for this?
Yes, you'd add the Driver option to gcc/common.opt and handle it from gcc/gcc.cc
Richard.
> Best wishes,
> Peter.
>
> >
> > Richard.
> >
> >> >
> >> > Richard.
> >> >
> >> >> I tested this both in environments both with and without sh present,
> >> >> and
> >> >> observed no issues.
> >> >>
> >> >> Signed-off-by: Peter Damianov <peter0...@disroot.org>
> >> >> ---
> >> >> gcc/lto-wrapper.cc | 35 ++++++++++++++++++++++++++++++++---
> >> >> 1 file changed, 32 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
> >> >> index 5186d040ce0..8dee0aaa2d8 100644
> >> >> --- a/gcc/lto-wrapper.cc
> >> >> +++ b/gcc/lto-wrapper.cc
> >> >> @@ -1389,6 +1389,27 @@ make_exists (void)
> >> >> return errmsg == NULL && exit_status == 0 && err == 0;
> >> >> }
> >> >>
> >> >> +/* Test that an sh command is present and working, return true if so.
> >> >> + This is only relevant for windows hosts, where a /bin/sh shell
> >> >> cannot
> >> >> + be assumed to exist. */
> >> >> +
> >> >> +static bool
> >> >> +sh_exists (void)
> >> >> +{
> >> >> + const char *sh = "sh";
> >> >> + const char *sh_args[] = {sh, "-c", "true", NULL};
> >> >> +#ifdef _WIN32
> >> >> + int exit_status = 0;
> >> >> + int err = 0;
> >> >> + const char *errmsg
> >> >> + = pex_one (PEX_SEARCH, sh_args[0], CONST_CAST (char **, sh_args),
> >> >> + "sh", NULL, NULL, &exit_status, &err);
> >> >> + return errmsg == NULL && exit_status == 0 && err == 0;
> >> >> +#else
> >> >> + return true;
> >> >> +#endif
> >> >> +}
> >> >> +
> >> >> /* Execute gcc. ARGC is the number of arguments. ARGV contains the
> >> >> arguments. */
> >> >>
> >> >> static void
> >> >> @@ -1402,6 +1423,7 @@ run_gcc (unsigned argc, char *argv[])
> >> >> const char *collect_gcc;
> >> >> char *collect_gcc_options;
> >> >> int parallel = 0;
> >> >> + bool have_sh = sh_exists ();
> >> >> int jobserver = 0;
> >> >> bool jobserver_requested = false;
> >> >> int auto_parallel = 0;
> >> >> @@ -2016,6 +2038,7 @@ cont:
> >> >> argv_ptr[5] = NULL;
> >> >> if (parallel)
> >> >> {
> >> >> + fprintf (mstream, "SHELL=%s\n", have_sh ? "sh" : "cmd");
> >> >> fprintf (mstream, "%s:\n\t@%s ", output_name,
> >> >> new_argv[0]);
> >> >> for (j = 1; new_argv[j] != NULL; ++j)
> >> >> fprintf (mstream, " '%s'", new_argv[j]);
> >> >> @@ -2024,9 +2047,15 @@ cont:
> >> >> truncate them as soon as we have processed it. This
> >> >> reduces temporary disk-space usage. */
> >> >> if (! save_temps)
> >> >> - fprintf (mstream, "\t@-touch -r \"%s\" \"%s.tem\" >
> >> >> /dev/null "
> >> >> - "2>&1 && mv \"%s.tem\" \"%s\"\n",
> >> >> - input_name, input_name, input_name,
> >> >> input_name);
> >> >> + {
> >> >> + fprintf (mstream,
> >> >> + have_sh
> >> >> + ? "\t@-touch -r \"%s\" \"%s.tem\" >
> >> >> /dev/null "
> >> >> + "2>&1 && mv \"%s.tem\" \"%s\"\n"
> >> >> + : "\t@-copy /y nul \"%s\" > NUL "
> >> >> + "2>&1\n",
> >> >> + input_name, input_name, input_name,
> >> >> input_name);
> >> >> + }
> >> >> }
> >> >> else
> >> >> {
> >> >> --
> >> >> 2.39.2
> >> >>
