From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15160 invoked by alias); 22 Oct 2002 22:27:24 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15000 invoked from network); 22 Oct 2002 22:27:22 -0000 Received: from unknown (HELO mms2.broadcom.com) (63.70.210.59) by sources.redhat.com with SMTP; 22 Oct 2002 22:27:22 -0000 Received: from 63.70.210.1 by mms2.broadcom.com with ESMTP (Broadcom MMS2 SMTP Relay (MMS v5.0)); Tue, 22 Oct 2002 15:24:56 -0700 X-Server-Uuid: 59F48136-7074-4F4B-B709-D7F3B6466DB0 Received: from mail-sj1-5.sj.broadcom.com (mail-sj1-5.sj.broadcom.com [10.16.128.236]) by mon-irva-11.broadcom.com (8.9.1/8.9.1) with ESMTP id PAA28120; Tue, 22 Oct 2002 15:27:18 -0700 (PDT) Received: from dt-sj3-118.sj.broadcom.com (dt-sj3-118 [10.21.64.118]) by mail-sj1-5.sj.broadcom.com (8.12.4/8.12.4/SSF) with ESMTP id g9MMRHER020368; Tue, 22 Oct 2002 15:27:17 -0700 (PDT) Received: (from cgd@localhost) by dt-sj3-118.sj.broadcom.com ( 8.9.1/SJ8.9.1) id PAA23176; Tue, 22 Oct 2002 15:27:14 -0700 (PDT) To: "Richard Sandiford" cc: gdb-patches@sources.redhat.com Subject: Re: sim/mips patch: add support for more NEC VR targets References: From: cgd@broadcom.com Date: Tue, 22 Oct 2002 15:27:00 -0000 In-Reply-To: "Richard Sandiford"'s message of "09 Oct 2002 15:14:43 +0100" Message-ID: MIME-Version: 1.0 X-WSS-ID: 11AB0FB2330447-01-01 Content-Type: text/plain Content-Transfer-Encoding: 7bit X-SW-Source: 2002-10/txt/msg00439.txt.bz2 At 09 Oct 2002 15:14:43 +0100, Richard Sandiford wrote: > Thanks for the review. That's what I'm here for. 8-) (Sorry for taking so long to get to this, but, as i mentioned in private mail before I left, I was out of the office. 8-) > Good catch. I checked that vr5000 implied mipsIV, but not the > other way around (duh!). (I dunno whether you were using the horrible compare_igen_models script or not, but it does both ways for you and is ... much easier than hand inspection. 8-) > Yeah. I don't understand why, really, since the vr41xx processors don't > have an FPU. Maybe the target environment was assumed to provide an > emulator or something? But key instructions (like bc1f and bc1t ;-) > are missing, so very little FP code is going to work. I don't know, the code predates my time here. 8-) FWIW, my personal preference is to have a simulator which is good enough to act like real hardware, then, if necessary and appropriate (i.e., trying to simulate a firmware or other SW environment which has an emulator) do actual emulation. (One implication of this is that a 'good' emulation for a particular processor might well trap on certain FP cases unimplemented in HW.) There are both plusses and minuses to this approach, of course. > > There are a couple of specific things that would be nice if you could > > arrange in follow-on patches, in addition to what you describe above: > > > > * it would be good if things like vr_run.c could be automatically > > generated. > > OK. Guilt at my earlier lameness forced me to give this a go. > New version attached. Not pretty (and I didn't use awk ;-) but > it should reduce the amount of cut&paste if other targets use > MULTI. I've also removed the need to define sim_multi_filter > and sim_multi_machine, since igen can cope with the same machine > & filter being given twice. Heh. (no awk?! what will we do?!) > > There is an annoying interaction between multi-arching a sim and using > > STATE_ARCHITECTURE (SD)->mach: e.g. if you have the thing above where > > you check bfd_mach_mips5500, if mips5500 is the 'default' for your sim > > (i.e., catches unknown machine types), that check will fail. > > > > Really, we need to be sure to check and, if necessary, set 'mach' to a > > default value when it is set. > > Where do you think this should happen? In sim_engine_run()? To be honest, I'm not really sure. However, I do believe that that is too late. 8-) (The problem is that simulator machine state initialization has happened by the time you invoke sim_engine_run(), and that may have to differe depending on the type of machine you're simulating.) Ideally what you want is a replacement for STATE_ARCHITECTURE (sd) ? STATE_ARCHITECTURE (sd)->mach : 0 that does the correct default handling, but is very very fast. Alas, I don't know that that's possible. 8-) Probably the right thing is to just turn it into a function call if doing MULTI, else have it #defined to the correct specific value if not MULTI (i.e., a single-arch sim). People running the GCC test suite in a MULTI sim don't care if they're getting 200k insns/sec or 1m insns/sec... 8-) On the other hand, people who want a really fast sim will build one that targets a single CPU, which will get the #define which also allows additional optimizations (code to be elided). (Having been in the position of booting firmware under a sim, then booting OSes under said firmware, I can say that I like the fastest sim possible sometimes. 8-) Thoughts? > + # The MULTI generator can combine several simulation engines into > + # one executable. A configuration which uses the MULTI should > + # set ${sim_multi_arch_configs} to the list of engines to build. > + # Each space-separated entry has the form NAME:MACHINE:FILTER@ARCHS, > + # where: > + # > + # - NAME is a C-compatible prefix for the engine, > + # - MACHINE is a -M argument, > + # - FILTER is a -F argument, and > + # - ARCHS is a comma-separated list of bfd machines that the > + # simulator can run. > + # > + # Each entry will have a separate simulation engine whose prefix is > + # m32. If the machine list includes "mips16", there will also > + # be a mips16 engine, prefix m16. The mips16 engine will be > + # generated using the same machine list as the 32-bit version, > + # but the filter will be "16" instead of FILTER. > + # > + # The simulator compares the bfd mach against ARCHS to decide > + # which engine to use. Entries in ARCHS can be a bfd_mach_mips* > + # value with "bfd_mach_mips" removed, or the special string > + # "default" to indicate that a particular engine should be > + # the default. I believe there _must_ be one default entry, and this should be checked for in the code too. What do you think? Why use "@" rather than another :... makes the seds slightly easier, that it? Suggest either: * archs have only bfd_mach_ prepended i.e., mips4111, mips_sb1, etc. The current method is ... not so pretty for archs whose bfd define isn't a number. This makes the specifictions a bit longer, but IMO that's not biggie. * more special casing, e.g.: [0-9]* add bfd_mach_mips [a-z]* add bfd_mach_mips_ 8-) This could get out of hand, though. (personally, my feeling is that adding even more defines of the form bfd_mach_mipsNNNN is ... not the right thing. It should be bfd_mach_mips_vendnum ^^^^^^^ vendor letter/number combo, e.g. sb1, vrNNNN ^^^^ ARCHITECTURE!!! or whatever... but that's water under the bridge.) > + if test ${sim_gen} = MULTI; then > + rm -f multi-include.h multi-switch.c > + sim_multi_flags= > + sim_multi_obj=multi_run.o > + for fc in ${sim_multi_arch_configs}; do > + c=`echo ${fc} | sed 's/@.*//'` > + archs=`echo ${fc} | sed 's/.*@//'` > + name=`echo ${c} | sed 's/:.*//'` > + m=`echo ${c} | sed 's/.*:\(.*\):.*/\1/'` > + f=`echo ${c} | sed 's/.*://'` > + sim_multi_flags="${sim_multi_flags} -F ${f} -M ${m}" > + case $c in > + *:*mips16*:*) > + ws="m32 m16" > + sim_multi_src="${sim_multi_src} m16${name}_run.c" > + sim_multi_obj="${sim_multi_obj} m16${name}_run.o" > + sim_multi_flags="${sim_multi_flags} -F 16" > + ;; > + *) > + ws=m32 > + ;; > + esac > + for w in ${ws}; do > + for base in engine icache idecode model semantics support; do > + sim_multi_src="${sim_multi_src} ${w}${name}_${base}.c" > + sim_multi_src="${sim_multi_src} ${w}${name}_${base}.h" > + sim_multi_obj="${sim_multi_obj} ${w}${name}_${base}.o" > + done > + sim_multi_configs="${sim_multi_configs} ${w}${c}" > + done > + echo "#include \"${w}${name}_engine.h\"" >> multi-include.h > + for a in `echo $archs | sed 's/,/ /g'`; do > + case $a in > + default) echo "default:" >> multi-switch.c ;; > + *) echo "case bfd_mach_mips$a:" >> multi-switch.c ;; > + esac > + done > + echo " ${w}${name}_engine_run (sd, next_cpu_nr, nr_cpus, signal);" \ > + >> multi-switch.c > + echo " break;" >> multi-switch.c > + done > + else > + # For clean-extra > + sim_multi_src=doesnt-exist.c > + fi > sim_igen_flags="-F ${sim_igen_filter} ${sim_igen_machine} ${sim_igen_smp}" > sim_m16_flags=" -F ${sim_m16_filter} ${sim_m16_machine} ${sim_igen_smp}" > AC_SUBST(sim_igen_flags) > AC_SUBST(sim_m16_flags) > AC_SUBST(sim_gen) > + AC_SUBST(sim_multi_flags) > + AC_SUBST(sim_multi_configs) > + AC_SUBST(sim_multi_src) > + AC_SUBST(sim_multi_obj) err, shouldn't multi-switch.c be cleaned somehow (during distclean, i guess, since it's config-generated). Also, shouldn't multi-run.o depend on multi-switch.c so that the right thing happens after re-config? (obviously, if there's a fn to pick the right machine type, the case statement in the engine generated above doesn't need to include the default, that fn does.) chris