This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH] Add micromips support to the MIPS simulator


Firstly, sorry for the long delay in replying back to your review comments.

> > --- a/sim/mips/Makefile.in
> > +++ b/sim/mips/Makefile.in
> >
> >  support.o: sim-main.h support.c $(SIM_EXTRA_DEPS)
> >  idecode.o: sim-main.h idecode.c $(SIM_EXTRA_DEPS)
> >  itable.o: sim-main.h itable.c $(SIM_EXTRA_DEPS)
> > -m16run.o: sim-main.h m16_idecode.h m32_idecode.h $(SIM_EXTRA_DEPS)
> > +m16run.o: sim-main.h m16_idecode.h m32_idecode.h m16run.c $(SIM_EXTRA_DEPS)
> > +micromipsrun.o: sim-main.h micromips16_idecode.h micromips32_idecode.h \
> > +	micromips_m32_idecode.h micromipsrun.c $(SIM_EXTRA_DEPS)
> 
> pretty sure you don't need any of this hand maintained dependency info
> anymore.
> can you please try deleting it all instead ?

Done.  I had to keep in the micromipsrun.o, m16run.o and interp.o rules
as these rely on files generated by igen.

> > --- a/sim/mips/configure.ac
> > +++ b/sim/mips/configure.ac
> >
> > +      *:*micromips32*:*)
> > +	# Run igen thrice, once for micromips32, once for micromips16,
> > +        # and once for m32.
> > +	ws="micromips_m32 micromips16 micromips32"
> 
> indentation is broken slightly on the second comment line.  seems to come up a
> few times ... you should fix them all.

This has been fixed.

> 
> > --- a/sim/mips/interp.c
> > +++ b/sim/mips/interp.c
> >
> > +/* microMIPS ISA mode */
> > +int isa_mode;
> 
> why is this a global instead of being part of the sim state ?
> 
> the comment also needs tweaking to match GNU style

This has now been moved to the sim_state structure.

> > --- /dev/null
> > +++ b/sim/mips/micromipsrun.c
> >
> > +/*  Run function for the micromips simulator
> > +
> > +    Copyright (C) 2005-2013 Free Software Foundation, Inc.
> 
> years needs updating

Done.

> 
> > +    This file is part of GDB, the GNU debugger.
> 
> i think you mean:
> 	This file is part of the GNU simulators.

Correct.  I have changed all files my patch changes to have this comment.

 
> > +#define SD sd
> > +#define CPU cpu
> 
> unused ?

No, they are required for some of the macros used in the file so they need to 
stay in.

> 
> > +void
> > +sim_engine_run (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int signal);
> 
> you should include sim-engine.h instead of declaring the prototype yourself

Done.

> -mike


I have attached the full updated patch, and I have inlined the changes I made to
the original patch below.  The patch also changes CIA_{GET/SET} to CPU_PC_{GET/SET},
and moves multi-run.o from sim_multi_obj to SIM_MULTI_OBJ to match the structure of 
the other SIM_*_OBJ variables.   Finally, I also need to add an extra ChangeLog 
entry to account for the change to the sim_state structure:

	sim/mips/
	
	* sim-main.h (sim_state): Add isa_mode field.


Ok to commit?


Regards,



Andrew




diff --git a/sim/mips/Makefile.in b/sim/mips/Makefile.in
index f4beb45..f02e1bd 100644
--- a/sim/mips/Makefile.in
+++ b/sim/mips/Makefile.in
@@ -55,7 +55,9 @@ SIM_MICROMIPS_OBJ = \
 	micromipsrun.o \
 
 
-SIM_MULTI_OBJ = itable.o @sim_multi_obj@
+SIM_MULTI_OBJ = @sim_multi_obj@ \
+		itable.o \
+		multi-run.o \
 
 MIPS_EXTRA_LIBS = @mips_extra_libs@
 
@@ -88,11 +90,11 @@ SIM_EXTRA_LIBS = $(MIPS_EXTRA_LIBS)
 ## COMMON_POST_CONFIG_FRAG
 
 interp.o: $(srcdir)/interp.c config.h sim-main.h itable.h
-cp1.o: $(srcdir)/cp1.c config.h sim-main.h
 
-mdmx.o: $(srcdir)/mdmx.c $(srcdir)/sim-main.h
+m16run.o: sim-main.h m16_idecode.h m32_idecode.h m16run.c $(SIM_EXTRA_DEPS)
 
-dsp.o: $(srcdir)/dsp.c $(srcdir)/sim-main.h
+micromipsrun.o: sim-main.h micromips16_idecode.h micromips32_idecode.h \
+		micromips_m32_idecode.h micromipsrun.c $(SIM_EXTRA_DEPS)
 
 multi-run.o: multi-include.h tmp-mach-multi
 
@@ -199,43 +201,6 @@ tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE)
 	$(SHELL) $(srcdir)/../../move-if-change tmp-irun.c irun.c
 	touch tmp-igen
 
-semantics.o: sim-main.h semantics.c $(SIM_EXTRA_DEPS)
-engine.o: sim-main.h engine.c $(SIM_EXTRA_DEPS)
-support.o: sim-main.h support.c $(SIM_EXTRA_DEPS)
-idecode.o: sim-main.h idecode.c $(SIM_EXTRA_DEPS)
-itable.o: sim-main.h itable.c $(SIM_EXTRA_DEPS)
-m16run.o: sim-main.h m16_idecode.h m32_idecode.h m16run.c $(SIM_EXTRA_DEPS)
-micromipsrun.o: sim-main.h micromips16_idecode.h micromips32_idecode.h \
-	micromips_m32_idecode.h micromipsrun.c $(SIM_EXTRA_DEPS)
-
-m16_semantics.o: sim-main.h m16_semantics.c $(SIM_EXTRA_DEPS)
-m16_support.o: sim-main.h m16_support.c $(SIM_EXTRA_DEPS)
-m16_idecode.o: sim-main.h m16_idecode.c $(SIM_EXTRA_DEPS)
-m16_icache.o: sim-main.h m16_icache.c $(SIM_EXTRA_DEPS)
-
-micromips_m32_semantics.o: sim-main.h micromips_m32_semantics.c \
-	$(SIM_EXTRA_DEPS)
-micromips_m32_support.o: sim-main.h micromips_m32_support.c $(SIM_EXTRA_DEPS)
-micromips_m32_idecode.o: sim-main.h micromips_m32_idecode.c $(SIM_EXTRA_DEPS)
-micromips_m32_icache.o: sim-main.h micromips_m32_icache.c $(SIM_EXTRA_DEPS)
-
-m32_semantics.o: sim-main.h m32_semantics.c $(SIM_EXTRA_DEPS)
-m32_support.o: sim-main.h m32_support.c $(SIM_EXTRA_DEPS)
-m32_idecode.o: sim-main.h m32_idecode.c $(SIM_EXTRA_DEPS)
-m32_icache.o: sim-main.h m32_icache.c $(SIM_EXTRA_DEPS)
-
-micromips16_semantics.o: sim-main.h micromips16_semantics.c $(SIM_EXTRA_DEPS)
-micromips16_support.o: sim-main.h micromips16_support.c $(SIM_EXTRA_DEPS)
-micromips16_idecode.o: sim-main.h micromips16_idecode.c $(SIM_EXTRA_DEPS)
-micromips16_icache.o: sim-main.h micromips16_icache.c $(SIM_EXTRA_DEPS)
-
-micromips32_semantics.o: sim-main.h micromips32_semantics.c $(SIM_EXTRA_DEPS)
-micromips32_support.o: sim-main.h micromips32_support.c $(SIM_EXTRA_DEPS)
-micromips32_idecode.o: sim-main.h micromips32_idecode.c $(SIM_EXTRA_DEPS)
-micromips32_icache.o: sim-main.h micromips32_icache.c $(SIM_EXTRA_DEPS)
-
-$(SIM_MULTI_OBJ): sim-main.h $(SIM_EXTRA_DEPS)
-
 BUILT_SRC_FROM_M16 = \
 	m16_icache.h \
 	m16_icache.c \
diff --git a/sim/mips/configure.ac b/sim/mips/configure.ac
index e2a4871..a642326 100644
--- a/sim/mips/configure.ac
+++ b/sim/mips/configure.ac
@@ -228,7 +228,7 @@ if test ${sim_gen} = MULTI; then
   rm -f multi-include.h multi-run.c
   sim_multi_flags=
   sim_multi_src=
-  sim_multi_obj=multi-run.o
+  sim_multi_obj=
   sim_multi_igen_configs=
   sim_seen_default=no
 
@@ -318,7 +318,7 @@ __EOF__
 	;;
       *:*micromips32*:*)
 	# Run igen thrice, once for micromips32, once for micromips16,
-        # and once for m32.
+	# and once for m32.
 	ws="micromips_m32 micromips16 micromips32"
 
 	# The top-level function for the micromips simulator is
@@ -330,7 +330,7 @@ __EOF__
 	;;
       *:*micromips64*:*)
 	# Run igen thrice, once for micromips64, once for micromips16,
-        # and once for m64.
+	# and once for m64.
 	ws="micromips_m64 micromips16 micromips64"
 
 	# The top-level function for the micromips simulator is
@@ -436,8 +436,6 @@ AC_SUBST(sim_multi_flags)
 AC_SUBST(sim_multi_igen_configs)
 AC_SUBST(sim_multi_src)
 AC_SUBST(sim_multi_obj)
-
-
 #
 # Add simulated hardware devices
 #
diff --git a/sim/mips/dsp.igen b/sim/mips/dsp.igen
index 35c90d0..b8d5a6c 100644
--- a/sim/mips/dsp.igen
+++ b/sim/mips/dsp.igen
@@ -4,7 +4,7 @@
 // Copyright (C) 2005-2015 Free Software Foundation, Inc.
 // Contributed by MIPS Technologies, Inc.  Written by Chao-ying Fu.
 //
-// This file is part of GDB, the GNU debugger.
+// This file is part of the MIPS sim
 //
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
diff --git a/sim/mips/dsp2.igen b/sim/mips/dsp2.igen
index 06b5a68..a871026 100644
--- a/sim/mips/dsp2.igen
+++ b/sim/mips/dsp2.igen
@@ -5,7 +5,7 @@
 // Contributed by MIPS Technologies, Inc.
 // Written by Chao-ying Fu (fu@mips.com).
 //
-// This file is part of GDB, the GNU debugger.
+// This file is part of the MIPS sim
 //
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
diff --git a/sim/mips/interp.c b/sim/mips/interp.c
index 686de61..9dc8964 100644
--- a/sim/mips/interp.c
+++ b/sim/mips/interp.c
@@ -146,9 +146,6 @@ static SIM_ADDR lsipmon_monitor_base = 0xBFC00200;
 
 static SIM_RC sim_firmware_command (SIM_DESC sd, char* arg);
 
-/* microMIPS ISA mode */
-int isa_mode;
-
 #define MEM_SIZE (8 << 20)	/* 8 MBytes */
 
 
diff --git a/sim/mips/micromips.igen b/sim/mips/micromips.igen
index fdd0368..2c62376 100644
--- a/sim/mips/micromips.igen
+++ b/sim/mips/micromips.igen
@@ -1,9 +1,9 @@
 // Simulator definition for the micromips ASE.
-// Copyright (C) 2005-2013 Free Software Foundation, Inc.
+// Copyright (C) 2005-2015 Free Software Foundation, Inc.
 // Contributed by Imagination Technologies, Ltd.
 // Written by Andrew Bennett <andrew.bennett@imgtec.com>
 //
-// This file is part of GDB, the GNU debugger.
+// This file is part of the MIPS sim.
 //
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -53,7 +53,7 @@
 
 :function:::address_word:process_isa_mode:address_word target
 {
-  isa_mode = target & 0x1;
+  SD->isa_mode = target & 0x1;
   return (target & (-1 << 1));
 }
 
@@ -1063,7 +1063,7 @@
   address_word region = (NIA & MASK (63, 26));
   NIA = do_micromips_jal (SD_, (region | (IMM_SHIFT_2BIT)) | ISA_MODE_MIPS32,
 			  NIA, MICROMIPS_DELAYSLOT_SIZE_32);
-  isa_mode = ISA_MODE_MIPS32;
+  SD->isa_mode = ISA_MODE_MIPS32;
 }
 
 000000,00000,5.RS,0000111100,111100:POOL32A:32::JR
diff --git a/sim/mips/micromipsdsp.igen b/sim/mips/micromipsdsp.igen
index c19de09..daa9f83 100644
--- a/sim/mips/micromipsdsp.igen
+++ b/sim/mips/micromipsdsp.igen
@@ -1,9 +1,9 @@
 // Simulator definition for the micromips DSP ASE.
-// Copyright (C) 2005-2013 Free Software Foundation, Inc.
+// Copyright (C) 2005-2015 Free Software Foundation, Inc.
 // Contributed by Imagination Technologies, Ltd.
 // Written by Andrew Bennett <andrew.bennett@imgtec.com>
 //
-// This file is part of GDB, the GNU debugger.
+// This file is part of the MIPS sim.
 //
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
diff --git a/sim/mips/micromipsrun.c b/sim/mips/micromipsrun.c
index f07ad8e..7dd10d7 100644
--- a/sim/mips/micromipsrun.c
+++ b/sim/mips/micromipsrun.c
@@ -1,10 +1,10 @@
 /*  Run function for the micromips simulator
 
-    Copyright (C) 2005-2013 Free Software Foundation, Inc.
+    Copyright (C) 2005-2015 Free Software Foundation, Inc.
     Contributed by Imagination Technologies, Ltd.
     Written by Andrew Bennett <andrew.bennett@imgtec.com>.
 
-    This file is part of GDB, the GNU debugger.
+    This file is part of the MIPS sim.
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -24,14 +24,11 @@
 #include "micromips32_idecode.h"
 #include "micromips_m32_idecode.h"
 #include "bfd.h"
-
+#include "sim-engine.h"
 
 #define SD sd
 #define CPU cpu
 
-void
-sim_engine_run (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int signal);
-
 address_word
 micromips_instruction_decode (SIM_DESC sd, sim_cpu * cpu,
 			      address_word cia,
@@ -74,8 +71,8 @@ sim_engine_run (SIM_DESC sd, int next_cpu_nr, int nr_cpus,
 {
   micromips_m32_instruction_word instruction_0;
   sim_cpu *cpu = STATE_CPU (sd, next_cpu_nr);
-  micromips32_instruction_address cia = CIA_GET (cpu);
-  isa_mode = ISA_MODE_MIPS32;
+  micromips32_instruction_address cia = CPU_PC_GET (cpu);
+  sd->isa_mode = ISA_MODE_MIPS32;
 
   while (1)
     {
@@ -87,17 +84,17 @@ sim_engine_run (SIM_DESC sd, int next_cpu_nr, int nr_cpus,
 	 from the elf header.
 	 2. Setting the correct isa mode after a MIPS32 jump or branch
 	 instruction.  */
-      if ((isa_mode == ISA_MODE_MIPS32)
+      if ((sd->isa_mode == ISA_MODE_MIPS32)
 	  && ((cia & 0x1) == ISA_MODE_MICROMIPS))
 	{
-	  isa_mode = ISA_MODE_MICROMIPS;
+	  sd->isa_mode = ISA_MODE_MICROMIPS;
 	  cia = cia & ~0x1;
 	}
 
 #if defined (ENGINE_ISSUE_PREFIX_HOOK)
       ENGINE_ISSUE_PREFIX_HOOK ();
 #endif
-      switch (isa_mode)
+      switch (sd->isa_mode)
 	{
 	case ISA_MODE_MICROMIPS:
 	  nia =
@@ -122,9 +119,9 @@ sim_engine_run (SIM_DESC sd, int next_cpu_nr, int nr_cpus,
       /* process any events */
       if (sim_events_tick (sd))
 	{
-	  CIA_SET (CPU, cia);
+	  CPU_PC_SET (cpu, cia);
 	  sim_events_process (sd);
-	  cia = CIA_GET (CPU);
+	  cia = CPU_PC_GET (cpu);
 	}
     }
 }
diff --git a/sim/mips/mips3264r2.igen b/sim/mips/mips3264r2.igen
index e003664..1c299c3 100644
--- a/sim/mips/mips3264r2.igen
+++ b/sim/mips/mips3264r2.igen
@@ -4,7 +4,7 @@
 // Copyright (C) 2004-2015 Free Software Foundation, Inc.
 // Contributed by David Ung, of MIPS Technologies.
 //
-// This file is part of GDB, the GNU debugger.
+// This file is part of the MIPS sim.
 // 
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
diff --git a/sim/mips/sim-main.h b/sim/mips/sim-main.h
index 4bfc06c..42d8db3 100644
--- a/sim/mips/sim-main.h
+++ b/sim/mips/sim-main.h
@@ -2,7 +2,7 @@
    Copyright (C) 1997-2015 Free Software Foundation, Inc.
    Contributed by Cygnus Support.
 
-This file is part of GDB, the GNU debugger.
+This file is part of the MIPS sim.
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -493,6 +493,9 @@ struct sim_state {
 
   sim_cpu *cpu[MAX_NR_PROCESSORS];
 
+  /* microMIPS ISA mode.  */
+  int isa_mode;
+
   sim_state_base base;
 };
 
diff --git a/sim/testsuite/sim/mips/hilo-hazard-4.s b/sim/testsuite/sim/mips/hilo-hazard-4.s
index c489a4f..e83fbfa 100644
--- a/sim/testsuite/sim/mips/hilo-hazard-4.s
+++ b/sim/testsuite/sim/mips/hilo-hazard-4.s
@@ -5,11 +5,11 @@
 # ld:		-N -Ttext=0x80010000
 # output:	pass\\n
 
-# Copyright (C) 2013 Imagination Technologies, Ltd.
+# Copyright (C) 2013-2015 Imagination Technologies, Ltd.
 # All rights reserved.
 # Contributed by Andrew Bennett (andrew.bennett@imgtec.com)
 #
-# This file is part of the GNU simulators.
+# This file is part of the MIPS sim.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by

Attachment: Add-micromips-support-to-MIPS-simulator-rev2.patch.gz
Description: Add-micromips-support-to-MIPS-simulator-rev2.patch.gz


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]