diff mbox

libstdc++ testsuite cxxflags

Message ID 528E8FB3.5090809@codesourcery.com
State New
Headers show

Commit Message

Cesar Philippidis Nov. 21, 2013, 10:56 p.m. UTC 
On 11/21/13, 5:42 AM, Jonathan Wakely wrote:
> On 20 November 2013 23:57, Cesar Philippidis wrote:
>> On 11/20/13, 1:46 PM, Jonathan Wakely wrote:
>>> On 20 November 2013 21:44, Jonathan Wakely wrote:
>>>> On 29 October 2013 15:37, Cesar Philippidis wrote:
>>>>> This patch addresses two issues with the libstdc++ testsuite:
>>>>>
>>>>>   * duplicate "-g -O2" CXXFLAGS
>>>>>   * missing "-g -O2" for remote targets
>>>>>
>>>>> The duplicate "-g -O2" flags is a result of testsuite_flags.in using
>>>>> build-time CXXFLAGS and proc libstdc++_init using the environmental
>>>>> CXXFLAGS, which defaults to its build-time value. This patch prevents
>>>>> testsuite_flags.in from using build-time CXXFLAGS.
>>>>
>>>>> Certain remote targets require a minimum optimization level -O1 in order
>>>>> to pass several atomics built-in function tests. This patch ensures
>>>>> cxxflags contains "-g -O2" at minimum when no other optimization flags
>>>>> are specified. The testsuite used to set those flags prior to Benjamin's
>>>>> patch to remove duplicate cxxflags here
>>>>> <http://gcc.gnu.org/ml/gcc-patches/2012-03/msg01572.html>.
>>>>>
>>>>> Is this OK for trunk? If so, please apply (I don't have commit rights).
>>>>
>>>> I think so ... although I'm not sure I've got my head round the
>>>> effects in all cases!
>>>
>>> Sorry, I didn't realise gmail thought Ctrl-Enter meant send. I meant
>>> to ask a couple of questions about it ...
>>>
>>> Is removing EXTRA_CXX_FLAGS necessary too?
>>
>> I looked at it again, and it seems to be OK to leave it in there.
>>
>>> For remote targets, if CXXFLAGS is set in the env can -g still end up missing?
>>
>> No, but CXXFLAGS isn't necessarily set in the env. Specifically, if you
>> run the testsuite without using the makefile, the CXXFLAGS may not be set.
>>
>> I revised the patch to preserve @EXTRA_CXX_FLAGS@. I also append the
>> '-g' flag with '-O2', since the '-g' isn't as important in the testsuite
>> as '-O2'.
>>
>> Is this patch OK? Is so, please commit it because I do not have an svn
>> account.
> 
> I've been playing around with this patch and CXXFLAGS further, and I'm
> not sure about it now.
> 
> What harm do the duplicate flags do? If you want different flags to be
> used when running the testsuite you can set CXXFLAGS, which will come
> later on the command-line and so take precedence. However, if we
> remove "-g -O2" from CXXFLAGS_config and you use CXXFLAGS=-DFOO when
> running the testsuite then after this change you won't get the same
> result, you'd have to change to use CXXFLAGS="-g -O2 -DFOO"
> 
> Is that really what we want?

I see your point. Well, if you want to override CXXFLAGS during testing,
it's probably better to use different environmental variable altogether
and include '-g -O2' as part of the base CXXFLAGS. The attached patch
does that with LIBSTDCXX_CXXFLAGS.

That said, I don't have a strong opinion on the matter, so if you want
to use the libstdcxx_testsuite-b.diff patch without the Makefile.in
changes, that's fine with me.

Cesar
2013-11-21  Cesar Philippidis  <cesar@codesourcery.com>

	libstdc++-v3/
	* scripts/testsuite_flags.in (cxxflags): Remove @CXXFLAGS@ since 
	libstdc++.exp imports those flags via getenv.
	* testsuite/lib/libstdc++.exp (libstdc++_init): Ensure that 
	cxxflags contains '-g -O2' flag. Also, use env LIBSTDCXX_CXXFLAGS 
	to augment cxxflags instead of env CXXFLAGS.
diff mbox

Patch

diff --git a/libstdc++-v3/scripts/testsuite_flags.in b/libstdc++-v3/scripts/testsuite_flags.in
index cf692f8..5e7ad32 100755
--- a/libstdc++-v3/scripts/testsuite_flags.in
+++ b/libstdc++-v3/scripts/testsuite_flags.in
@@ -57,7 +57,7 @@  case ${query} in
       ;;
     --cxxflags)
       CXXFLAGS_default="-D_GLIBCXX_ASSERT -fmessage-length=0"
-      CXXFLAGS_config="@SECTION_FLAGS@ @CXXFLAGS@ @EXTRA_CXX_FLAGS@"
+      CXXFLAGS_config="@SECTION_FLAGS@ @EXTRA_CXX_FLAGS@"
       echo ${CXXFLAGS_default} ${CXXFLAGS_config} 
       ;;
     --cxxvtvflags)
diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 0dff98c..2848ca7 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -278,8 +278,8 @@  proc libstdc++_init { testfile } {
 	set cc [exec sh $flags_file --build-cc]
 	set includes [exec sh $flags_file --build-includes]
     }
-    append cxxflags " "
-    append cxxflags [getenv CXXFLAGS]
+    append cxxflags " -g -O2 "
+    append cxxflags [getenv LIBSTDCXX_CXXFLAGS]
     v3track cxxflags 2
 
     # Always use MO files built by this test harness.