From patchwork Thu Aug 27 15:05:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Bennett X-Patchwork-Id: 8482 X-Patchwork-Delegate: vapier@gentoo.org Received: (qmail 43275 invoked by alias); 27 Aug 2015 15:05:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 43260 invoked by uid 89); 27 Aug 2015 15:05:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS, ZIP_ATTACHED autolearn=no version=3.3.2 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Aug 2015 15:05:14 +0000 Received: from KLMAIL01.kl.imgtec.org (unknown [192.168.5.35]) by Websense Email Security Gateway with ESMTPS id 0D51DA560B41; Thu, 27 Aug 2015 16:05:06 +0100 (IST) Received: from LEMAIL01.le.imgtec.org (192.168.152.62) by KLMAIL01.kl.imgtec.org (192.168.5.35) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 27 Aug 2015 16:05:08 +0100 Received: from LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9]) by LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9%17]) with mapi id 14.03.0210.002; Thu, 27 Aug 2015 16:05:08 +0100 From: Andrew Bennett To: Mike Frysinger CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH] Add micromips support to the MIPS simulator Date: Thu, 27 Aug 2015 15:05:07 +0000 Message-ID: <0DA23CC379F5F945ACB41CF394B98277211129DE@LEMAIL01.le.imgtec.org> References: <0DA23CC379F5F945ACB41CF394B9827720F51185@LEMAIL01.le.imgtec.org> <20150224054441.GA6655@vapier> In-Reply-To: <20150224054441.GA6655@vapier> MIME-Version: 1.0 X-IsSubscribed: yes 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 // -// 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 // -// 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 . - 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