[PATCH] Add micromips support to the MIPS simulator

[PATCH] Add micromips support to the MIPS simulator

[Andrew Bennett]

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

Hi,

This patch adds support for micromips to the MIPS simulator.  For more
information on micromips ASE please refer to the following URL:
http://www.imgtec.com/mips/architectures/mips32.asp, which also contains links to
the ISA documentation.

I have refactored the exisiting MIPS backend to place common instruction code
into functions.  I have then added the micromips instruction definitions.  Finally 
I have added the build system and testsuite support.

To test the new backend I have extended the current MIPS tests to support
micromips.  I have also run it through the GCC testsuite.  

The patch is attached to this email, and the ChangeLog entry is below:

Ok to commit?

Many thanks,


Andrew


2015-01-21  Andrew Bennett  <andrew.bennett@imgtec.com>
 	      Ali Lown  <ali.lown@imgtec.com>

	sim/common/
	* sim-bits.h (EXTEND6): New macro.
	(EXTEND12): New macro. 
	(EXTEND25): New macro.

	sim/mips/
	* Makefile.in (tmp-micromips): New rule.	
	(tmp-mach-multi): Add support for micromips.	
	* configure.ac (mips*-sde-elf* | mips*-mti-elf*): Made a multi sim
	that works for both mips64 and micromips64.
	(mipsisa32r2*-*-*): Made a multi sim that works for mips32 and
	micromips32.
	Add build support for micromips.
	* dsp.igen (do_ph_s_absq, do_w_s_absq, do_qb_s_absq, do_addsc, 
	do_addwc, do_bitrev, do_extpv, do_extrv, do_extrv_s_h, do_insv, 
	do_lxx do_modsub, do_mthlip, do_mulsaq_s_w_ph, do_ph_packrl, do_qb_pick
	do_ph_pick, do_qb_ph_precequ, do_qb_ph_preceu, do_w_preceq
	do_w_ph_precrq, do_ph_qb_precrq, do_w_ph_rs_precrq do_qb_w_raddu, 
	do_rddsp, do_repl, do_shilov, do_ph_shl, do_qb_shl do_w_s_shllv, 
	do_ph_shrlv, do_w_r_shrav, do_wrdsp, do_qb_shrav, do_append, 
	do_balign, do_ph_w_mulsa, do_ph_qb_precr, do_prepend): New functions.
	Refactored instruction code to use these functions.
	* dsp2.igen: Refactored instruction code to use the new functions.
	* interp.c (decode_coproc): Refactored to work with any instruction
	encoding.
	(isa_mode): New variable 
	(RSVD_INSTRUCTION): Changed to 0x00000039.
	* m16.igen (BREAK16): Refactored instruction to use do_break16.
	(JALX32): Add mips32, mips64, mips32r2 and mips64r2 models.
	* micromips.dc: New file.
	* micromips.igen: New file. 
	* micromips16.dc: New file.
	* micromipsdsp.igen: New file.
	* micromipsrun.c: New file.
	* mips.igen (do_swc1): Changed to work with any instruction encoding.
	(do_add do_addi do_andi do_dadd do_daddi do_dsll32 do_dsra32
	do_dsrl32, do_dsub, do_break, do_break16, do_clo, do_clz, do_dclo
	do_dclz, do_lb, do_lh, do_lwr, do_lwl, do_lwc, do_lw, do_lwu, do_lhu
	do_ldc, do_lbu, do_ll, do_lld, do_lui, do_madd, do_dsp_madd, do_maddu
	do_dsp_maddu, do_dsp_mfhi, do_dsp_mflo, do_movn, do_movz, do_msub
	do_dsp_msub, do_msubu, do_dsp_msubu, do_mthi, do_dsp_mthi, do_mtlo
	do_dsp_mtlo, do_mul, do_dsp_mult, do_dsp_multu, do_pref, do_sc, do_scd
	do_sub, do_sw, do_teq, do_teqi, do_tge, do_tgei, do_tgeiu, do_tgeu, do_tlt
	do_tlti, do_tltiu, do_tltu, do_tne, do_tnei, do_abs_fmt, do_add_fmt
	do_alnv_ps, do_c_cond_fmt, do_ceil_fmt, do_cfc1, do_ctc1, do_cvt_d_fmt
	do_cvt_l_fmt, do_cvt_ps_s, do_cvt_s_fmt, do_cvt_s_pl, do_cvt_s_pu
	do_cvt_w_fmt, do_div_fmt, do_dmfc1b, do_dmtc1b, do_floor_fmt, do_luxc1_32
	do_luxc1_64, do_lwc1, do_lwxc1, do_madd_fmt, do_mfc1b, do_mov_fmt, do_movtf
	do_movtf_fmt, do_movn_fmt, do_movz_fmt, do_msub_fmt, do_mtc1b, do_mul_fmt
	do_neg_fmt, do_nmadd_fmt, do_nmsub_fmt, do_pll_ps, do_plu_ps, do_pul_ps
	do_puu_ps, do_recip_fmt, do_round_fmt, do_rsqrt_fmt, do_prefx, do_sdc1
	do_suxc1_32, do_suxc1_64, do_sqrt_fmt, do_sub_fmt, do_swc1, do_swxc1
	do_trunc_fmt): New functions, refactored from existing instructions.
	Refactored instruction code to use these functions.
	(RSVD): Changed to use new reserved instruction.
	(loadstore_ea, not_word_value, unpredictable, check_mt_hilo, check_mf_hilo, 
	check_mult_hilo, check_div_hilo, check_u64, do_luxc1_32, do_sdc1, do_suxc1_32, 
	check_fmt_p, check_fpu, do_load_double, do_store_double):  Added micromips32 
	and micromips64 models.
	Added include for micromips.igen and micromipsdsp.igen
	Add micromips32 and micromips64 models.
	(DecodeCoproc): Updated to use new macro definition.
	* mips3264r2.igen (do_dsbh, do_dshd, do_dext, do_dextm, do_dextu, do_di, 
	do_dins, do_dinsm, do_ei, do_ext, do_mfhc1, do_mthc1, do_ins, do_dinsu, 
	do_seb, do_seh do_rdhwr, do_wsbh): New functions.
	Refactored instruction code to use these functions.
	* sim-main.h (CP0_operation): New enum.
	(DecodeCoproc): Updated macro.
	(IMEM32_MICROMIPS, IMEM16_MICROMIPS, MICROMIPS_MINOR_OPCODE, 
	MICROMIPS_DELAYSLOT_SIZE_ANY, MICROMIPS_DELAYSLOT_SIZE_16, MICROMIPS_DELAYSLOT_SIZE_32,
	ISA_MODE_MIPS32 and ISA_MODE_MICROMIPS): New defines.

	sim/testsuite/sim/mips/
       * basic.exp (run_micromips_test, run_sim_tests): New functions
	Add support for micromips tests. 
	* hilo-hazard-4.s: New file.
	* testutils.inc (_dowrite): Changed reserved instruction encoding.
	(writemsg): Moved the la and li instructions before the data they are 
	assigned to, which prevents a bug where MIPS32 relocations are used instead 
	of micromips relocations when building for micromips.


Andrew Bennett
Software Design Engineer, MIPS Processor IP
Imagination Technologies Limited
t: +44 (0)113 2429814
www.imgtec.com


[-- Attachment #2: Add-micromips-support-to-MIPS-simulator.patch.gz --]
[-- Type: application/x-gzip, Size: 41125 bytes --]

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

[Mike Frysinger]

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

On 21 Jan 2015 12:44, Andrew Bennett wrote:

the igen files i pretty much skimmed over

> --- 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 ?

> --- 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.

> --- 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

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

years needs updating

> +    This file is part of GDB, the GNU debugger.

i think you mean:
	This file is part of the GNU simulators.

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

unused ?

> +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
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Andrew Bennett]

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

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 #2: Add-micromips-support-to-MIPS-simulator-rev2.patch.gz --]
[-- Type: application/x-gzip, Size: 3036 bytes --]

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

[Andrew Bennett]

Ping.

I was wondering if anyone has had a chance to look at this yet?

Many thanks,


Andrew

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Andrew Bennett
> Sent: 27 August 2015 16:05
> To: Mike Frysinger
> Cc: gdb-patches@sourceware.org
> Subject: 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

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

[Mike Frysinger]

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

On 27 Aug 2015 15:05, Andrew Bennett wrote:
> > > +#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.

this needs to be documented as to why.  other ports don't need this, and
it sounds like something wrong with the mips port that should be fixed at
some point.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Andrew Bennett]

> > No, they are required for some of the macros used in the file so they need
> to
> > stay in.
> 
> this needs to be documented as to why.  other ports don't need this, and
> it sounds like something wrong with the mips port that should be fixed at
> some point.

The definitions come from the *_support.h files generated by igen and are required 
because they are used in some of the macros in the micromipsrun.c file.  
Unfortunately we can not just blindly include the *_support.h files to get these 
definitions because some of the defines in these files are specific for a particular 
configuration of the simulator for example instruction word size is 16 bits for 
micromips16 and 32 bits for micromips32.  This means we could break future code 
changes by doing this, so a safer approach is to just extract the defines that 
we need to get the file to compile.  

I have added a comment above the defines which explains this.  The diff from my
previous patch is below.

Ok to commit?

Many thanks,



Andrew


diff --git a/sim/mips/micromipsrun.c b/sim/mips/micromipsrun.c
index 7dd10d7..c39138b 100644
--- a/sim/mips/micromipsrun.c
+++ b/sim/mips/micromipsrun.c
@@ -26,6 +26,14 @@
 #include "bfd.h"
 #include "sim-engine.h"
 
+/* These definitions come from the *_support.h files generated by igen and are
+   required because they are used in some of the macros in the code below.
+   Unfortunately we can not just blindly include the *_support.h files to get
+   these definitions because some of the defines in these files are specific
+   for a particular configuration of the simulator for example instruction word
+   size is 16 bits for micromips16 and 32 bits for micromips32.  This means we
+   could break future code changes by doing this, so a safer approach is to just
+   extract the defines that we need to get this file to compile.  */
 #define SD sd
 #define CPU cpu

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

[Mike Frysinger]

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

On 25 Sep 2015 12:06, Andrew Bennett wrote:
> Ok to commit?

OK, thanks !
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Andrew Bennett]

> On 25 Sep 2015 12:06, Andrew Bennett wrote:
>> Ok to commit?
> OK, thanks !

This has now been committed.  Once I did the committing I realised that
I only have Binutils write access permissions.  Is this ok, or do I need
to get GDB write access as well?

Many thanks,


Andrew

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

[Mike Frysinger]

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

On 25 Sep 2015 20:22, Andrew Bennett wrote:
> > On 25 Sep 2015 12:06, Andrew Bennett wrote:
> >> Ok to commit?
> > OK, thanks !
> 
> This has now been committed.  Once I did the committing I realised that
> I only have Binutils write access permissions.  Is this ok, or do I need
> to get GDB write access as well?

binutils & gdb are the same repo now, and thus there's a single set
of acls controlling `git push`.  your patch has been approved, so
that's all you need.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Joel Brobecker]

Hello Andrew,

> > --- 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.

Any reason why this file has a copyright header not citing
the FSF as the copyright holder? I am asking because although
the sim is not formally part of the GDB project, we do prefer
sim contributions to be assigned to the FSF.

Assuming this was just an oversight, would it be OK for me to change
the holder to the FSF?

Thank you!
-- 
Joel

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

[Andrew Bennett]

> Any reason why this file has a copyright header not citing
> the FSF as the copyright holder? I am asking because although
> the sim is not formally part of the GDB project, we do prefer
> sim contributions to be assigned to the FSF.

I was following the convention from the mips32-dsp.s and mips32-dsp2.s
testsuite files.

> Assuming this was just an oversight, would it be OK for me to change
> the holder to the FSF?

Yes, that will be fine.


Regards,


Andrew

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

[Joel Brobecker]

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

> > Any reason why this file has a copyright header not citing
> > the FSF as the copyright holder? I am asking because although
> > the sim is not formally part of the GDB project, we do prefer
> > sim contributions to be assigned to the FSF.
> 
> I was following the convention from the mips32-dsp.s and mips32-dsp2.s
> testsuite files.

I thought this might be the case...

> > Assuming this was just an oversight, would it be OK for me to change
> > the holder to the FSF?
> 
> Yes, that will be fine.

Thanks a lot. Done with the following commit.

sim/testsuite/sim/mips/ChangeLog:

        * hilo-hazard-4.s: Change copyright ownder to FSF.

-- 
Joel

[-- Attachment #2: 0001-Change-copyright-owner-to-FSF-in-sim-testsuite-sim-m.patch --]
[-- Type: text/x-diff, Size: 1434 bytes --]

From 68477034727ad85aeed248ec995da746f7639e53 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 6 Jan 2016 09:38:21 +0400
Subject: [PATCH] Change copyright owner to FSF in
 sim/testsuite/sim/mips/hilo-hazard-4.s

sim/testsuite/sim/mips/ChangeLog:

        * hilo-hazard-4.s: Change copyright ownder to FSF.
---
 sim/testsuite/sim/mips/ChangeLog       | 4 ++++
 sim/testsuite/sim/mips/hilo-hazard-4.s | 3 +--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sim/testsuite/sim/mips/ChangeLog b/sim/testsuite/sim/mips/ChangeLog
index ada4e4c..ea95441 100644
--- a/sim/testsuite/sim/mips/ChangeLog
+++ b/sim/testsuite/sim/mips/ChangeLog
@@ -1,3 +1,7 @@
+2016-01-06  Joel Brobecker  <brobecker@adacore.com>
+
+	* hilo-hazard-4.s: Change copyright ownder to FSF.
+
 2015-09-25  Andrew Bennett  <andrew.bennett@imgtec.com>
 	    Ali Lown  <ali.lown@imgtec.com>
 
diff --git a/sim/testsuite/sim/mips/hilo-hazard-4.s b/sim/testsuite/sim/mips/hilo-hazard-4.s
index e83fbfa..8a4c888 100644
--- a/sim/testsuite/sim/mips/hilo-hazard-4.s
+++ b/sim/testsuite/sim/mips/hilo-hazard-4.s
@@ -5,8 +5,7 @@
 # ld:		-N -Ttext=0x80010000
 # output:	pass\\n
 
-# Copyright (C) 2013-2015 Imagination Technologies, Ltd.
-# All rights reserved.
+# Copyright (C) 2013-2016 Free Software Foundation, Inc.
 # Contributed by Andrew Bennett (andrew.bennett@imgtec.com)
 #
 # This file is part of the MIPS sim.
-- 
2.5.0

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

[Maciej W. Rozycki]

Andrew,

On Fri, 25 Sep 2015, Andrew Bennett wrote:

> This has now been committed.  Once I did the committing I realised that
> I only have Binutils write access permissions.  Is this ok, or do I need
> to get GDB write access as well?

 While investigating PR 18964 I noticed your change (commit 8e394ffc) 
broke building for the mips-linux-gnu target, which does not enable the 
microMIPS instruction set:

gcc -DHAVE_CONFIG_H     -DPROFILE=1 -DWITH_PROFILE=-1  -DWITH_ALIGNMENT=NONSTRICT_ALIGNMENT -DWITH_TARGET_WORD_BITSIZE=32 -DWITH_TARGET_WORD_MSB=31 -DWITH_DEFAULT_TARGET_BYTE_ORDER=BFD_ENDIAN_BIG  -DWITH_FLOATING_POINT=HARD_FLOATING_POINT -DWITH_TARGET_FLOATING_POINT_BITSIZE=32 -DWITH_HW=0    -DWITH_RESERVED_BITS=1  -DWITH_SMP=0  -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wold-style-definition -Wformat-nonliteral    -DMIPS_MACH_DEFAULT=bfd_mach_mips8000   -I. -I.../sim/mips -I../common -I.../sim/mips/../common -I../../include -I.../sim/mips/../..//include -I../../bfd -I.../sim/mips/../..//bfd -I../../opcodes -I.../sim/mips/../..//opcodes  -pipe -O0 -g  -o run \
          nrun.o libsim.a ../../bfd/libbfd.a ../../opcodes/libopcodes.a  ../../libiberty/libiberty.a -lm -ldl -lnsl  -L../../zlib -lz
libsim.a(engine.o): In function `delayslot_micromips':
.../sim/mips/micromips.igen:49: undefined reference to `micromips_instruction_decode'
libsim.a(semantics.o): In function `delayslot_micromips':
.../sim/mips/micromips.igen:49: undefined reference to `micromips_instruction_decode'
libsim.a(idecode.o): In function `delayslot_micromips':
.../sim/mips/micromips.igen:49: undefined reference to `micromips_instruction_decode'
collect2: ld returned 1 exit status
make[2]: *** [run] Error 1

As a microMIPS engine is not included in a mips-linux-gnu build the file 
supposed to provide `micromips_instruction_decode' is not generated or 
compiled, whereas `delayslot_micromips' (and a bunch of other functions) 
is built unconditionally.

 Restricting the affected functions to microMIPS processors only has fixed 
the build problem for me, see the patch below.  I have not verified it 
further though, will you be able to look into it soon?

	sim/mips/
	* micromips.igen (delayslot_micromips): Enable for `micromips32',
	`micromips64' and `micromipsdsp' only.
	(process_isa_mode): Enable for `micromips32' and `micromips64'
	only.
	(do_micromips_jalr, do_micromips_jal): Likewise.
	(compute_movep_src_reg): Likewise.
	(compute_andi16_imm): Likewise.
	(convert_fmt_micromips): Likewise.
	(convert_fmt_micromips_cvt_d): Likewise.
	(convert_fmt_micromips_cvt_s): Likewise.

  Maciej

sim-umips-function-proc.diff
Index: gdb/sim/mips/micromips.igen
===================================================================
--- gdb.orig/sim/mips/micromips.igen	2016-01-12 22:29:38.000000000 +0000
+++ gdb/sim/mips/micromips.igen	2016-01-12 22:37:35.169972819 +0000
@@ -39,6 +39,9 @@
 :compute:::int:IMM_SHIFT_2BIT:IMMEDIATE:(IMMEDIATE << 2)
 
 :function:::address_word:delayslot_micromips:address_word target, address_word nia, int delayslot_instruction_size
+*micromips32:
+*micromips64:
+*micromipsdsp:
 {
   instruction_word delay_insn;
   sim_events_slip (SD, 1);
@@ -52,12 +55,16 @@
 }
 
 :function:::address_word:process_isa_mode:address_word target
+*micromips32:
+*micromips64:
 {
   SD->isa_mode = target & 0x1;
   return (target & (-(1 << 1)));
 }
 
 :function:::address_word:do_micromips_jalr:int rt, int rs, address_word nia, int delayslot_instruction_size
+*micromips32:
+*micromips64:
 {
   GPR[rt] = (nia + delayslot_instruction_size) | ISA_MODE_MICROMIPS;
   return (process_isa_mode (SD_,
@@ -65,6 +72,8 @@
 }
 
 :function:::address_word:do_micromips_jal:address_word target, address_word nia, int delayslot_instruction_size
+*micromips32:
+*micromips64:
 {
   RA = (nia + delayslot_instruction_size) | ISA_MODE_MICROMIPS;
   return delayslot_micromips (SD_, target, nia, delayslot_instruction_size);
@@ -72,6 +81,8 @@
 
 
 :function:::unsigned32:compute_movep_src_reg:int reg
+*micromips32:
+*micromips64:
 {
   switch(reg)
     {
@@ -88,6 +99,8 @@
 }
 
 :function:::unsigned32:compute_andi16_imm:int encoded_imm
+*micromips32:
+*micromips64:
 {
   switch (encoded_imm)
     {
@@ -112,6 +125,8 @@
 }
 
 :function:::FP_formats:convert_fmt_micromips:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -123,6 +138,8 @@
 }
 
 :function:::FP_formats:convert_fmt_micromips_cvt_d:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -135,6 +152,8 @@
 
 
 :function:::FP_formats:convert_fmt_micromips_cvt_s:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {

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

[Andrew Bennett]

> As a microMIPS engine is not included in a mips-linux-gnu build the file
> supposed to provide `micromips_instruction_decode' is not generated or
> compiled, whereas `delayslot_micromips' (and a bunch of other functions)
> is built unconditionally.
> 
>  Restricting the affected functions to microMIPS processors only has fixed
> the build problem for me, see the patch below.  I have not verified it
> further though, will you be able to look into it soon?

Hi Maciej,

The patch looks good, I noticed that the FMT_MICROMIPS, FMT_MICROMIPS_CVT_D
and FMT_MICROMIPS_CVT_S functions also needed fixing.  I have built it for the 
mips-elf, mips-linux-gnu and mips-mti-elf targets and they all build successfully.
The updated patch and ChangeLog is below.

Ok to commit?

Many thanks,



Andrew  


	sim/mips/
 	* micromips.igen (delayslot_micromips): Enable for `micromips32',
 	`micromips64' and `micromipsdsp' only.
 	(process_isa_mode): Enable for `micromips32' and `micromips64'
 	only.
 	(do_micromips_jalr, do_micromips_jal): Likewise.
 	(compute_movep_src_reg): Likewise.
 	(compute_andi16_imm): Likewise.
 	(convert_fmt_micromips): Likewise.
 	(convert_fmt_micromips_cvt_d): Likewise.
 	(convert_fmt_micromips_cvt_s): Likewise.
	(FMT_MICROMIPS): Likewise
	(FMT_MICROMIPS_CVT_D): Likewise
	(FMT_MICROMIPS_CVT_S): Likewise


--- a/sim/mips/micromips.igen
+++ b/sim/mips/micromips.igen
@@ -39,6 +39,9 @@
 :compute:::int:IMM_SHIFT_2BIT:IMMEDIATE:(IMMEDIATE << 2)
 
 :function:::address_word:delayslot_micromips:address_word target, address_word nia, int delayslot_instruction_size
+*micromips32:
+*micromips64:
+*micromipsdsp:
 {
   instruction_word delay_insn;
   sim_events_slip (SD, 1);
@@ -52,12 +55,16 @@
 }
 
 :function:::address_word:process_isa_mode:address_word target
+*micromips32:
+*micromips64:
 {
   SD->isa_mode = target & 0x1;
   return (target & (-(1 << 1)));
 }
 
 :function:::address_word:do_micromips_jalr:int rt, int rs, address_word nia, int delayslot_instruction_size
+*micromips32:
+*micromips64:
 {
   GPR[rt] = (nia + delayslot_instruction_size) | ISA_MODE_MICROMIPS;
   return (process_isa_mode (SD_,
@@ -65,6 +72,8 @@
 }
 
 :function:::address_word:do_micromips_jal:address_word target, address_word nia, int delayslot_instruction_size
+*micromips32:
+*micromips64:
 {
   RA = (nia + delayslot_instruction_size) | ISA_MODE_MICROMIPS;
   return delayslot_micromips (SD_, target, nia, delayslot_instruction_size);
@@ -72,6 +81,8 @@
 
 
 :function:::unsigned32:compute_movep_src_reg:int reg
+*micromips32:
+*micromips64:
 {
   switch(reg)
     {
@@ -88,6 +99,8 @@
 }
 
 :function:::unsigned32:compute_andi16_imm:int encoded_imm
+*micromips32:
+*micromips64:
 {
   switch (encoded_imm)
     {
@@ -112,6 +125,8 @@
 }
 
 :function:::FP_formats:convert_fmt_micromips:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -123,6 +138,8 @@
 }
 
 :function:::FP_formats:convert_fmt_micromips_cvt_d:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -135,6 +152,8 @@
 
 
 :function:::FP_formats:convert_fmt_micromips_cvt_s:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -2252,6 +2271,8 @@
 
 
 :%s::::FMT_MICROMIPS:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -2264,6 +2285,8 @@
 
 
 :%s::::FMT_MICROMIPS_CVT_D:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {
@@ -2276,6 +2299,8 @@
 
 
 :%s::::FMT_MICROMIPS_CVT_S:int fmt
+*micromips32:
+*micromips64:
 {
   switch (fmt)
     {

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

[Mike Frysinger]

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

On 15 Jan 2016 16:22, Andrew Bennett wrote:
> > As a microMIPS engine is not included in a mips-linux-gnu build the file
> > supposed to provide `micromips_instruction_decode' is not generated or
> > compiled, whereas `delayslot_micromips' (and a bunch of other functions)
> > is built unconditionally.
> > 
> >  Restricting the affected functions to microMIPS processors only has fixed
> > the build problem for me, see the patch below.  I have not verified it
> > further though, will you be able to look into it soon?
> 
> Hi Maciej,
> 
> The patch looks good, I noticed that the FMT_MICROMIPS, FMT_MICROMIPS_CVT_D
> and FMT_MICROMIPS_CVT_S functions also needed fixing.  I have built it for the 
> mips-elf, mips-linux-gnu and mips-mti-elf targets and they all build successfully.
> The updated patch and ChangeLog is below.
> 
> Ok to commit?

looks fine to me.  note that this is PR sim/19441 (which is assigned
to you), so you should note that in the ChangeLog/commit message.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]