diff mbox

s390-linux fails to build

Message ID 55B0D807.90808@redhat.com
State New
Headers show

Commit Message

Nick Clifton July 23, 2015, 12:03 p.m. UTC
Hi Helmut, Hi Ulrich, Hi Andreas,

   A toolchain configured as --target=s390-linux currently fails to build
   gcc because of an undefined function:

     undefined reference to `s390_host_detect_local_cpu(int, char const**)'
     Makefile:1858: recipe for target 'xgcc' failed

   The patch below fixes the problem for me by adding a stub function in
   s390-common.c, but I am not sure if it is the correct solution.
   Please can you advise ?

Cheers
   Nick

Comments

Jakub Jelinek July 23, 2015, 12:18 p.m. UTC | #1
On Thu, Jul 23, 2015 at 01:03:19PM +0100, Nick Clifton wrote:
> Hi Helmut, Hi Ulrich, Hi Andreas,
> 
>   A toolchain configured as --target=s390-linux currently fails to build
>   gcc because of an undefined function:
> 
>     undefined reference to `s390_host_detect_local_cpu(int, char const**)'
>     Makefile:1858: recipe for target 'xgcc' failed
> 
>   The patch below fixes the problem for me by adding a stub function in
>   s390-common.c, but I am not sure if it is the correct solution.
>   Please can you advise ?

Isn't it better to just follow what other arches do?
E.g. on i?86/x86_64, the EXTRA_SPEC_FUNCTIONS definition is guarded
with
#if defined(__i386__) || defined(__x86_64__)
and similarly on mips:
#if defined(__mips__)
and thus I'd expect s390{,x} should guard it with
#if defined(__s390__) || defined(__s390x__)
or so.

The config.host change also looks wrong, e.g. i?86 or mips have:
  i[34567]86-*-* \
  | x86_64-*-* )
    case ${target} in
      i[34567]86-*-* \
      | x86_64-*-* )
        host_extra_gcc_objs="driver-i386.o"
        host_xmake_file="${host_xmake_file} i386/x-i386"
        ;;
    esac
    ;;
  mips*-*-linux*)
    case ${target} in
      mips*-*-linux*)
        host_extra_gcc_objs="driver-native.o"
        host_xmake_file="${host_xmake_file} mips/x-native"
      ;;
    esac
    ;;
while s390 has:
  s390-*-* | s390x-*-*)
    host_extra_gcc_objs="driver-native.o"
    host_xmake_file="${host_xmake_file} s390/x-native"
    ;;
I bet that is gone break also cross-compilers from s390* to other targets.

	Jakub
Jakub Jelinek July 23, 2015, 12:56 p.m. UTC | #2
On Thu, Jul 23, 2015 at 02:46:43PM +0200, Ulrich Weigand wrote:
> This is supposed to be fixed by this pending patch:
> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01546.html

LGTM.

> > The config.host change also looks wrong, e.g. i?86 or mips have:
> >   i[34567]86-*-* \
> >   | x86_64-*-* )
> >     case ${target} in
> >       i[34567]86-*-* \
> >       | x86_64-*-* )
> >         host_extra_gcc_objs="driver-i386.o"
> >         host_xmake_file="${host_xmake_file} i386/x-i386"
> >         ;;
> >     esac
> >     ;;
> >   mips*-*-linux*)
> >     case ${target} in
> >       mips*-*-linux*)
> >         host_extra_gcc_objs="driver-native.o"
> >         host_xmake_file="${host_xmake_file} mips/x-native"
> >       ;;
> >     esac
> >     ;;
> > while s390 has:
> >   s390-*-* | s390x-*-*)
> >     host_extra_gcc_objs="driver-native.o"
> >     host_xmake_file="${host_xmake_file} s390/x-native"
> >     ;;
> > I bet that is gone break also cross-compilers from s390* to other targets.
> 
> I think this should be fine on s390.  The problem with i386 is that
> the driver-native.c file uses data types only defined by the i386
> target files (e.g. enum processor_type).  But on s390, the file does
> not any target-specific types and should be fully portable.

That hunk means that driver-native.o is added to EXTRA_GCC_OBJS
even say for s390x-*-* -> x86_64-*-* compiler.  While it might compile
there, nothing will use it, so what is it good for?
i?86/x86_64 backend will certainly not reference s390_host_detect_local_cpu
anywhere.

	Jakub
Jakub Jelinek July 23, 2015, 1:09 p.m. UTC | #3
On Thu, Jul 23, 2015 at 03:03:29PM +0200, Ulrich Weigand wrote:
> Jakub Jelinek wrote:
> > On Thu, Jul 23, 2015 at 02:46:43PM +0200, Ulrich Weigand wrote:
> > > > I bet that is gone break also cross-compilers from s390* to other targets.
> > > 
> > > I think this should be fine on s390.  The problem with i386 is that
> > > the driver-native.c file uses data types only defined by the i386
> > > target files (e.g. enum processor_type).  But on s390, the file does
> > > not any target-specific types and should be fully portable.
> > 
> > That hunk means that driver-native.o is added to EXTRA_GCC_OBJS
> > even say for s390x-*-* -> x86_64-*-* compiler.  While it might compile
> > there, nothing will use it, so what is it good for?
> > i?86/x86_64 backend will certainly not reference s390_host_detect_local_cpu
> > anywhere.
> 
> Oh, I agree this will not be *used*.  I just wanted to point out that it
> will not *break* cross-compilers as is.

I think it would be better for consistency and sanity do what other
targets do, even if it won't break the cross-compilation.
Do you agree?

	Jakub
Dominik Vogt July 23, 2015, 1:35 p.m. UTC | #4
On Thu, Jul 23, 2015 at 03:09:46PM +0200, Jakub Jelinek wrote:
> On Thu, Jul 23, 2015 at 03:03:29PM +0200, Ulrich Weigand wrote:
> > Oh, I agree this will not be *used*.  I just wanted to point out that it
> > will not *break* cross-compilers as is.
> 
> I think it would be better for consistency and sanity do what other
> targets do, even if it won't break the cross-compilation.
> Do you agree?

It's certainly better to not compile a file with code that was
written to run on a different platform.  I'll make a patch in a
couple of days.

Ciao

Dominik ^_^  ^_^
Dominik Vogt July 24, 2015, 8:43 a.m. UTC | #5
On Thu, Jul 23, 2015 at 03:03:29PM +0200, Ulrich Weigand wrote:
> Oh, I agree this will not be *used*.  I just wanted to point out that it
> will not *break* cross-compilers as is.

I've just verified that cross compilation *does* work at the
moment with the patches quoted in some earlier post.  (Andreas
will commit them shortly.)

Ciao

Dominik ^_^  ^_^
diff mbox

Patch

Index: gcc/common/config/s390/s390-common.c
===================================================================
--- gcc/common/config/s390/s390-common.c	(revision 226094)
+++ gcc/common/config/s390/s390-common.c	(working copy)
@@ -119,6 +119,14 @@ 
      }
  }

+const char * s390_host_detect_local_cpu (int, const char **) 
__attribute__((weak));
+const char *
+s390_host_detect_local_cpu (int argc ATTRIBUTE_UNUSED,
+			    const char **argv ATTRIBUTE_UNUSED)
+{
+  return NULL;
+}
+
  #undef TARGET_DEFAULT_TARGET_FLAGS
  #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT)