| Message ID | 20110304214837.06A58AE1C6@tobiano.tor.corp.google.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 4, 2011 at 13:48, Diego Novillo <dnovillo@google.com> wrote: > > When running tests in a toolchain configure --with-build-sysroot, we > need to execute the testsuite pointing every compiler invocation to > the build sysroot (which in our case is different than the sysroot > that the compiler is configured with). > > This patch adds a new global variable TEST_SYSROOT that takes its > value from SYSROOT_CFLAGS_FOR_TARGET. > > Mike, Rainer, is this approach OK? I had started by setting > TOOL_OPTIONS, but that confused libgloss.exp when it tried to compute > multilibs (get_multilibs). The presence of the argument > --sysroot=.... causes get_multilibs to call gcc with > --print-multilibs, which returns the string '.'. This causes massive > failures on all the tests that run with non-default multilibs. > > I'm applying this on google/integration. OK for 4.7? (the patch for > trunk would be slightly different, but the idea is the same). I forgot to add. Simon, this still shows up some failures in google/integration, but these failures are the same we have in trunk. For our builds, we have been XFAILing these failures, right? Do you want me to mark them XFAIL to avoid problems with the auto builds? Diego.
Hello Diego, * Diego Novillo wrote on Fri, Mar 04, 2011 at 10:48:36PM CET: > ChangeLog.google-integration > > * Makefile.in (GCC_FOR_TARGET): Remove use of > $(SYSROOT_CFLAGS_FOR_TARGET). > (site.exp): Add definition of TEST_SYSROOT. > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -371,8 +371,10 @@ UNWIND_H = $(srcdir)/unwind-generic.h > # The GCC to use for compiling crt*.o. > # Usually the one we just built. > # Don't use this as a dependency--use $(GCC_PASSES). > +GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) ./xgcc -B./ -B$(build_tooldir)/bin/ -isystem $(build_tooldir)/include -isystem $(build_tooldir)/sys-include -L$(objdir)/../ld > + > +# Set if the compiler was configured with --with-build-sysroot. > SYSROOT_CFLAGS_FOR_TARGET = @SYSROOT_CFLAGS_FOR_TARGET@ > -GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) $(shell $(PWD_COMMAND))/xgcc $(SYSROOT_CFLAGS_FOR_TARGET) -B$(shell $(PWD_COMMAND))/ -B$(build_tooldir)/bin/ -isystem $(build_tooldir)/include -isystem $(build_tooldir)/sys-include -L$(objdir)/../ld No comment on the rest, but shouldn't toplevel Makefile.tpl and configure machinery not also be updated to reflect this (e.g., so you can run 'make check-gcc' from the toplevel)? Thanks, Ralf
On Fri, Mar 4, 2011 at 14:01, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote: > No comment on the rest, but shouldn't toplevel Makefile.tpl and > configure machinery not also be updated to reflect this > (e.g., so you can run 'make check-gcc' from the toplevel)? Yeah, that's why I mentioned that the 4.7 patch would be slightly different. We already have those changes in google/integration (so they didn't show up in my patch). My question to you and Mike was regarding the approach of creating TEST_SYSROOT. If you think that's a good approach, I will prepare a patch vs 4.7. Thanks. Diego.
On Mar 4, 2011, at 1:48 PM, Diego Novillo wrote: > When running tests in a toolchain configure --with-build-sysroot, we > need to execute the testsuite pointing every compiler invocation to > the build sysroot (which in our case is different than the sysroot > that the compiler is configured with). When testing the built compiler, we test it on the host. The path name on the command line, is a path name on the build machine. You can't use that, in general on the host machine. So, for this to work, you'd need to work out a location on the host, and then remote_download that to the host, and then use that for the host compiler for testing. So, roughly, check out copy-file.exp or additional_files for example. In building, we use the build compiler to compile the target libraries, which means we can use that argument directly in that environment. I hate the cut-n-paste programming model (still). Though, you didn't invent it, so you don't have to pay the price to fix it, I'll just note it in passing. > # Usually the one we just built. > # Don't use this as a dependency--use $(GCC_PASSES). > +GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) ./xgcc -B./ -B$(build_tooldir)/bin/ -isystem $(build_tooldir)/include -isystem $(build_tooldir)/sys-include -L$(objdir)/../ld > + > +# Set if the compiler was configured with --with-build-sysroot. > SYSROOT_CFLAGS_FOR_TARGET = @SYSROOT_CFLAGS_FOR_TARGET@ > -GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) $(shell $(PWD_COMMAND))/xgcc $(SYSROOT_CFLAGS_FOR_TARGET) -B$(shell $(PWD_COMMAND))/ -B$(build_tooldir)/bin/ -isystem $(build_tooldir)/include -isystem $(build_tooldir)/sys-include -L$(objdir)/../ld $(shell $(PWD_COMMAND)) is slow (unless make short circuits it). Beyond that, this confuses the build != host setup I think. See, . is special and the testing infrastructure works hard to appear the compiler is ., not any other directory. You can't change the name for testing and have it just work. > - @echo "set GCC_UNDER_TEST \"$(GCC_FOR_TARGET)\"" >> ./tmp0 ? What's this? GCC_UNDER_TEST is kinda important to testing. I feel like I'm missing something trivial. > + @echo "set TEST_SYSROOT \"$(SYSROOT_CFLAGS_FOR_TARGET)\"" >> ./tmp0 > + if [info exists TEST_SYSROOT] { > + lappend options "additional_flags=$TEST_SYSROOT" > + } So, this will cause a certain ordering on the command line. If there is a testcase in the testsuite that tests that flag, this might cause the option under test to be overridden unless the TEST_SYSROOT option comes first on the command line. You can glance at the .log file, and ensure that this option comes before the -O2 say, of the multilib options in a torture suite.
On 4 March 2011 22:50, Diego Novillo <dnovillo@google.com> wrote: > ... > I forgot to add. Simon, this still shows up some failures in > google/integration, but these failures are the same we have in trunk. > > For our builds, we have been XFAILing these failures, right? Do you > want me to mark them XFAIL to avoid problems with the auto builds? No, it's okay to leave them failing for now. I'll go through and address each individually once the overall framework is settled. -- Google UK Limited | Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ | Registered in England Number: 3977902
On Fri, Mar 4, 2011 at 20:07, Mike Stump <mikestump@comcast.net> wrote: >> - @echo "set GCC_UNDER_TEST \"$(GCC_FOR_TARGET)\"" >> ./tmp0 > > ? What's this? GCC_UNDER_TEST is kinda important to testing. I feel like I'm missing something trivial. This is an artifact of the branch. We used to force GCC_UNDER_TEST, but we don't need to anymore. It will become clear when I send the patch against trunk. I'm testing it now. I will send it as soon as I finish testing all the variants. > >> + @echo "set TEST_SYSROOT \"$(SYSROOT_CFLAGS_FOR_TARGET)\"" >> ./tmp0 > >> + if [info exists TEST_SYSROOT] { >> + lappend options "additional_flags=$TEST_SYSROOT" >> + } > > So, this will cause a certain ordering on the command line. If there is a testcase in the > testsuite that tests that flag, this might cause the option under test to be overridden > unless the TEST_SYSROOT option comes first on the command line. You can glance > at the .log file, and ensure that this option comes before the -O2 say, of the multilib > options in a torture suite. Thanks. I see that I'm emitting it fairly late in the options. I will move it so this is added early in the command line. Diego.
On Mon, Mar 7, 2011 at 05:44, Simon Baldwin <simonb@google.com> wrote: > On 4 March 2011 22:50, Diego Novillo <dnovillo@google.com> wrote: >> ... >> I forgot to add. Simon, this still shows up some failures in >> google/integration, but these failures are the same we have in trunk. >> >> For our builds, we have been XFAILing these failures, right? Do you >> want me to mark them XFAIL to avoid problems with the auto builds? > > > No, it's okay to leave them failing for now. I'll go through and > address each individually once the overall framework is settled. OK, thanks. Diego.
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 2ff382e..dca5302 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -371,8 +371,10 @@ UNWIND_H = $(srcdir)/unwind-generic.h # The GCC to use for compiling crt*.o. # Usually the one we just built. # Don't use this as a dependency--use $(GCC_PASSES). +GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) ./xgcc -B./ -B$(build_tooldir)/bin/ -isystem $(build_tooldir)/include -isystem $(build_tooldir)/sys-include -L$(objdir)/../ld + +# Set if the compiler was configured with --with-build-sysroot. SYSROOT_CFLAGS_FOR_TARGET = @SYSROOT_CFLAGS_FOR_TARGET@ -GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) $(shell $(PWD_COMMAND))/xgcc $(SYSROOT_CFLAGS_FOR_TARGET) -B$(shell $(PWD_COMMAND))/ -B$(build_tooldir)/bin/ -isystem $(build_tooldir)/include -isystem $(build_tooldir)/sys-include -L$(objdir)/../ld # This is used instead of ALL_CFLAGS when compiling with GCC_FOR_TARGET. # It specifies -B./. @@ -4833,7 +4835,7 @@ site.exp: ./config.status Makefile @echo "set CXXFLAGS \"\"" >> ./tmp0 @echo "set HOSTCC \"$(CC)\"" >> ./tmp0 @echo "set HOSTCFLAGS \"$(CFLAGS)\"" >> ./tmp0 - @echo "set GCC_UNDER_TEST \"$(GCC_FOR_TARGET)\"" >> ./tmp0 + @echo "set TEST_SYSROOT \"$(SYSROOT_CFLAGS_FOR_TARGET)\"" >> ./tmp0 # When running the tests we set GCC_EXEC_PREFIX to the install tree so that # files that have already been installed there will be found. The -B option # overrides it, so use of GCC_EXEC_PREFIX will not result in using GCC files diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp index 9de817b..481edb5 100644 --- a/gcc/testsuite/lib/g++.exp +++ b/gcc/testsuite/lib/g++.exp @@ -282,6 +282,7 @@ proc g++_target_compile { source dest type options } { global gluefile wrap_flags global ALWAYS_CXXFLAGS global GXX_UNDER_TEST + global TEST_SYSROOT if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { lappend options "libs=${gluefile}" @@ -292,6 +293,9 @@ proc g++_target_compile { source dest type options } { lappend options "compiler=$GXX_UNDER_TEST" lappend options "timeout=[timeout_value]" + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } set options [concat $gpp_compile_options $options] set options [concat "$ALWAYS_CXXFLAGS" $options] diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp index ddbf9ef..208fb6e 100644 --- a/gcc/testsuite/lib/gcc.exp +++ b/gcc/testsuite/lib/gcc.exp @@ -126,6 +126,7 @@ proc gcc_target_compile { source dest type options } { global gluefile wrap_flags global GCC_UNDER_TEST global TOOL_OPTIONS + global TEST_SYSROOT if {[target_info needs_status_wrapper] != "" && \ [target_info needs_status_wrapper] != "0" && \ @@ -148,6 +149,9 @@ proc gcc_target_compile { source dest type options } { if [info exists TOOL_OPTIONS] { set options [concat "{additional_flags=$TOOL_OPTIONS}" $options] } + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } lappend options "timeout=[timeout_value]" lappend options "compiler=$GCC_UNDER_TEST" set options [dg-additional-files-options $options $source] diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp index 4a1a636..f4c8c2b 100644 --- a/gcc/testsuite/lib/gfortran.exp +++ b/gcc/testsuite/lib/gfortran.exp @@ -227,6 +227,7 @@ proc gfortran_target_compile { source dest type options } { global gluefile wrap_flags global ALWAYS_GFORTRANFLAGS global GFORTRAN_UNDER_TEST + global TEST_SYSROOT if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { lappend options "libs=${gluefile}" @@ -236,6 +237,9 @@ proc gfortran_target_compile { source dest type options } { lappend options "compiler=$GFORTRAN_UNDER_TEST" lappend options "timeout=[timeout_value]" + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } set options [concat "$ALWAYS_GFORTRANFLAGS" $options] set options [dg-additional-files-options $options $source] return [target_compile $source $dest $type $options] diff --git a/gcc/testsuite/lib/gnat.exp b/gcc/testsuite/lib/gnat.exp index ad33069..ea6a067 100644 --- a/gcc/testsuite/lib/gnat.exp +++ b/gcc/testsuite/lib/gnat.exp @@ -115,6 +115,7 @@ proc gnat_target_compile { source dest type options } { global GNAT_UNDER_TEST global TOOL_OPTIONS global gnat_target_current + global TEST_SYSROOT # dg-require-effective-target tests must be compiled as C. if [ string match "*.c" $source ] then { @@ -160,6 +161,10 @@ proc gnat_target_compile { source dest type options } { set options [concat "additional_flags=$TOOL_OPTIONS" $options] } + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } + return [target_compile $source $dest $type $options] } diff --git a/gcc/testsuite/lib/go.exp b/gcc/testsuite/lib/go.exp index ee5eb76..a67fe63 100644 --- a/gcc/testsuite/lib/go.exp +++ b/gcc/testsuite/lib/go.exp @@ -202,12 +202,17 @@ proc go_target_compile { source dest type options } { global gluefile wrap_flags global ALWAYS_GOCFLAGS global GOC_UNDER_TEST + global TEST_SYSROOT if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { lappend options "libs=${gluefile}" lappend options "ldflags=${wrap_flags}" } + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } + lappend options "compiler=$GOC_UNDER_TEST" set options [concat "$ALWAYS_GOCFLAGS" $options] diff --git a/gcc/testsuite/lib/obj-c++.exp b/gcc/testsuite/lib/obj-c++.exp index 57bab52..c2b5f85 100644 --- a/gcc/testsuite/lib/obj-c++.exp +++ b/gcc/testsuite/lib/obj-c++.exp @@ -291,6 +291,7 @@ proc obj-c++_target_compile { source dest type options } { global ALWAYS_OBJCXXFLAGS global OBJCXX_UNDER_TEST global shlib_ext + global TEST_SYSROOT set shlib_ext [get_shlib_extension] verbose "input-options: $options" 4 @@ -362,6 +363,9 @@ proc obj-c++_target_compile { source dest type options } { } } + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } lappend options "additional_flags=[libio_include_flags]" lappend options "compiler=$OBJCXX_UNDER_TEST"; lappend options "timeout=[timeout_value]" diff --git a/gcc/testsuite/lib/objc.exp b/gcc/testsuite/lib/objc.exp index ae0671f..ca6ea46 100644 --- a/gcc/testsuite/lib/objc.exp +++ b/gcc/testsuite/lib/objc.exp @@ -142,6 +142,7 @@ proc objc_target_compile { source dest type options } { global ld_library_path global objc_libgcc_s_path global shlib_ext + global TEST_SYSROOT set shlib_ext [get_shlib_extension] set ld_library_path ".:${objc_libgcc_s_path}" @@ -205,6 +206,10 @@ proc objc_target_compile { source dest type options } { set options [concat "{additional_flags=$TOOL_OPTIONS}" $options] } + if [info exists TEST_SYSROOT] { + lappend options "additional_flags=$TEST_SYSROOT" + } + # If we have built libobjc along with the compiler, point the test harness # at it (and associated headers).