PR libgcj/40868 - ecjx should be build with host compiler

PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

This is a new attempt at the PR libgcj/40868 [1].

This time around a compiler variable is introduced in configure.ac and
Makefile.am is patched as well.

This has been tested with build/host=linux-gnu target=i686-w64-mingw32.
This has not been tested with a canadian-cross but "it should just work".

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40868


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ecjx.patch --]
[-- Type: text/x-diff, Size: 1931 bytes --]

From 8f20bb45ffebbd2cdf4cfc640624a371851fce2b Mon Sep 17 00:00:00 2001
From: Dmitrijs Ledkovs <dmitrij.ledkov@ubuntu.com>
Date: Wed, 7 Jul 2010 15:57:56 +0100
Subject: [PATCH 2/2] 2010-07-25  Dmitrijs Ledkovs  <dmitrij.ledkov@ubuntu.com>

	PR libgcj/40868
	* configure.ac: add GCC_FOR_ECJX variable
	* Makefile.am: use GCC_FOR_ECJX compiler to compile ecjx.o
	* Makefile.in: Regenerate.
	* configure: Regenerate.
	* gcj/Makefile.in: Regenerate.
	* include/Makefile.in: Regenerate.
	* testsuite/Makefile.in: Regenerate.
---
 libjava/Makefile.am  |    2 +-
 libjava/configure.ac |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/libjava/Makefile.am b/libjava/Makefile.am
index 7b67ed0..8930903 100644
--- a/libjava/Makefile.am
+++ b/libjava/Makefile.am
@@ -1161,7 +1161,7 @@ endif
 
 else !NATIVE
 
-ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
+ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
 ecjx_LDFLAGS = $(ECJX_BASE_FLAGS) $(ECJ_BUILD_JAR)
 ecjx_LDADD = 
 ecjx_DEPENDENCIES = 
diff --git a/libjava/configure.ac b/libjava/configure.ac
index 125e9ce..06e6f96 100644
--- a/libjava/configure.ac
+++ b/libjava/configure.ac
@@ -395,6 +395,7 @@ NATIVE=yes
 which_gcj=default
 host_exeext=${ac_exeext}
 GCJ_FOR_ECJX=
+GCC_FOR_ECJX=
 built_gcc_dir="`cd ${builddotdot}/../../${host_subdir}/gcc && ${PWDCMD-pwd}`"
 if test -n "${with_cross_host}"; then
   # We are being configured with a cross compiler. We can't
@@ -402,6 +403,7 @@ if test -n "${with_cross_host}"; then
   NATIVE=no
   cross_host_exeext=
   GCJ_FOR_ECJX="${with_cross_host}-gcj"
+  GCC_FOR_ECJX="${with_cross_host}-gcc"
   case "${with_cross_host}" in
      *mingw* | *cygwin*)
          cross_host_exeext=.exe
@@ -467,6 +469,7 @@ JAVAC="$GCJ -C"
 export JAVAC
 
 AC_SUBST(GCJ_FOR_ECJX)
+AC_SUBST(GCC_FOR_ECJX)
 AC_SUBST(GCJH)
 AC_SUBST(host_exeext)
 
-- 
1.7.0.4

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Andrew Haley]

On 08/12/2010 06:39 PM, Dmitrijs Ledkovs wrote:
> 	PR libgcj/40868
> 	* configure.ac: add GCC_FOR_ECJX variable
> 	* Makefile.am: use GCC_FOR_ECJX compiler to compile ecjx.o
> 	* Makefile.in: Regenerate.
> 	* configure: Regenerate.
> 	* gcj/Makefile.in: Regenerate.
> 	* include/Makefile.in: Regenerate.
> 	* testsuite/Makefile.in: Regenerate.

OK, thanks.

Andrew.

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

On 13 August 2010 12:01, Andrew Haley <aph@redhat.com> wrote:
> On 08/12/2010 06:39 PM, Dmitrijs Ledkovs wrote:
>>       PR libgcj/40868
>>       * configure.ac: add GCC_FOR_ECJX variable
>>       * Makefile.am: use GCC_FOR_ECJX compiler to compile ecjx.o
>>       * Makefile.in: Regenerate.
>>       * configure: Regenerate.
>>       * gcj/Makefile.in: Regenerate.
>>       * include/Makefile.in: Regenerate.
>>       * testsuite/Makefile.in: Regenerate.
>
> OK, thanks.
>
> Andrew.
>
>

Would someone please apply this patch to trunk, if there are no more comments?

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Andrew Haley]

On 08/16/2010 12:41 AM, Dmitrijs Ledkovs wrote:
> On 13 August 2010 12:01, Andrew Haley <aph@redhat.com> wrote:
>> On 08/12/2010 06:39 PM, Dmitrijs Ledkovs wrote:
>>>       PR libgcj/40868
>>>       * configure.ac: add GCC_FOR_ECJX variable
>>>       * Makefile.am: use GCC_FOR_ECJX compiler to compile ecjx.o
>>>       * Makefile.in: Regenerate.
>>>       * configure: Regenerate.
>>>       * gcj/Makefile.in: Regenerate.
>>>       * include/Makefile.in: Regenerate.
>>>       * testsuite/Makefile.in: Regenerate.
>>
>> OK, thanks.
>>
>> Andrew.
>>
>>
> 
> Would someone please apply this patch to trunk, if there are no more comments?

I'm testing it.

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Andrew Haley]

On 08/16/2010 12:41 AM, Dmitrijs Ledkovs wrote:
> On 13 August 2010 12:01, Andrew Haley <aph@redhat.com> wrote:
>> On 08/12/2010 06:39 PM, Dmitrijs Ledkovs wrote:
>>>       PR libgcj/40868
>>>       * configure.ac: add GCC_FOR_ECJX variable
>>>       * Makefile.am: use GCC_FOR_ECJX compiler to compile ecjx.o
>>>       * Makefile.in: Regenerate.
>>>       * configure: Regenerate.
>>>       * gcj/Makefile.in: Regenerate.
>>>       * include/Makefile.in: Regenerate.
>>>       * testsuite/Makefile.in: Regenerate.
>>
> 
> Would someone please apply this patch to trunk, if there are no more comments?

I tested it native, and it's fine.

Paging Tom Tromey:

All this patch does is substitute "${with_cross_host}-gcc" for "${with_cross_host}-gcj".
I don't understand why using the gcc driver rather than the gcj driver makes a difference.
Is this because the cross host at this point has gcc but not gcj?  If so, I guess
that's fine.

@@ -402,6 +403,7 @@
   NATIVE=no
   cross_host_exeext=
   GCJ_FOR_ECJX="${with_cross_host}-gcj"
+  GCC_FOR_ECJX="${with_cross_host}-gcc"
   case "${with_cross_host}" in
      *mingw* | *cygwin*)
          cross_host_exeext=.exe


-ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
+ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)

Andrew.

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Ralf Wildenhues]

Hello,

* Andrew Haley wrote on Mon, Aug 16, 2010 at 11:37:00AM CEST:
> -ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
> +ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)

Why should a _LINK variable (which is used for linking) contain -c?
Why should -o be followed by a variable name that presumably contains
several objects?

This hunk looks very wrong.  If it causes the right thing to happen,
then in a very hacky way.

Cheers,
Ralf

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

> Hello,
>
> * Andrew Haley wrote on Mon, Aug 16, 2010 at 11:37:00AM CEST:
>> -ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>> +ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>
> Why should a _LINK variable (which is used for linking) contain -c?

I couldn't figure out making a custom ecjx_COMPILE command for ecjx.cc
only. Hence I've used a "hack" in _LINK.

> Why should -o be followed by a variable name that presumably contains
> several objects?
>

In theory, yes. In reality, no. ecjx.cc is an empty file, needed to
create an "empty" object such that libtool can link-it with all the libs
it needs to be linked.

> This hunk looks very wrong.  If it causes the right thing to happen,
> then in a very hacky way.
>

Yes, the hunk looks wrong, the real issue is deeper. How to tie in
ecj.jar into the automake/libtool build-system. The current way works
for native builds, but building regular cross fails since ecj.o becomes
"target" object type, instead of "build/host" object type.

I better test would be trying to build canadian-cross & reverse-cross
libjava. I'm not quite sure, but it should be broken now and this patch
may or may not fix those too.

With this patch I have managed to build build/host=i686-linux-gnu
target=i686-w64-mingw32 libjava & gcj.

I would be valuable to find who thought up of ecjx.cc and put more
comments into ecjx.cc 

I have even less clue than reviewers in this thread =)

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Ralf Wildenhues]

* Dmitrijs Ledkovs wrote on Tue, Aug 17, 2010 at 03:31:37PM CEST:
> Ralf Wildenhues writes:
> > * Andrew Haley wrote on Mon, Aug 16, 2010 at 11:37:00AM CEST:
> >> -ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
> >> +ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
> >
> > Why should a _LINK variable (which is used for linking) contain -c?
> 
> I couldn't figure out making a custom ecjx_COMPILE command for ecjx.cc
> only. Hence I've used a "hack" in _LINK.

If this issue can wait for a few more days, I can look into it
(I forget the GCC build system after a few weeks of not hacking on it).

Cheers,
Ralf

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

On 17 August 2010 21:45, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> * Dmitrijs Ledkovs wrote on Tue, Aug 17, 2010 at 03:31:37PM CEST:
>> Ralf Wildenhues writes:
>> > * Andrew Haley wrote on Mon, Aug 16, 2010 at 11:37:00AM CEST:
>> >> -ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>> >> +ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>> >
>> > Why should a _LINK variable (which is used for linking) contain -c?
>>
>> I couldn't figure out making a custom ecjx_COMPILE command for ecjx.cc
>> only. Hence I've used a "hack" in _LINK.
>
> If this issue can wait for a few more days, I can look into it
> (I forget the GCC build system after a few weeks of not hacking on it).
>

Yes, this can wait =)

Dima.


> Cheers,
> Ralf
>

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dave Korn]

On 17/08/2010 19:45, Ralf Wildenhues wrote:
> * Dmitrijs Ledkovs wrote on Tue, Aug 17, 2010 at 03:31:37PM CEST:
>> Ralf Wildenhues writes:
>>> * Andrew Haley wrote on Mon, Aug 16, 2010 at 11:37:00AM CEST:
>>>> -ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>>>> +ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>>> Why should a _LINK variable (which is used for linking) contain -c?
>> I couldn't figure out making a custom ecjx_COMPILE command for ecjx.cc
>> only. Hence I've used a "hack" in _LINK.
> 
> If this issue can wait for a few more days, I can look into it
> (I forget the GCC build system after a few weeks of not hacking on it).

  That's not forgetting, that's PTSD!

    cheers,
      DaveK

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

> * Dmitrijs Ledkovs wrote on Tue, Aug 17, 2010 at 03:31:37PM CEST:
>> Ralf Wildenhues writes:
>> > * Andrew Haley wrote on Mon, Aug 16, 2010 at 11:37:00AM CEST:
>> >> -ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>> >> +ecjx_LINK = $(GCC_FOR_ECJX) -c -o $(am_ecjx_OBJECTS) $(top_srcdir)/$(ecjx_SOURCES) && $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
>> >
>> > Why should a _LINK variable (which is used for linking) contain -c?
>> 
>> I couldn't figure out making a custom ecjx_COMPILE command for ecjx.cc
>> only. Hence I've used a "hack" in _LINK.
>
> If this issue can wait for a few more days, I can look into it
> (I forget the GCC build system after a few weeks of not hacking on it).
>
> Cheers,
> Ralf
>

ping.

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Ralf Wildenhues]

[ http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00930.html ]

Sorry for the delay.

Below is my attempt.  Tested in native configuration.  Can you please
test the cross compile case?  You need to use --enable-maintainer-mode
or run 'cd libjava && autoreconf -v' explicitly for the patch to have
effect.  Thanks.

Tom, if that succeeds, OK to commit?

Notes:

The patch assumes that in the native setup, users expect
CXXFLAGS and GCJFLAGS to be applied to the compile resp. link.
The latter is already done, so the former seems just logical,
which is why the compile rule is placed in the !NATIVE block only.

Ensuring both of the variables are always nonempty was done for
consistency reasons.

The dummy object is needed because, well, there are gcj versions out
there that will refuse to link without finding an object on the command
line.  It could have probably just as well been created with the same
driver, but I don't think that this way makes things worse than they
were before.

Thanks,
Ralf

Fix building ecjx in cross compile mode.

2010-08-26  Dmitrijs Ledkovs  <dmitrij.ledkov@ubuntu.com>
	    Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

libjava/:
	PR libgcj/40868
	* configure.ac: Add GCC_FOR_ECJX variable.  Ensure
	GCJ_FOR_ECJX is always set.
	* Makefile.am [!NATIVE] (ecjx.$(OBJEXT)): New rule.
	* Makefile.in: Regenerate.
	* configure: Regenerate.
	* gcj/Makefile.in: Regenerate.
	* include/Makefile.in: Regenerate.
	* testsuite/Makefile.in: Regenerate.

diff --git a/libjava/Makefile.am b/libjava/Makefile.am
index dcdf827..52d0995 100644
--- a/libjava/Makefile.am
+++ b/libjava/Makefile.am
@@ -1170,6 +1170,8 @@ ecjx_LINK = $(GCJ_FOR_ECJX_LINK) $(ecjx_LDFLAGS)
 ecjx_LDFLAGS = $(ECJX_BASE_FLAGS) $(ECJ_BUILD_JAR)
 ecjx_LDADD = 
 ecjx_DEPENDENCIES = 
+ecjx.$(OBJEXT): ecjx.cc
+	$(GCC_FOR_ECJX) -c $<
 
 endif !NATIVE
 
diff --git a/libjava/configure.ac b/libjava/configure.ac
index 685cb72..df6bca3 100644
--- a/libjava/configure.ac
+++ b/libjava/configure.ac
@@ -394,7 +394,8 @@ NATIVE=yes
 # Which gcj and host gcj (for ecjx) do we use?
 which_gcj=default
 host_exeext=${ac_exeext}
-GCJ_FOR_ECJX=
+GCJ_FOR_ECJX='$(GCJ)'
+GCC_FOR_ECJX='$(CXX)'
 built_gcc_dir="`cd ${builddotdot}/../../${host_subdir}/gcc && ${PWDCMD-pwd}`"
 if test -n "${with_cross_host}"; then
   # We are being configured with a cross compiler. We can't
@@ -402,6 +403,7 @@ if test -n "${with_cross_host}"; then
   NATIVE=no
   cross_host_exeext=
   GCJ_FOR_ECJX="${with_cross_host}-gcj"
+  GCC_FOR_ECJX="${with_cross_host}-gcc"
   case "${with_cross_host}" in
      *mingw* | *cygwin*)
          cross_host_exeext=.exe
@@ -467,6 +469,7 @@ JAVAC="$GCJ -C"
 export JAVAC
 
 AC_SUBST(GCJ_FOR_ECJX)
+AC_SUBST(GCC_FOR_ECJX)
 AC_SUBST(GCJH)
 AC_SUBST(host_exeext)
 

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Tom Tromey]

>>>>> "Ralf" == Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

Ralf> Below is my attempt.  Tested in native configuration.  Can you please
Ralf> test the cross compile case?  You need to use --enable-maintainer-mode
Ralf> or run 'cd libjava && autoreconf -v' explicitly for the patch to have
Ralf> effect.  Thanks.

Ralf> Tom, if that succeeds, OK to commit?

Yes, thanks.

Tom

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

On 26 August 2010 19:49, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> [ http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00930.html ]
>
> Sorry for the delay.
>
> Below is my attempt.  Tested in native configuration.  Can you please
> test the cross compile case?  You need to use --enable-maintainer-mode
> or run 'cd libjava && autoreconf -v' explicitly for the patch to have
> effect.  Thanks.
>

Successfully build cross-compiler: build/host i686-gnu-linux & target
= i686-w64-mingw32.

Thanks a lot for looking at this issue.

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Tom Tromey]

>>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:

Andrew> All this patch does is substitute "${with_cross_host}-gcc" for
Andrew> "${with_cross_host}-gcj".  I don't understand why using the gcc
Andrew> driver rather than the gcj driver makes a difference.  Is this
Andrew> because the cross host at this point has gcc but not gcj?  If
Andrew> so, I guess that's fine.

I still don't really understand why the dummy ecjx.cc is needed.
But since it is, the important thing here is to arrange to compile it
with the proper gcc -- the one that matches GCJX_FOR_ECJX.

I think the configure parts of this patch looks ok, but I don't see how
this patch arranges for ecjx.cc to be compiled by GCC_FOR_ECJX.

Also, what Ralf said.

Tom

Re: PR libgcj/40868 - ecjx should be build with host compiler

[Dmitrijs Ledkovs]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:
>
> Andrew> All this patch does is substitute "${with_cross_host}-gcc" for
> Andrew> "${with_cross_host}-gcj".  I don't understand why using the gcc
> Andrew> driver rather than the gcj driver makes a difference.  Is this
> Andrew> because the cross host at this point has gcc but not gcj?  If
> Andrew> so, I guess that's fine.
>
> I still don't really understand why the dummy ecjx.cc is needed.

Me neither. All I know ecjx.cc works in native builds, and doesn't in
regular cross-builds. Trying to fix regular cross-builds, not the
"why-ecjx.cc-exists?". 

> But since it is, the important thing here is to arrange to compile it
> with the proper gcc -- the one that matches GCJX_FOR_ECJX.
>

As far as I can see ecjx.o should be the same type as the other objects
linked to produce a host binary. But I couldn't figure out how to get
the regular gcc, since it doesn't seem to be passed to the libjava
configure at all.

The config.log shows xgcc for CC and CXX.

> I think the configure parts of this patch looks ok, but I don't see how
> this patch arranges for ecjx.cc to be compiled by GCC_FOR_ECJX.
>

First compiled "wrong" way, then _LINK target removes bogus object file
&& compile using GCC_FOR_ECJX && do the actual link.

> Also, what Ralf said.
>
> Tom
>