diff mbox series

cygwin: Don't try to support multilibs [PR107998]

Message ID Y/XfcHZqlRUGJ+GQ@tucnak
State Unresolved
Headers
Series cygwin: Don't try to support multilibs [PR107998] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Feb. 22, 2023, 9:25 a.m. UTC 
  Hi!

As discussed in the PR, t-cygwin-w64 file has been introduced in 2013
and has one important problem, two different multilib options -m64 and -m32,
but MULTILIB_DIRNAMES with just one word in it.
Before the genmultilib sanity checking was added, my understanding is that
this essentially resulted in effective --disable-multilib,
$ gcc -print-multi-lib
.;
;@m32
$ gcc -print-multi-directory
.
$ gcc -print-multi-directory -m64
.
$ gcc -print-multi-directory -m32

$ gcc -print-multi-os-directory
../lib
$ gcc -print-multi-os-directory -m64
../lib
$ gcc -print-multi-os-directory -m32
../lib32
and because of the way e.g. config-ml.in operates
multidirs=
for i in `${CC-gcc} --print-multi-lib 2>/dev/null`; do
  dir=`echo $i | sed -e 's/;.*$//'`
  if [ "${dir}" = "." ]; then
    true
  else
    if [ -z "${multidirs}" ]; then
      multidirs="${dir}"
    else
      multidirs="${multidirs} ${dir}"
    fi
  fi
done
dir was . first time (and so nothing was done) and empty
second time, multidirs empty too, so multidirs was set to empty
like it would be with --disable-multilib.

With the added sanity checking the build fails unless --disable-multilib
is used in configure (dunno whether people usually configure that way
on cygwin).

From what has been said in the PR, multilibs were not meant to be supported
and e.g. cygwin headers probably aren't ready for it.

So the following patch just removes the file with the (incorrect) multilib
stuff instead of fixing it (say by setting MULTILIB_DIRNAMES to 64 32).

I have no way to test this though, no Windows around, can anyone please
test this?  I just would like to get some progress on the P1s we have...

2023-02-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/107998
	* config.gcc (x86_64-*-cygwin*): Don't add i386/t-cygwin-w64 into
	$tmake_file.
	* config/i386/t-cygwin-w64: Remove.


	Jakub

Comments

Jonathan Yong Feb. 22, 2023, 1 p.m. UTC | #1 
On 2/22/23 09:25, Jakub Jelinek wrote:
> Hi!
> 
> As discussed in the PR, t-cygwin-w64 file has been introduced in 2013
> and has one important problem, two different multilib options -m64 and -m32,
> but MULTILIB_DIRNAMES with just one word in it.
> Before the genmultilib sanity checking was added, my understanding is that
> this essentially resulted in effective --disable-multilib,
> $ gcc -print-multi-lib
> .;
> ;@m32
> $ gcc -print-multi-directory
> .
> $ gcc -print-multi-directory -m64
> .
> $ gcc -print-multi-directory -m32
> 
> $ gcc -print-multi-os-directory
> ../lib
> $ gcc -print-multi-os-directory -m64
> ../lib
> $ gcc -print-multi-os-directory -m32
> ../lib32
> and because of the way e.g. config-ml.in operates
> multidirs=
> for i in `${CC-gcc} --print-multi-lib 2>/dev/null`; do
>    dir=`echo $i | sed -e 's/;.*$//'`
>    if [ "${dir}" = "." ]; then
>      true
>    else
>      if [ -z "${multidirs}" ]; then
>        multidirs="${dir}"
>      else
>        multidirs="${multidirs} ${dir}"
>      fi
>    fi
> done
> dir was . first time (and so nothing was done) and empty
> second time, multidirs empty too, so multidirs was set to empty
> like it would be with --disable-multilib.
> 
> With the added sanity checking the build fails unless --disable-multilib
> is used in configure (dunno whether people usually configure that way
> on cygwin).
> 
>>From what has been said in the PR, multilibs were not meant to be supported
> and e.g. cygwin headers probably aren't ready for it.
> 
> So the following patch just removes the file with the (incorrect) multilib
> stuff instead of fixing it (say by setting MULTILIB_DIRNAMES to 64 32).
> 
> I have no way to test this though, no Windows around, can anyone please
> test this?  I just would like to get some progress on the P1s we have...
> 
> 2023-02-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/107998
> 	* config.gcc (x86_64-*-cygwin*): Don't add i386/t-cygwin-w64 into
> 	$tmake_file.
> 	* config/i386/t-cygwin-w64: Remove.
> 
> --- gcc/config.gcc.jj	2023-02-18 12:38:30.803025062 +0100
> +++ gcc/config.gcc	2023-02-21 17:07:12.143164563 +0100
> @@ -2105,7 +2105,7 @@ x86_64-*-cygwin*)
>   	need_64bit_isa=yes
>   	tm_file="${tm_file} i386/unix.h i386/bsd.h i386/gas.h i386/cygming.h i386/cygwin.h i386/cygwin-w64.h i386/cygwin-stdint.h"
>   	xm_file=i386/xm-cygwin.h
> -	tmake_file="${tmake_file} i386/t-cygming t-slibgcc i386/t-cygwin-w64"
> +	tmake_file="${tmake_file} i386/t-cygming t-slibgcc"
>   	target_gtfiles="$target_gtfiles \$(srcdir)/config/i386/winnt.cc"
>   	extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
>   	extra_objs="${extra_objs} winnt.o winnt-stubs.o"
> --- gcc/config/i386/t-cygwin-w64.jj	2020-01-12 11:54:36.333414616 +0100
> +++ gcc/config/i386/t-cygwin-w64	2023-02-21 17:06:44.121572616 +0100
> @@ -1,3 +0,0 @@
> -MULTILIB_OPTIONS = m64/m32
> -MULTILIB_DIRNAMES = 64
> -MULTILIB_OSDIRNAMES = ../lib ../lib32
> 
> 	Jakub
> 

Achim,

Mind checking this out?
Jonathan Yong Feb. 22, 2023, 1:02 p.m. UTC | #2 
On 2/22/23 09:25, Jakub Jelinek wrote:
> Hi!
> 
> As discussed in the PR, t-cygwin-w64 file has been introduced in 2013
> and has one important problem, two different multilib options -m64 and -m32,
> but MULTILIB_DIRNAMES with just one word in it.
> Before the genmultilib sanity checking was added, my understanding is that
> this essentially resulted in effective --disable-multilib,
> $ gcc -print-multi-lib
> .;
> ;@m32
> $ gcc -print-multi-directory
> .
> $ gcc -print-multi-directory -m64
> .
> $ gcc -print-multi-directory -m32
> 
> $ gcc -print-multi-os-directory
> ../lib
> $ gcc -print-multi-os-directory -m64
> ../lib
> $ gcc -print-multi-os-directory -m32
> ../lib32
> and because of the way e.g. config-ml.in operates
> multidirs=
> for i in `${CC-gcc} --print-multi-lib 2>/dev/null`; do
>    dir=`echo $i | sed -e 's/;.*$//'`
>    if [ "${dir}" = "." ]; then
>      true
>    else
>      if [ -z "${multidirs}" ]; then
>        multidirs="${dir}"
>      else
>        multidirs="${multidirs} ${dir}"
>      fi
>    fi
> done
> dir was . first time (and so nothing was done) and empty
> second time, multidirs empty too, so multidirs was set to empty
> like it would be with --disable-multilib.
> 
> With the added sanity checking the build fails unless --disable-multilib
> is used in configure (dunno whether people usually configure that way
> on cygwin).
> 
>>From what has been said in the PR, multilibs were not meant to be supported
> and e.g. cygwin headers probably aren't ready for it.
> 
> So the following patch just removes the file with the (incorrect) multilib
> stuff instead of fixing it (say by setting MULTILIB_DIRNAMES to 64 32).
> 
> I have no way to test this though, no Windows around, can anyone please
> test this?  I just would like to get some progress on the P1s we have...
> 
> 2023-02-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/107998
> 	* config.gcc (x86_64-*-cygwin*): Don't add i386/t-cygwin-w64 into
> 	$tmake_file.
> 	* config/i386/t-cygwin-w64: Remove.
> 
> --- gcc/config.gcc.jj	2023-02-18 12:38:30.803025062 +0100
> +++ gcc/config.gcc	2023-02-21 17:07:12.143164563 +0100
> @@ -2105,7 +2105,7 @@ x86_64-*-cygwin*)
>   	need_64bit_isa=yes
>   	tm_file="${tm_file} i386/unix.h i386/bsd.h i386/gas.h i386/cygming.h i386/cygwin.h i386/cygwin-w64.h i386/cygwin-stdint.h"
>   	xm_file=i386/xm-cygwin.h
> -	tmake_file="${tmake_file} i386/t-cygming t-slibgcc i386/t-cygwin-w64"
> +	tmake_file="${tmake_file} i386/t-cygming t-slibgcc"
>   	target_gtfiles="$target_gtfiles \$(srcdir)/config/i386/winnt.cc"
>   	extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
>   	extra_objs="${extra_objs} winnt.o winnt-stubs.o"
> --- gcc/config/i386/t-cygwin-w64.jj	2020-01-12 11:54:36.333414616 +0100
> +++ gcc/config/i386/t-cygwin-w64	2023-02-21 17:06:44.121572616 +0100
> @@ -1,3 +0,0 @@
> -MULTILIB_OPTIONS = m64/m32
> -MULTILIB_DIRNAMES = 64
> -MULTILIB_OSDIRNAMES = ../lib ../lib32
> 
> 	Jakub
> 

Achim, mind looking at this?
Resending due to mail client problems, hopefully not a duplicate.
Jakub Jelinek March 3, 2023, 6:44 p.m. UTC | #3 
On Wed, Feb 22, 2023 at 01:02:58PM +0000, Jonathan Yong wrote:
> On 2/22/23 09:25, Jakub Jelinek wrote:
> > As discussed in the PR, t-cygwin-w64 file has been introduced in 2013
> > and has one important problem, two different multilib options -m64 and -m32,
> > but MULTILIB_DIRNAMES with just one word in it.
> > Before the genmultilib sanity checking was added, my understanding is that
> > this essentially resulted in effective --disable-multilib,
> > $ gcc -print-multi-lib
> > .;
> > ;@m32
> > $ gcc -print-multi-directory
> > .
> > $ gcc -print-multi-directory -m64
> > .
> > $ gcc -print-multi-directory -m32
> > 
> > $ gcc -print-multi-os-directory
> > ../lib
> > $ gcc -print-multi-os-directory -m64
> > ../lib
> > $ gcc -print-multi-os-directory -m32
> > ../lib32
> > and because of the way e.g. config-ml.in operates
> > multidirs=
> > for i in `${CC-gcc} --print-multi-lib 2>/dev/null`; do
> >    dir=`echo $i | sed -e 's/;.*$//'`
> >    if [ "${dir}" = "." ]; then
> >      true
> >    else
> >      if [ -z "${multidirs}" ]; then
> >        multidirs="${dir}"
> >      else
> >        multidirs="${multidirs} ${dir}"
> >      fi
> >    fi
> > done
> > dir was . first time (and so nothing was done) and empty
> > second time, multidirs empty too, so multidirs was set to empty
> > like it would be with --disable-multilib.
> > 
> > With the added sanity checking the build fails unless --disable-multilib
> > is used in configure (dunno whether people usually configure that way
> > on cygwin).
> > 
> > > From what has been said in the PR, multilibs were not meant to be supported
> > and e.g. cygwin headers probably aren't ready for it.
> > 
> > So the following patch just removes the file with the (incorrect) multilib
> > stuff instead of fixing it (say by setting MULTILIB_DIRNAMES to 64 32).
> > 
> > I have no way to test this though, no Windows around, can anyone please
> > test this?  I just would like to get some progress on the P1s we have...
> > 
> > 2023-02-22  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR target/107998
> > 	* config.gcc (x86_64-*-cygwin*): Don't add i386/t-cygwin-w64 into
> > 	$tmake_file.
> > 	* config/i386/t-cygwin-w64: Remove.
> > 
> > --- gcc/config.gcc.jj	2023-02-18 12:38:30.803025062 +0100
> > +++ gcc/config.gcc	2023-02-21 17:07:12.143164563 +0100
> > @@ -2105,7 +2105,7 @@ x86_64-*-cygwin*)
> >   	need_64bit_isa=yes
> >   	tm_file="${tm_file} i386/unix.h i386/bsd.h i386/gas.h i386/cygming.h i386/cygwin.h i386/cygwin-w64.h i386/cygwin-stdint.h"
> >   	xm_file=i386/xm-cygwin.h
> > -	tmake_file="${tmake_file} i386/t-cygming t-slibgcc i386/t-cygwin-w64"
> > +	tmake_file="${tmake_file} i386/t-cygming t-slibgcc"
> >   	target_gtfiles="$target_gtfiles \$(srcdir)/config/i386/winnt.cc"
> >   	extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
> >   	extra_objs="${extra_objs} winnt.o winnt-stubs.o"
> > --- gcc/config/i386/t-cygwin-w64.jj	2020-01-12 11:54:36.333414616 +0100
> > +++ gcc/config/i386/t-cygwin-w64	2023-02-21 17:06:44.121572616 +0100
> > @@ -1,3 +0,0 @@
> > -MULTILIB_OPTIONS = m64/m32
> > -MULTILIB_DIRNAMES = 64
> > -MULTILIB_OSDIRNAMES = ../lib ../lib32
> 
> Achim, mind looking at this?
> Resending due to mail client problems, hopefully not a duplicate.

NightStrike on IRC said he has tested the patch and it worked fine.

Is the patch ok for trunk then?

	Jakub
Jakub Jelinek March 10, 2023, 9:37 a.m. UTC | #4 
Hi!

I'd like to ping this patch (as I wrote a week ago, NightStrike has tested
it):

On Fri, Mar 03, 2023 at 07:44:47PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > > 2023-02-22  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > > 	PR target/107998
> > > 	* config.gcc (x86_64-*-cygwin*): Don't add i386/t-cygwin-w64 into
> > > 	$tmake_file.
> > > 	* config/i386/t-cygwin-w64: Remove.
> > > 
> > > --- gcc/config.gcc.jj	2023-02-18 12:38:30.803025062 +0100
> > > +++ gcc/config.gcc	2023-02-21 17:07:12.143164563 +0100
> > > @@ -2105,7 +2105,7 @@ x86_64-*-cygwin*)
> > >   	need_64bit_isa=yes
> > >   	tm_file="${tm_file} i386/unix.h i386/bsd.h i386/gas.h i386/cygming.h i386/cygwin.h i386/cygwin-w64.h i386/cygwin-stdint.h"
> > >   	xm_file=i386/xm-cygwin.h
> > > -	tmake_file="${tmake_file} i386/t-cygming t-slibgcc i386/t-cygwin-w64"
> > > +	tmake_file="${tmake_file} i386/t-cygming t-slibgcc"
> > >   	target_gtfiles="$target_gtfiles \$(srcdir)/config/i386/winnt.cc"
> > >   	extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
> > >   	extra_objs="${extra_objs} winnt.o winnt-stubs.o"
> > > --- gcc/config/i386/t-cygwin-w64.jj	2020-01-12 11:54:36.333414616 +0100
> > > +++ gcc/config/i386/t-cygwin-w64	2023-02-21 17:06:44.121572616 +0100
> > > @@ -1,3 +0,0 @@
> > > -MULTILIB_OPTIONS = m64/m32
> > > -MULTILIB_DIRNAMES = 64
> > > -MULTILIB_OSDIRNAMES = ../lib ../lib32
> > 
> > Achim, mind looking at this?
> > Resending due to mail client problems, hopefully not a duplicate.
> 
> NightStrike on IRC said he has tested the patch and it worked fine.
> 
> Is the patch ok for trunk then?

	Jakub
Jonathan Yong March 10, 2023, 10:19 a.m. UTC | #5 
On 3/10/23 09:37, Jakub Jelinek wrote:
> Hi!
> 
> I'd like to ping this patch (as I wrote a week ago, NightStrike has tested
> it):
> 

Thanks, pushed to master branch.
diff mbox series

Patch

--- gcc/config.gcc.jj	2023-02-18 12:38:30.803025062 +0100
+++ gcc/config.gcc	2023-02-21 17:07:12.143164563 +0100
@@ -2105,7 +2105,7 @@  x86_64-*-cygwin*)
 	need_64bit_isa=yes
 	tm_file="${tm_file} i386/unix.h i386/bsd.h i386/gas.h i386/cygming.h i386/cygwin.h i386/cygwin-w64.h i386/cygwin-stdint.h"
 	xm_file=i386/xm-cygwin.h
-	tmake_file="${tmake_file} i386/t-cygming t-slibgcc i386/t-cygwin-w64"
+	tmake_file="${tmake_file} i386/t-cygming t-slibgcc"
 	target_gtfiles="$target_gtfiles \$(srcdir)/config/i386/winnt.cc"
 	extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
 	extra_objs="${extra_objs} winnt.o winnt-stubs.o"
--- gcc/config/i386/t-cygwin-w64.jj	2020-01-12 11:54:36.333414616 +0100
+++ gcc/config/i386/t-cygwin-w64	2023-02-21 17:06:44.121572616 +0100
@@ -1,3 +0,0 @@ 
-MULTILIB_OPTIONS = m64/m32
-MULTILIB_DIRNAMES = 64
-MULTILIB_OSDIRNAMES = ../lib ../lib32