Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] sh-sim loose ends
@ 2004-02-12 21:56 Michael Snyder
  2004-02-12 22:39 ` Joern Rennecke
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2004-02-12 21:56 UTC (permalink / raw)
  To: Joern Rennecke, joern.rennecke, gdb-patches

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

Joern, a few loose ends.  Some of 'em I could justify calling obvious,
but I'd like your review of the 'L' and 'SET_NIP' changes.


[-- Attachment #2: foo --]
[-- Type: text/plain, Size: 6380 bytes --]

2004-02-12  Michael Snyder  <msnyder@redhat.com>

	* interp.c (saved_state, fail, raise_exception, 
	raise_buserror, control_c, fsca_s, fsrra_s: Make static.
	* gencode.c (movt): Modifies R[n]; call 'L' macro.
	(trapa): Factor out duplicate variable 'imm' (same as 'i').
	(sleep, trapa, ppi): Use SET_NIP to modify nip.
	(trap, gensim_caselist): Whitespace.

Index: interp.c
===================================================================
RCS file: /cvs/src/src/sim/sh/interp.c,v
retrieving revision 1.15
diff -p -r1.15 interp.c
*** interp.c	12 Feb 2004 19:32:12 -0000	1.15
--- interp.c	12 Feb 2004 20:34:39 -0000
*************** typedef union
*** 157,163 ****
    int asints[40];
  } saved_state_type;
  
! saved_state_type saved_state;
  
  struct loop_bounds { unsigned char *start, *end; };
  
--- 157,163 ----
    int asints[40];
  } saved_state_type;
  
! static saved_state_type saved_state;
  
  struct loop_bounds { unsigned char *start, *end; };
  
*************** do { \
*** 314,320 ****
  
  #define DSR  (saved_state.asregs.sregs.named.fpscr)
  
! int 
  fail ()
  {
    abort ();
--- 314,320 ----
  
  #define DSR  (saved_state.asregs.sregs.named.fpscr)
  
! static int 
  fail ()
  {
    abort ();
*************** fail ()
*** 326,339 ****
  /* This function exists mainly for the purpose of setting a breakpoint to
     catch simulated bus errors when running the simulator under GDB.  */
  
! void
  raise_exception (x)
       int x;
  {
    RAISE_EXCEPTION (x);
  }
  
! void
  raise_buserror ()
  {
    raise_exception (SIGBUS);
--- 326,339 ----
  /* This function exists mainly for the purpose of setting a breakpoint to
     catch simulated bus errors when running the simulator under GDB.  */
  
! static void
  raise_exception (x)
       int x;
  {
    RAISE_EXCEPTION (x);
  }
  
! static void
  raise_buserror ()
  {
    raise_exception (SIGBUS);
*************** trap (i, regs, insn_ptr, memory, maskl, 
*** 979,985 ****
      case 2:
        raise_exception (SIGQUIT);
        break;
!     case 3:			/* FIXME: for backwards compat, should be removed */
      case 33:
        {
  	unsigned int countp = * (unsigned int *) (insn_ptr + 4);
--- 979,985 ----
      case 2:
        raise_exception (SIGQUIT);
        break;
!     case 3:		/* FIXME: for backwards compat, should be removed */
      case 33:
        {
  	unsigned int countp = * (unsigned int *) (insn_ptr + 4);
*************** trap (i, regs, insn_ptr, memory, maskl, 
*** 1191,1197 ****
    return 0;
  }
  
! void
  control_c (sig, code, scp, addr)
       int sig;
       int code;
--- 1191,1197 ----
    return 0;
  }
  
! static void
  control_c (sig, code, scp, addr)
       int sig;
       int code;
*************** get_loop_bounds_ext (rs, re, memory, mem
*** 1445,1451 ****
    return loop;
  }
  
! float
  fsca_s (int in, double (*f) (double))
  {
    double rad = ldexp ((in & 0xffff), -15) * 3.141592653589793238462643383;
--- 1445,1451 ----
    return loop;
  }
  
! static float
  fsca_s (int in, double (*f) (double))
  {
    double rad = ldexp ((in & 0xffff), -15) * 3.141592653589793238462643383;
*************** fsca_s (int in, double (*f) (double))
*** 1467,1473 ****
    return abs (upper - result) >= abs (lower - result) ? upper : lower;
  }
  
! float
  fsrra_s (float in)
  {
    double result = 1. / sqrt (in);
--- 1467,1473 ----
    return abs (upper - result) >= abs (lower - result) ? upper : lower;
  }
  
! static float
  fsrra_s (float in)
  {
    double result = 1. / sqrt (in);
Index: gencode.c
===================================================================
RCS file: /cvs/src/src/sim/sh/gencode.c,v
retrieving revision 1.27
diff -p -r1.27 gencode.c
*** gencode.c	12 Feb 2004 19:32:12 -0000	1.27
--- gencode.c	12 Feb 2004 20:34:39 -0000
*************** op tab[] =
*** 862,867 ****
--- 862,868 ----
  
    { "n", "", "movt <REG_N>", "0000nnnn00101001",
      "R[n] = T;",
+     "L (n);"
    },
  
    { "0", "n", "movua.l @<REG_N>,R0", "0100nnnn10101001",
*************** op tab[] =
*** 1102,1108 ****
    },
  
    { "", "", "sleep", "0000000000011011",
!     "nip += trap (0xc3, &R0, PC, memory, maskl, maskw, endianw);",
    },
  
    { "n", "", "stc <CREG_M>,<REG_N>", "0000nnnnmmmm0010",
--- 1103,1109 ----
    },
  
    { "", "", "sleep", "0000000000011011",
!     "SET_NIP (nip + trap (0xc3, &R0, PC, memory, maskl, maskw, endianw));",
    },
  
    { "n", "", "stc <CREG_M>,<REG_N>", "0000nnnnmmmm0010",
*************** op tab[] =
*** 1191,1199 ****
    },
  
    { "0", "", "trapa #<imm>", "11000011i8*1....", 
-     "long imm = 0xff & i;",
      "if (i < 20 || i == 33 || i == 34 || i == 0xc3)",
!     "  nip += trap (i, &R0, PC, memory, maskl, maskw, endianw);",
  #if 0
      "else {",
      /* SH-[12] */
--- 1192,1199 ----
    },
  
    { "0", "", "trapa #<imm>", "11000011i8*1....", 
      "if (i < 20 || i == 33 || i == 34 || i == 0xc3)",
!     "  SET_NIP (nip + trap (i, &R0, PC, memory, maskl, maskw, endianw));",
  #if 0
      "else {",
      /* SH-[12] */
*************** op tab[] =
*** 1208,1214 ****
      "  SET_SR (GET_SR () | SR_MASK_MD | SR_MASK_BL | SR_MASK_RB);",
      "  /* FIXME: EXPEVT = 0x00000160; */",
  #endif
!     "  SET_NIP (PT2H (RLAT (VBR + (imm<<2))));",
      "}",
    },
  
--- 1208,1214 ----
      "  SET_SR (GET_SR () | SR_MASK_MD | SR_MASK_BL | SR_MASK_RB);",
      "  /* FIXME: EXPEVT = 0x00000160; */",
  #endif
!     "  SET_NIP (PT2H (RLAT (VBR + ((long) (i << 2)))));",
      "}",
    },
  
*************** op movsxy_tab[] =
*** 1516,1522 ****
    },
    { "", "", "ppi", "1111100000000000",
      "ppi_insn (RIAT (nip));",
!     "nip += 2;",
      "iword &= 0xf7ff; goto top;",
    },
  #endif
--- 1516,1522 ----
    },
    { "", "", "ppi", "1111100000000000",
      "ppi_insn (RIAT (nip));",
!     "SET_NIP (nip + 2);",
      "iword &= 0xf7ff; goto top;",
    },
  #endif
*************** gensim_caselist (p)
*** 2669,2677 ****
  	char *r;
  	for (r = p->defs; *r; r++) 
  	  {
! 	    if (*r == '0') printf("      CDEF (0);\n"); 
! 	    if (*r == 'n') printf("      CDEF (n);\n"); 
! 	    if (*r == 'm') printf("      CDEF (m);\n"); 
  	  }
        }
  
--- 2669,2677 ----
  	char *r;
  	for (r = p->defs; *r; r++) 
  	  {
! 	    if (*r == '0') printf ("      CDEF (0);\n"); 
! 	    if (*r == 'n') printf ("      CDEF (n);\n"); 
! 	    if (*r == 'm') printf ("      CDEF (m);\n"); 
  	  }
        }
  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] sh-sim loose ends
  2004-02-12 21:56 [RFA] sh-sim loose ends Michael Snyder
@ 2004-02-12 22:39 ` Joern Rennecke
  2004-02-12 23:52   ` Michael Snyder
  0 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2004-02-12 22:39 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Joern Rennecke, joern.rennecke, gdb-patches

> 	* gencode.c (movt): Modifies R[n]; call 'L' macro.

That doesn't make sense, 'L' simulates the data read memory latency of an
SH[123].  movt doesn't incur such a latency.

> 	(trapa): Factor out duplicate variable 'imm' (same as 'i').

OK.  But I don't see why you need the cast to long.

> 	(sleep, trapa, ppi): Use SET_NIP to modify nip.

I don't see any need for this.  RAISE_EXCEPTION already clears
saved_state.asregs.insn_end , so that takes care of the
exceptions that might arise during sleep.
For trapa, that leaves just the possibility that we fail to miss a
loop bound that is set to somewhere inside a profiler trap; I think
you deserve whatever you get when you do that.
Similar for ppi; are you afrais that we fail to miss a loop bound
that is set to field_b of a ppi insn?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] sh-sim loose ends
  2004-02-12 22:39 ` Joern Rennecke
@ 2004-02-12 23:52   ` Michael Snyder
  2004-02-13 12:29     ` Joern Rennecke
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2004-02-12 23:52 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: Joern Rennecke, gdb-patches

Joern Rennecke wrote:
>>	* gencode.c (movt): Modifies R[n]; call 'L' macro.
> 
> 
> That doesn't make sense, 'L' simulates the data read memory latency of an
> SH[123].  movt doesn't incur such a latency.

Hmmm?  I thought 'L' was for registers, and 'MA' was for memory access.
This is the only instruction that modifies a gpr but doesn't call 'L'.
Figured it was an oversight.

>>	(trapa): Factor out duplicate variable 'imm' (same as 'i').
> 
> OK.  But I don't see why you need the cast to long.

I guess I don't.

>>	(sleep, trapa, ppi): Use SET_NIP to modify nip.
> 
> 
> I don't see any need for this.  RAISE_EXCEPTION already clears
> saved_state.asregs.insn_end , so that takes care of the
> exceptions that might arise during sleep.
> For trapa, that leaves just the possibility that we fail to miss a
> loop bound that is set to somewhere inside a profiler trap; I think
> you deserve whatever you get when you do that.
> Similar for ppi; are you afrais that we fail to miss a loop bound
> that is set to field_b of a ppi insn?

Actually I was just going for consistancy (everywhere else that
modifies nip uses SET_NIP).  I'll gladly drop this part of the
patch if you don't like it.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] sh-sim loose ends
  2004-02-12 23:52   ` Michael Snyder
@ 2004-02-13 12:29     ` Joern Rennecke
  2004-02-13 19:17       ` Michael Snyder
  0 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2004-02-13 12:29 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Joern Rennecke, Joern Rennecke, gdb-patches

> Hmmm?  I thought 'L' was for registers, and 'MA' was for memory access.
> This is the only instruction that modifies a gpr but doesn't call 'L'.
> Figured it was an oversight.

No, MA is for the memory *access conflict* between data access and
instruction fetch for the non/unified cache SH[123], while L is for
the memory access *latency* on SH[123].

And, of course, there is also no L for "mov <REG_M>,<REG_N>".

> Actually I was just going for consistancy (everywhere else that
> modifies nip uses SET_NIP).  I'll gladly drop this part of the
> patch if you don't like it.

Well, I don't like it because it adds more code unnecessarily, thus
increasing the working set.
And it isn't really inconsistent; SET_NIP is used when nip is changed
for something other than linear instruction fetch, so that we know
if we'll sooner hit a loop boundary or the end of memory.
ppi_insn and the profile trap just do linear instruction fetch for
larger instructions; sleep / trap 0xc3 re-fetches the same instruction
again.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] sh-sim loose ends
  2004-02-13 12:29     ` Joern Rennecke
@ 2004-02-13 19:17       ` Michael Snyder
  2004-02-14  4:48         ` Joern Rennecke
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2004-02-13 19:17 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: Joern Rennecke, gdb-patches

Joern Rennecke wrote:
>>Hmmm?  I thought 'L' was for registers, and 'MA' was for memory access.
>>This is the only instruction that modifies a gpr but doesn't call 'L'.
>>Figured it was an oversight.
> 
> 
> No, MA is for the memory *access conflict* between data access and
> instruction fetch for the non/unified cache SH[123], while L is for
> the memory access *latency* on SH[123].
> 
> And, of course, there is also no L for "mov <REG_M>,<REG_N>".
> 
> 
>>Actually I was just going for consistancy (everywhere else that
>>modifies nip uses SET_NIP).  I'll gladly drop this part of the
>>patch if you don't like it.
> 
> 
> Well, I don't like it because it adds more code unnecessarily, thus
> increasing the working set.
> And it isn't really inconsistent; SET_NIP is used when nip is changed
> for something other than linear instruction fetch, so that we know
> if we'll sooner hit a loop boundary or the end of memory.
> ppi_insn and the profile trap just do linear instruction fetch for
> larger instructions; sleep / trap 0xc3 re-fetches the same instruction
> again.

OK, I'll drop those.  I assume the others are OK?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] sh-sim loose ends
  2004-02-13 19:17       ` Michael Snyder
@ 2004-02-14  4:48         ` Joern Rennecke
  0 siblings, 0 replies; 6+ messages in thread
From: Joern Rennecke @ 2004-02-14  4:48 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Joern Rennecke, Joern Rennecke, gdb-patches

> OK, I'll drop those.  I assume the others are OK?

The whitespace / static patches?  Yes.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-02-14  4:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-02-12 21:56 [RFA] sh-sim loose ends Michael Snyder
2004-02-12 22:39 ` Joern Rennecke
2004-02-12 23:52   ` Michael Snyder
2004-02-13 12:29     ` Joern Rennecke
2004-02-13 19:17       ` Michael Snyder
2004-02-14  4:48         ` Joern Rennecke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox