On Sat, Jun 06, 2026 at 03:03:27PM +0000, Jeffrey Law wrote:
> It looks generally sensible.  I think a cursory investigation of how 
> other host OSs handle the dot case and the results of your testing 
> probably drive the integrate as-is or iterate decision.

I think the first hunk is conceptionally correct and we also have other
guards spread around as e.g. in for_each_path() from gcc.cc where we
have

  if (do_multi && multilib_dir && strcmp (multilib_dir, ".") != 0)

However, with the second hunk I'm not so confident anymore.  There is
one place dealing explicitly with consecutive colons:

  /* When --disable-multilib was used but target defines
     MULTILIB_OSDIRNAMES, entries starting with .: (and not starting
     with .:: for multiarch configurations) are there just to find
     multilib_os_dir, so skip them from output.  */
  if (this_path[0] == '.' && this_path[1] == ':' && this_path[2] != ':')
    skip = 1;

Thus, my first impression that

  defaultosdirname=::${multiarch}

from genmultilib is a typo is not true.  Debugging this further is a bit
tedious since the format of the individual spec tokens is not documented
(or at least I didn't find it).  Therefore, when and in which forms two
consecutive colons are allowed/expected is not clear to me.  At least to
me it looks like that parser set_multilib_dir() from gcc.cc doesn't seem
to be prepared to accept specs containing consecutive colons.  What also
adds to my confusion is that multilib_os_dir is set to NULL if it equals
"."

  if (multilib_dir == NULL && multilib_os_dir != NULL
      && strcmp (multilib_os_dir, ".") == 0)
    {
      free (const_cast<char *> (multilib_os_dir));
      multilib_os_dir = NULL;
    }

but the same is not done for multilib_dir.  While reading this I was
wondering whether multilib_dir set to NULL or "." means something
different?  Though, this was just one of many questions which came up
while reading the code.

Long story short: I don't want to paper over and certainly not make it
worse.  Since having a multilib setup for a single ABI works for s390,
I'm inclined to drop this patch.  My initial intention was to make it
easier/straight forward to maintain (especially for others who are not
familiar with the history of the back end), however, the proposed
combination/path seems to be not supported and may introduce more
problems than it solves.  Sorry for the noise.

Cheers,
Stefan

Reply via email to