RFA: V850 sim: various fixes.

RFA: V850 sim: various fixes.

[Nick Clifton]

Hi DJ,

  The patch below fixes a variety of small bugs in the V850 simulator's
  igen file.  It also adds a snippet of code to catch attempts to
  execute code via a NULL pointer.  With these changes applied I am
  seeing much better gcc testsuite results for the v850 toolchain,
  especially when checking the e2v3 multilib.

  OK to apply ?

Cheers
  Nick

sim/v850/ChangeLog
2012-09-10  Nick Clifton  <nickc@redhat.com>

	* v850.igen (W,WWWW): Correct computation of register number.
	(JR32): Remove unnecessary comma.
	(nop): Add trap when executing code at address 0.
	(cmovf.s): Register 0 is an invalid source register.
	(maddf.s): Remove bogus intermediary rounding.
	(nmaddf.s): Likewise.
	(trncf.sl): Remove bogus initial rounding.
	(trncf.dw): Likewise.
	(trncf.sl): Likewise.
	(trncf.sw): Likewise.

Index: sim/v850/v850.igen
===================================================================
RCS file: /cvs/src/src/sim/v850/v850.igen,v
retrieving revision 1.10
diff -u -3 -p -r1.10 v850.igen
--- sim/v850/v850.igen	29 Mar 2012 00:57:19 -0000	1.10
+++ sim/v850/v850.igen	10 Sep 2012 10:12:45 -0000
@@ -25,7 +25,7 @@
 :cache:::unsigned:reg1:RRRRR:(RRRRR)
 :cache:::unsigned:reg2:rrrrr:(rrrrr)
 :cache:::unsigned:reg3:wwwww:(wwwww)
-:cache:::unsigned:reg4:W,WWWW:((W << 4) + WWWW)
+:cache:::unsigned:reg4:W,WWWW:(W + (WWWW << 1))
 
 :cache:::unsigned:reg1e:RRRR:(RRRR << 1)
 :cache:::unsigned:reg2e:rrrr:(rrrr << 1)
@@ -773,7 +773,7 @@ rrrrr!0,11110,dddddd + ddddddddddddddd,0
 
 
 // JR32
-00000010111,00000 + iiiiiiiiiiiiiiii + IIIIIIIIIIIIIIII:VI:::jr32
+0000001011100000 + iiiiiiiiiiiiiiii + IIIIIIIIIIIIIIII:VI:::jr32
 *v850e2
 *v850e2v3
 "jr <imm32>"
@@ -1280,7 +1280,10 @@ rrrrr,111111,iiiii + wwwww,01001,IIII,10
 0000000000000000:I:::nop
 "nop"
 {
-  /* do nothing, trace nothing */
+  /* Catch indirection via a NULL pointer.
+     Memory from 0 - 0x10000 is filled with NOP instructions...  */
+  if (cia == 0)
+    sim_engine_halt (SD, CPU, NULL, cia, sim_stopped, SIM_SIGTRAP);
 }
 
 
@@ -2345,7 +2348,7 @@ rrrr,0111111,RRRR,0 + wwww!0,01000001,bb
 }
 
 // CMOVF.S
-rrrrr,111111,RRRRR + wwwww!0,1000000,bbb,0:F_I:::cmovf_s
+rrrrr,111111,RRRRR!0 + wwwww!0,1000000,bbb,0:F_I:::cmovf_s
 *v850e2v3
 "cmovf.d <bbb>, r<reg1>, r<reg2>, r<reg3>"
 {
@@ -2676,7 +2679,6 @@ rrrrr,111111,RRRRR + wwwww,101,W,00,WWWW
   TRACE_FP_INPUT_FPU3 (&wop1, &wop2, &wop3);
 
   status = sim_fpu_mul (&ans, &wop1, &wop2);
-  status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
   wop1 = ans;
   status |= sim_fpu_add (&ans, &wop1, &wop3);
   status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
@@ -2954,7 +2956,6 @@ rrrrr,111111,RRRRR + wwwww,101,W,10,WWWW
   TRACE_FP_INPUT_FPU3 (&wop1, &wop2, &wop3);
 
   status = sim_fpu_mul (&ans, &wop1, &wop2);
-  status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
   wop1 = ans;
   status |= sim_fpu_add (&ans, &wop1, &wop3);
   status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
@@ -3190,8 +3191,7 @@ rrrr,011111100001 + wwww,010001010100:F_
   sim_fpu_232to (&wop, GR[reg2e+1], GR[reg2e]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_64 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
 
   check_cvt_fi(sd, status, 1);
 
@@ -3212,8 +3212,7 @@ rrrr,011111100001 + wwwww,10001010000:F_
   sim_fpu_232to (&wop, GR[reg2e+1], GR[reg2e]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_32 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
 
   check_cvt_fi(sd, status, 1);
 
@@ -3233,8 +3232,7 @@ rrrrr,11111100001 + wwww,010001000100:F_
   sim_fpu_32to (&wop, GR[reg2]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_64 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
 
   GR[reg3e] = ans;
   GR[reg3e+1] = ans >> 32L;
@@ -3253,12 +3251,10 @@ rrrrr,11111100001 + wwwww,10001000000:F_
   sim_fpu_32to (&wop, GR[reg2]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_32 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
 
   check_cvt_fi(sd, status, 0);
 
   GR[reg3] = ans;
   TRACE_FP_RESULT_WORD1 (ans);
 }
-

Re: RFA: V850 sim: various fixes.

[Mike Frysinger]

On Mon, Sep 10, 2012 at 3:15 AM, Nick Clifton wrote:
>   The patch below fixes a variety of small bugs in the V850 simulator's
>   igen file.  It also adds a snippet of code to catch attempts to
>   execute code via a NULL pointer.

usually the simulator represents the hardware.  so if the hardware has
memory at address 0 and someone were to load up an image with code
there, there's nothing invalid when it comes to jumping there.

what i do on the Blackfin side when it comes to testing is to have the
ELF manually pad the start with insns that'll trigger an exception
(just like it would on the hw).  then i set the entry point beyond
that.
-mike

Re: RFA: V850 sim: various fixes.

[nick clifton]

Hi Mike,

>>    It also adds a snippet of code to catch attempts to
>>    execute code via a NULL pointer.
>
> usually the simulator represents the hardware.  so if the hardware has
> memory at address 0 and someone were to load up an image with code
> there, there's nothing invalid when it comes to jumping there.

Personally I see the simulator as more of a testing and debugging tool. 
  It does not try to be hardware accurate - it does not model the cache 
for example - so adding a feature that helps catch program errors, even 
if it not strictly modelling the hardware, seems OK to me.

> what i do on the Blackfin side when it comes to testing is to have the
> ELF manually pad the start with insns that'll trigger an exception
> (just like it would on the hw).  then i set the entry point beyond
> that.

Another fine approach.  I'll see what DJ thinks, but I will be happy
to remove the NULL trapping part of the patch if he does not like it.

Cheers
   Nick

Re: RFA: V850 sim: various fixes.

[Mike Frysinger]

On Mon, Sep 10, 2012 at 8:34 AM, nick clifton wrote:
>>>    It also adds a snippet of code to catch attempts to
>>>    execute code via a NULL pointer.
>>
>> usually the simulator represents the hardware.  so if the hardware has
>> memory at address 0 and someone were to load up an image with code
>> there, there's nothing invalid when it comes to jumping there.
>
> Personally I see the simulator as more of a testing and debugging tool.  It
> does not try to be hardware accurate - it does not model the cache for
> example - so adding a feature that helps catch program errors, even if it
> not strictly modelling the hardware, seems OK to me.

well, you're talking about cycle accurate simulators vs functional
accurate simulators.  yes, many sims don't model down to the cache
level (although that functionality is available), nor things like the
pipeline, but at a functional level, those don't matter.  being able
to read/write/execute address 0 is a functional aspect.

i'm not against adding a flag to control this, but in general i don't
think completely blocking a functional aspect is correct ... although
DJ is the v850 guy, and i certainly won't force this policy on every
sim since not everyone uses the sim as a generically as i do.
-mike

Re: RFA: V850 sim: various fixes.

[DJ Delorie]

I've added NULL checks in simulators even when the hardware has RAM at
address zero.  Typically, a command line option and/or gdb command is
required to enable it, since it usually just gets in the way more
often than it helps.  I wouldn't want it to be unconditional.

Re: RFA: V850 sim: various fixes.

[nick clifton]

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

Hi DJ.

> I've added NULL checks in simulators even when the hardware has RAM at
> address zero.  Typically, a command line option and/or gdb command is
> required to enable it, since it usually just gets in the way more
> often than it helps.  I wouldn't want it to be unconditional.

OK - here is a revised v850.igen patch without the trap for executing 
via a NULL pointer.  I'll look at adding that feature back in, 
controlled via a command line option, at some later date.

OK to apply ?

Cheers
   Nick

sim/v850/ChangeLog

2012-09-10  Nick Clifton  <nickc@redhat.com>

	* v850.igen (W,WWWW): Correct computation of register number.
	(JR32): Remove unnecessary comma.
	(cmovf.s): Register 0 is an invalid source register.
	(maddf.s): Remove bogus intermediary rounding.
	(nmaddf.s): Likewise.
	(trncf.sl): Remove bogus initial rounding.
	(trncf.dw): Likewise.
	(trncf.sl): Likewise.
	(trncf.sw): Likewise.



[-- Attachment #2: v850.igen.patch --]
[-- Type: text/x-patch, Size: 3473 bytes --]

Index: sim/v850/v850.igen
===================================================================
RCS file: /cvs/src/src/sim/v850/v850.igen,v
retrieving revision 1.10
diff -u -3 -p -r1.10 v850.igen
--- sim/v850/v850.igen	29 Mar 2012 00:57:19 -0000	1.10
+++ sim/v850/v850.igen	10 Sep 2012 16:31:01 -0000
@@ -25,7 +25,7 @@
 :cache:::unsigned:reg1:RRRRR:(RRRRR)
 :cache:::unsigned:reg2:rrrrr:(rrrrr)
 :cache:::unsigned:reg3:wwwww:(wwwww)
-:cache:::unsigned:reg4:W,WWWW:((W << 4) + WWWW)
+:cache:::unsigned:reg4:W,WWWW:(W + (WWWW << 1))
 
 :cache:::unsigned:reg1e:RRRR:(RRRR << 1)
 :cache:::unsigned:reg2e:rrrr:(rrrr << 1)
@@ -773,7 +773,7 @@ rrrrr!0,11110,dddddd + ddddddddddddddd,0
 
 
 // JR32
-00000010111,00000 + iiiiiiiiiiiiiiii + IIIIIIIIIIIIIIII:VI:::jr32
+0000001011100000 + iiiiiiiiiiiiiiii + IIIIIIIIIIIIIIII:VI:::jr32
 *v850e2
 *v850e2v3
 "jr <imm32>"
@@ -2345,7 +2345,7 @@ rrrr,0111111,RRRR,0 + wwww!0,01000001,bb
 }
 
 // CMOVF.S
-rrrrr,111111,RRRRR + wwwww!0,1000000,bbb,0:F_I:::cmovf_s
+rrrrr,111111,RRRRR!0 + wwwww!0,1000000,bbb,0:F_I:::cmovf_s
 *v850e2v3
 "cmovf.d <bbb>, r<reg1>, r<reg2>, r<reg3>"
 {
@@ -2676,7 +2676,6 @@ rrrrr,111111,RRRRR + wwwww,101,W,00,WWWW
   TRACE_FP_INPUT_FPU3 (&wop1, &wop2, &wop3);
 
   status = sim_fpu_mul (&ans, &wop1, &wop2);
-  status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
   wop1 = ans;
   status |= sim_fpu_add (&ans, &wop1, &wop3);
   status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
@@ -2954,7 +2953,6 @@ rrrrr,111111,RRRRR + wwwww,101,W,10,WWWW
   TRACE_FP_INPUT_FPU3 (&wop1, &wop2, &wop3);
 
   status = sim_fpu_mul (&ans, &wop1, &wop2);
-  status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
   wop1 = ans;
   status |= sim_fpu_add (&ans, &wop1, &wop3);
   status |= sim_fpu_round_64 (&ans, FPSR_GET_ROUND(), sim_fpu_denorm_underflow_inexact);
@@ -3190,8 +3188,7 @@ rrrr,011111100001 + wwww,010001010100:F_
   sim_fpu_232to (&wop, GR[reg2e+1], GR[reg2e]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_64 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
 
   check_cvt_fi(sd, status, 1);
 
@@ -3212,8 +3209,7 @@ rrrr,011111100001 + wwwww,10001010000:F_
   sim_fpu_232to (&wop, GR[reg2e+1], GR[reg2e]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_32 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
 
   check_cvt_fi(sd, status, 1);
 
@@ -3233,8 +3229,7 @@ rrrrr,11111100001 + wwww,010001000100:F_
   sim_fpu_32to (&wop, GR[reg2]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_64 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to64i (&ans, &wop, sim_fpu_round_zero);
 
   GR[reg3e] = ans;
   GR[reg3e+1] = ans >> 32L;
@@ -3253,12 +3248,10 @@ rrrrr,11111100001 + wwwww,10001000000:F_
   sim_fpu_32to (&wop, GR[reg2]);
   TRACE_FP_INPUT_FPU1 (&wop);
 
-  status = sim_fpu_round_32 (&wop, sim_fpu_round_zero, sim_fpu_denorm_zero);
-  status |= sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
+  status = sim_fpu_to32i (&ans, &wop, sim_fpu_round_zero);
 
   check_cvt_fi(sd, status, 0);
 
   GR[reg3] = ans;
   TRACE_FP_RESULT_WORD1 (ans);
 }
-

Re: RFA: V850 sim: various fixes.

[DJ Delorie]

This is OK.

> 	* v850.igen (W,WWWW): Correct computation of register number.
> 	(JR32): Remove unnecessary comma.
> 	(cmovf.s): Register 0 is an invalid source register.
> 	(maddf.s): Remove bogus intermediary rounding.
> 	(nmaddf.s): Likewise.
> 	(trncf.sl): Remove bogus initial rounding.
> 	(trncf.dw): Likewise.
> 	(trncf.sl): Likewise.
> 	(trncf.sw): Likewise.