diff mbox

RFA: PR42843: Fix selection of compiler for native bootstrap. (Was: Re: [PATCH] Fix PR42843 to use built compilers)

Message ID 20100714162707.gz4dld54qowgos88-nzlynne@webmail.spamcop.net
State New
Headers show

Commit Message

Joern Rennecke July 14, 2010, 8:27 p.m. UTC
Quoting Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>:

> Joern Rennecke <amylaar@spamcop.net> writes:
>
>> Quoting Jack Howarth <howarth@bromo.med.uc.edu>:
>>
>>>   The previous fix for PR testsuite/42843 (r160461) incorrectly
>>> set PLUGINCC and PLUGINCFLAGS to the values for the host
>>> compiler rather than the built compiler.
>>
>> Actually, it's the build compiler vs. built (target) compiler.
>> And using the build compiler is right in the non-bootstrap case.
>> Even in the boostrap case, it only makes a difference if there
>> is an incompatibly between the build compiler and the stage2/
>> stage3 AKA target compiler.
>
> ... which is likely to be the case.  The problem is biggest if you
> bootstrap with some vendor compiler, but I've also observed the problem
> when bootstrapping on Solaris 8 and 9 with GCC 4.4: while 4.5 and
> mainline provide their own <stdint.h>, GCC 4.4 does not and the host OS
> lacks it, so all plugin tests fail.  Even if you add
> -I$(objdir)/gcc/include to fetch the mainline-provided file, this
> doesn't work because it references macros like __INT_LEAST8_TYPE__ that
> aren't defined by pre-4.5 GCCs.  One could instead use the stage1
> headers where configure was run with the build compiler, but that breaks
> down with make bootstrap-lean.
>
> Your change for PR testsuite/42843 introduced a regression from 4.5.

Indeed.  The previous code used @CC@, which is directly substituted by
autoconf, whereas my patch used $(COMPILER), which is based on $(CC) and
$(CXX), which, albeit set to @CC@ / @CXX@ in the Makefile, are overridden
by the toplevel make.

The attached patch restores the status quo ante of PLUGINCC / PLUGINCFLAGS
settings when not using --enable-build-with-cxx, while using
@CXX@ / @CXXFLAGS@ when --enable-build-with-cxx is in use.

Bootstrapped & regession tested with and without --enable-build-with-cxx
in revision 162120 on i5585-pc-linux-gnu.

The results with and without this patch are identical on my system,
except that I can observe in the generated site.exp that the previous
stage gcc is used instead of the system gcc / g++;
in comment #18 on PR42843, Jack Howarth confirmed that this patch
fixes the compiler selection issue that he sees on his system.

What this patch does not address is the failure of several tests to include
"diagnostic.h", which should probably done in a separate patch.
2010-07-14  Joern Rennecke  <joern.rennecke@embecosm.com>

	PR testsuite/42843
	* Makefile.in (PLUGINCC): Define in terms of @CC@ / @CXX@
	* Makefile.in (PLUGINCFLAGS): Define in terms of @CFLAGS@ / @CXXFLAGS@

Comments

Rainer Orth Aug. 26, 2010, 3:17 p.m. UTC | #1
Joern Rennecke <amylaar@spamcop.net> writes:

>> Your change for PR testsuite/42843 introduced a regression from 4.5.
>
> Indeed.  The previous code used @CC@, which is directly substituted by
> autoconf, whereas my patch used $(COMPILER), which is based on $(CC) and
> $(CXX), which, albeit set to @CC@ / @CXX@ in the Makefile, are overridden
> by the toplevel make.
>
> The attached patch restores the status quo ante of PLUGINCC / PLUGINCFLAGS
> settings when not using --enable-build-with-cxx, while using
> @CXX@ / @CXXFLAGS@ when --enable-build-with-cxx is in use.
>
> Bootstrapped & regession tested with and without --enable-build-with-cxx
> in revision 162120 on i5585-pc-linux-gnu.
>
> The results with and without this patch are identical on my system,
> except that I can observe in the generated site.exp that the previous
> stage gcc is used instead of the system gcc / g++;
> in comment #18 on PR42843, Jack Howarth confirmed that this patch
> fixes the compiler selection issue that he sees on his system.
>
> What this patch does not address is the failure of several tests to include
> "diagnostic.h", which should probably done in a separate patch.

Unfortunately, this patch has remained unreviewed for 6 weeks.  Given
that it fixes a regression from 4.5 by (apart from also dealing with
--enable-build-with-cxx) reverting an earlier patch of yours, I'd
suggest to install it as obvious.

Thanks.
	Rainer
Paolo Bonzini Aug. 26, 2010, 3:30 p.m. UTC | #2
On Thu, Aug 26, 2010 at 17:17, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
> Unfortunately, this patch has remained unreviewed for 6 weeks.  Given
> that it fixes a regression from 4.5 by (apart from also dealing with
> --enable-build-with-cxx) reverting an earlier patch of yours, I'd
> suggest to install it as obvious.

Reverts do not need explicit maintainer approval indeed.

Paolo
Rainer Orth Sept. 3, 2010, 4:03 p.m. UTC | #3
Paolo Bonzini <bonzini@gnu.org> writes:

> On Thu, Aug 26, 2010 at 17:17, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>> Unfortunately, this patch has remained unreviewed for 6 weeks.  Given
>> that it fixes a regression from 4.5 by (apart from also dealing with
>> --enable-build-with-cxx) reverting an earlier patch of yours, I'd
>> suggest to install it as obvious.
>
> Reverts do not need explicit maintainer approval indeed.

Given all this and the fact that the patch still hasn't been committed,
I've done that now.

	Rainer
diff mbox

Patch

Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 162120)
+++ gcc/Makefile.in	(working copy)
@@ -330,11 +330,14 @@  enable_lto = @enable_lto@
 LTO_BINARY_READER = @LTO_BINARY_READER@
 LTO_USE_LIBELF = @LTO_USE_LIBELF@
 
-# Compiler needed for plugin support
-PLUGINCC = $(COMPILER)
-
-# Flags needed for plugin support
-PLUGINCFLAGS = $(COMPILER_FLAGS)
+# Compiler and flags needed for plugin support
+ifneq ($(ENABLE_BUILD_WITH_CXX),yes)
+PLUGINCC = @CC@
+PLUGINCFLAGS = @CFLAGS@
+else
+PLUGINCC = @CXX@
+PLUGINCFLAGS = @CXXFLAGS@
+endif
 
 # Libs and linker options needed for plugin support
 PLUGINLIBS = @pluginlibs@