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

login
register
mail settings
Submitter Dmitrijs Ledkovs
Date Aug. 12, 2010, 5:39 p.m.
Message ID <87eie34tnv.fsf@ubuntu.com>
Download mbox | patch
Permalink /patch/61632/
State New
Headers show

Comments

Dmitrijs Ledkovs - Aug. 12, 2010, 5:39 p.m.
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
Andrew Haley - Aug. 13, 2010, 9:01 a.m.
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.
Dmitrijs Ledkovs - Aug. 15, 2010, 11:41 p.m.
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?
Andrew Haley - Aug. 16, 2010, 9:22 a.m.
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.
Andrew Haley - Aug. 16, 2010, 9:37 a.m.
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.
Ralf Wildenhues - Aug. 16, 2010, 6:53 p.m.
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
Tom Tromey - Aug. 16, 2010, 9:03 p.m.
>>>>> "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
Dmitrijs Ledkovs - Aug. 17, 2010, 1:31 p.m.
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 =)
Dmitrijs Ledkovs - Aug. 17, 2010, 1:41 p.m.
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
>
Ralf Wildenhues - Aug. 17, 2010, 6:45 p.m.
* 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
Dmitrijs Ledkovs - Aug. 17, 2010, 8:31 p.m.
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
>
Dave Korn - Aug. 17, 2010, 11:09 p.m.
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
Dmitrijs Ledkovs - Aug. 26, 2010, 2:58 p.m.
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.

Patch

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