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 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?

> >
> > 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?

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
> >>

Reply via email to