From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12053 invoked by alias); 3 Aug 2012 20:56:10 -0000 Received: (qmail 12043 invoked by uid 22791); 3 Aug 2012 20:56:09 -0000 X-SWARE-Spam-Status: No, hits=-4.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 03 Aug 2012 20:55:42 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SxOuL-0004BF-0h from Maciej_Rozycki@mentor.com ; Fri, 03 Aug 2012 13:55:41 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Fri, 3 Aug 2012 13:55:40 -0700 Received: from [172.30.4.177] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Fri, 3 Aug 2012 21:55:38 +0100 Date: Fri, 03 Aug 2012 21:46:00 -0000 From: "Maciej W. Rozycki" To: Richard Sandiford CC: Alan Modra , Subject: Re: [PATCH] MIPS: Enable NewABI tests for SDE targets In-Reply-To: <87ipd1nnpb.fsf@talisman.home> Message-ID: References: <87ipd1nnpb.fsf@talisman.home> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2012-08/txt/msg00073.txt.bz2 On Thu, 2 Aug 2012, Richard Sandiford wrote: > > Index: binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp > > =================================================================== > > --- binutils-fsf-trunk-quilt.orig/ld/testsuite/lib/ld-lib.exp 2012-07-24 15:29:41.000000000 +0100 > > +++ binutils-fsf-trunk-quilt/ld/testsuite/lib/ld-lib.exp 2012-07-26 03:08:32.951798913 +0100 > > @@ -431,7 +431,7 @@ proc ld_simple_link_defsyms {} { > > return $flags > > } > > > > -# run_dump_test FILE > > +# run_dump_test FILE (optional:) EXTRA_OPTIONS > > # Copied from gas testsuite, tweaked and further extended. > > # > > # Assemble a .s file, then run some utility on it and check the output. > > @@ -456,6 +456,12 @@ proc ld_simple_link_defsyms {} { > > # list ends with the first line that doesn't match the above syntax > > # (hmm, not great for error detection). > > # > > +# The optional EXTRA_OPTIONS argument to `run_dump_test' is a list of > > +# two-element lists. The first element of each is an option name, and > > +# the second additional arguments to be added on to the end of the > > +# option list as given in FILE.d. (If omitted, no additional options > > +# are added.) > > +# > > # The interesting options are: > > # > > # name: TEST-NAME > > @@ -503,6 +509,11 @@ proc ld_simple_link_defsyms {} { > > # More than one "source" directive can be given, which is useful > > # when testing linking. > > # > > +# dump: DUMP > > +# Match against DUMP.d. If omitted, this defaults to FILE.d. This > > +# is useful if several .d files differ by options only. Options are > > +# always read from FILE.d. > > +# > > # xfail: TARGET > > # The test is expected to fail on TARGET. This may occur more than > > # once. > > @@ -534,7 +545,7 @@ proc ld_simple_link_defsyms {} { > > # regexps in FILE.d. `regexp_diff' is defined in binutils-common.exp; > > # see further comments there. > > # > > -proc run_dump_test { name } { > > +proc run_dump_test { name {extra_options {}} } { > > global subdir srcdir > > global OBJDUMP NM AS OBJCOPY READELF LD > > global OBJDUMPFLAGS NMFLAGS ASFLAGS OBJCOPYFLAGS READELFFLAGS LDFLAGS > > I think extra_options ought to be a one-level list, i.e. > {key1 value key2 value2 ...}. That makes the calls simpler > (because there's one fewer [list ...]) and: > > > + foreach i $extra_options { > > + set opt_name [lindex $i 0] > > + set opt_val [lindex $i 1] > > + if ![info exists opts($opt_name)] { > > + perror "unknown option $opt_name given in extra_opts" > > + unresolved $subdir/$name > > + return > > + } > > + # Add extra option to end of existing option, adding space > > + # if necessary. > > + if { ![regexp "warning|error" $opt_name] > > + && [string length $opts($opt_name)] } { > > + append opts($opt_name) " " > > + } > > + append opts($opt_name) $opt_val > > + } > > reduces to: > > foreach { opt_name opt_val } $extra_options { > if ![info exists opts($opt_name)] { > perror "unknown option $opt_name given in extra_opts" > unresolved $subdir/$name > return > } > # Add extra option to end of existing option, adding space > # if necessary. > if { ![regexp "warning|error" $opt_name] > && [string length $opts($opt_name)] } { > append opts($opt_name) " " > } > append opts($opt_name) $opt_val > } > > OK with that change, thanks. OK for the follow-up too. A list of lists seems more in the spirit of TCL to me, while a vector is more C-like, but I have no strong preference either way. However please note that implementing your suggestion will make the two run_dump_test implementations confusingly different -- do you still want me to proceed? If so, wouldn't it make sense to update the GAS version and any callers accordingly so that the two versions don't diverge? I wonder if the two couldn't simply be built around a common core moved over to binutils/testsuite/lib/binutils-common.exp instead. There's not much difference really and TCL has ways do such stuff more easily than for example C. Also some calls to "subst" over key values, specifically "as" and "ld" (set opt_val [subst $opt_val]), look mysterious to me. While they might be useful for options to have parts referring to TCL variables, this however is not going to be particularly useful given that they are executed in the context of run_dump_test. It looks to me it would make more sense if they were executed in the caller's context with "uplevel". Do you or anybody know what the original purpose of this substitution was? Maciej