| Message ID | 87eie34tnv.fsf@ubuntu.com |
|---|---|
| State | New |
| Headers | show |
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.
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?
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.
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.
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
>>>>> "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
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 =)
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 >
* 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
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 >
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
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.
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