Your sim testsuite changes broke other targets

Your sim testsuite changes broke other targets

[Hans-Peter Nilsson]

All tests broke for all targets in my sim autotester.  For
*some* reason, they now want to run cr16-elf-as for the tests...

2008-02-12  M Ranga Swami Reddy <MR.Swami.Reddy@nsc.com>

	* sim/cr16/: New directory. Tests for CR16 simulator.
	sim/cr16/allinsn.exp and sim/cr16/misc.exp: New files: Test scripts.

At the top of that allinsn.exp:

------
# CR16 simulator testsuite.

set target_alias cr16-elf
set global_ld_options "-L/local/user/swami/cr16-5.1/build/CR_R_2007-08-24_11-34-56/release/linux/lib/gcc/cr16-elf/4.1.2 -T/home10/swami/dummy.ld"

if [istarget cr16*-*-*] {
...
------

I think I see why!  (Hint: they aren't cr16-elf. ;)

Did you really mean to commit with those "set"s there?  Please
remove them; they look wrong.  At least move them inside the
"if".  (If you remove them, I'll consider adding cr16 to the
targets I test in my autotester.)

brgds, H-P

Re: Your sim testsuite changes broke other targets

[M R Swami Reddy]

[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]

Hi,

Opps, sorry for this.

Fixed this issue. Please find the attached patch file and let me know if this ok 
to apply?

ChangeLog:
====
2008-04-08  M R Swami Reddy <MR.Swami.Reddy@nsc.com>

         * allinsn.exp: Moved target_alias and global_ld_options into
         if condition
====

Thanks,
Swami

Hans-Peter Nilsson wrote:
> All tests broke for all targets in my sim autotester.  For
> *some* reason, they now want to run cr16-elf-as for the tests...
> 
> 2008-02-12  M Ranga Swami Reddy <MR.Swami.Reddy@nsc.com>
> 
> 	* sim/cr16/: New directory. Tests for CR16 simulator.
> 	sim/cr16/allinsn.exp and sim/cr16/misc.exp: New files: Test scripts.
> 
> At the top of that allinsn.exp:
> 
> ------
> # CR16 simulator testsuite.
> 
> set target_alias cr16-elf
> set global_ld_options "-L/local/user/swami/cr16-5.1/build/CR_R_2007-08-24_11-34-56/release/linux/lib/gcc/cr16-elf/4.1.2 -T/home10/swami/dummy.ld"
> 
> if [istarget cr16*-*-*] {
> ...
> ------
> 
> I think I see why!  (Hint: they aren't cr16-elf. ;)
> 
> Did you really mean to commit with those "set"s there?  Please
> remove them; they look wrong.  At least move them inside the
> "if".  (If you remove them, I'll consider adding cr16 to the
> targets I test in my autotester.)
> 
> brgds, H-P
> 

[-- Attachment #2: allinsn.exp.patch --]
[-- Type: text/plain, Size: 896 bytes --]

Index: allinsn.exp
===================================================================
RCS file: /cvs/src/src/sim/testsuite/sim/cr16/allinsn.exp,v
retrieving revision 1.1
diff -c -r1.1 allinsn.exp
*** allinsn.exp	8 Apr 2008 09:03:17 -0000	1.1
--- allinsn.exp	8 Apr 2008 11:29:10 -0000
***************
*** 1,7 ****
  # CR16 simulator testsuite.
  
- set target_alias cr16-elf
- set global_ld_options "-L/local/user/swami/cr16-5.1/build/CR_R_2007-08-24_11-34-56/release/linux/lib/gcc/cr16-elf/4.1.2 -T/home10/swami/dummy.ld"
  
  if [istarget cr16*-*-*] {
      # load support procs
--- 1,5 ----
***************
*** 10,15 ****
--- 8,16 ----
      # all machines
      set all_machs "cr16"
  
+     set target_alias cr16-elf
+     set global_ld_options "-T$srcdir/$subdir/dummy.ld"
+ 
  
      # The .cgs suffix is for "cgen .s".
      foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.cgs]] {

Re: Your sim testsuite changes broke other targets

[Daniel Jacobowitz]

On Tue, Apr 08, 2008 at 05:07:15PM +0530, M R Swami Reddy wrote:
> +     set target_alias cr16-elf
> +     set global_ld_options "-T$srcdir/$subdir/dummy.ld"

You shouldn't need these at all.  Why are they necessary?  The -T
option can go in your board file if it has to go somewhere.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Your sim testsuite changes broke other targets

[M R Swami Reddy]

Hi,

Daniel Jacobowitz wrote:
> On Tue, Apr 08, 2008 at 05:07:15PM +0530, M R Swami Reddy wrote:
>> +     set target_alias cr16-elf
>> +     set global_ld_options "-T$srcdir/$subdir/dummy.ld"
> 
> You shouldn't need these at all.  Why are they necessary?  The -T
> option can go in your board file if it has to go somewhere.

Yes, I can set these option in board file also. Tested the sim test suite
by moving these 2 option from "allinsn.exp" to board file. Test suite is working 
as expected.

Thank you very much for your suggestion.

I will update the patch and submit the review.

Thanks,
Swami

Re: Your sim testsuite changes broke other targets

[M R Swami Reddy]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

Hi,

Please review the attached allinsn.exp.patch file.

ChangeLog:
==
2008-04-08  M R Swami Reddy <MR.Swami.Reddy@nsc.com>

         * allinsn.exp: Removed target_alias and global_ld_options.
====

Thanks,
Swami

Daniel Jacobowitz wrote:
> On Tue, Apr 08, 2008 at 05:07:15PM +0530, M R Swami Reddy wrote:
>> +     set target_alias cr16-elf
>> +     set global_ld_options "-T$srcdir/$subdir/dummy.ld"
> 
> You shouldn't need these at all.  Why are they necessary?  The -T
> option can go in your board file if it has to go somewhere.
> 

[-- Attachment #2: allinsn.exp.patch --]
[-- Type: text/plain, Size: 594 bytes --]

Index: allinsn.exp
===================================================================
RCS file: /cvs/src/src/sim/testsuite/sim/cr16/allinsn.exp,v
retrieving revision 1.1
diff -c -r1.1 allinsn.exp
*** allinsn.exp	8 Apr 2008 09:03:17 -0000	1.1
--- allinsn.exp	8 Apr 2008 12:20:32 -0000
***************
*** 1,7 ****
  # CR16 simulator testsuite.
  
- set target_alias cr16-elf
- set global_ld_options "-L/local/user/swami/cr16-5.1/build/CR_R_2007-08-24_11-34-56/release/linux/lib/gcc/cr16-elf/4.1.2 -T/home10/swami/dummy.ld"
  
  if [istarget cr16*-*-*] {
      # load support procs
--- 1,5 ----

Re: Your sim testsuite changes broke other targets

[Daniel Jacobowitz]

On Tue, Apr 08, 2008 at 05:57:20PM +0530, M R Swami Reddy wrote:
> Hi,
>
> Please review the attached allinsn.exp.patch file.
>
> ChangeLog:
> ==
> 2008-04-08  M R Swami Reddy <MR.Swami.Reddy@nsc.com>
>
>         * allinsn.exp: Removed target_alias and global_ld_options.
> ====

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Your sim testsuite changes broke other targets

[M R Swami Reddy]

Hi,

Daniel Jacobowitz wrote:
> On Tue, Apr 08, 2008 at 05:57:20PM +0530, M R Swami Reddy wrote:
>> Please review the attached allinsn.exp.patch file.
>>
>> ChangeLog:
>> ==
>> 2008-04-08  M R Swami Reddy <MR.Swami.Reddy@nsc.com>
>>
>>         * allinsn.exp: Removed target_alias and global_ld_options.
>> ====
> 
> OK.

Thank you very much for review and approval.

Applied the patch.

Thanks,
Swami