From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 580 invoked by alias); 7 Nov 2002 19:08:47 -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 573 invoked from network); 7 Nov 2002 19:08:44 -0000 Received: from unknown (HELO mms3.broadcom.com) (63.70.210.38) by sources.redhat.com with SMTP; 7 Nov 2002 19:08:44 -0000 Received: from 63.70.210.1mms3.broadcom.com with ESMTP (Broadcom MMS3 SMTP Relay (MMS v5.0)); Thu, 07 Nov 2002 11:08:37 -0800 X-Server-Uuid: 56E0BE8C-BF0B-4001-8E26-C8DC95F71F5F 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 LAA13254; Thu, 7 Nov 2002 11:08:33 -0800 (PST) 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 gA7J8YER017365; Thu, 7 Nov 2002 11:08:34 -0800 (PST) Received: (from cgd@localhost) by dt-sj3-118.sj.broadcom.com ( 8.9.1/SJ8.9.1) id LAA26503; Thu, 7 Nov 2002 11:08:30 -0800 (PST) To: "Andrew Cagney" cc: rsandifo@redhat.com, gdb-patches@sources.redhat.com Subject: Re: sim/mips patch: add support for more NEC VR targets References: <3DC9B7CF.8050401@redhat.com> <3DC9C49D.2090801@redhat.com> From: cgd@broadcom.com Date: Thu, 07 Nov 2002 11:08:00 -0000 In-Reply-To: "Andrew Cagney"'s message of "Wed, 06 Nov 2002 20:40:45 -0500" Message-ID: MIME-Version: 1.0 X-WSS-ID: 11D465BF193906-01-01 Content-Type: text/plain Content-Transfer-Encoding: 7bit X-SW-Source: 2002-11/txt/msg00183.txt.bz2 At Wed, 06 Nov 2002 20:40:45 -0500, Andrew Cagney wrote: > > well: > > > > (1) w/ separate fn, you'd need to tag _all_ of the 5500 machine insns > > w/ mips5500. (as opposed to ISA + a few mips5500 extensions.) > > Er, all the instructions that belonged to the MIPS 5500 should be taged > with 5500 anyway. As I noted: > > > The real world order is: ISA YYY implements MIPS XXX, BUT with a few tiny exceptions .... You end up having to check that every single *&@^#$(*&@#$ instruction matches the generic ISA. Sigh. So, i'd like to explore this a bit more. I'll state up front I don't agree, so you know where i'm going before you're hip-deep in spew. 8-) I hope this clears up where i'm trying to take things. I think you knew when you signed me on as co-maintainer that I was likely to have strong opinions. 8-) My belief is the that following things, at least, should be considered when trying to figure out how to support a new ISA, in no particular order: * clarity of code. E.g., if it's MIPS32 with a few small deviations or something, IMO it's _much_ clearer to code that using mips32 and then test for the machine to handle the diffferences, than it is to scatter machine_name tags all over and make people use compare_igen_models to tell the difference. Obviously, there's a tradeoff: if there are enough if() checks, then the code quality suffers. * performance. Being able to tune a simulator so that it performs well is important to me, and to serious simulator users. Like i've said before, we've simulated ... a whole heck of a lot of instructions (i believe probably "trillions") using a sim-based simulator, and we prefer that things run as fast as possible. 8-) Because there's a desire to support multi-machine simulators, there's a tradeoff here, too. To my mind, people who want multi-machine simulators are willing to sacrifice some performance to do it (but do not want to lose horribly, either). A few scattered run-time checks will not significantly harm performance (and heck, they've already bought a runtime check in sim_engine_run at minimum). On the other hand, the ability to provide multiple machine support should minimally impact single machine support. Once consequence of that is, when building a single machine sim, as many run-time tests as possible should be able to be removed at compile time (e.g. due to constant comparison results). * correctness. By this i mean "matches the actual hardware," and not "produces computationally-correct results given what the author of the code was trying to simulatr." The latter is not something which is negotiable. 8-) Obviously, the simulator has to be "as close as necessary" for the author's and the community's use. There are differing degrees as to what's required there. For instace, our internal simulator has a full and complete cp0 implementation, good enough to run OSes and other "hard" tests of the MIPS architecture. Many people don't need that, or in fact don't even _want_ it. For instance, our CP1 simulation actually does trap on the same inputs that our real FPU does (denorms for certain ops, or whatever -- i forget) and it's important to us that it does (when running in "raw hardware" mode). That's desirable in certain circumstances, but not in many (e.g. running the GCC testsuite on simulated firmware), since it'll cause some tests to fail that would normally pass after OS or firmware fixups. So, anyway, to get on to the specifics of how to add new machines, with the choices being: * leave 'shared' instructions tagged w/ the base ISA type implemented by the machine (with additional tags for ASEs, etc.) and use run-time tests to differentiate as necessary in 'common' instructions * tag each instruction as being specific to that machine, differentiating by providing different functions or by instructions. * In the case of MIPS32/MIPS64 implementations, i strongly believe that the former ("the new world order" as i see it) is the right thing. there are three basic types of deviations which require conditionalization: * cp0 stuff. IMO, this pretty much _has_ to be done by checking the machine type in C code. If you try to do it in igen code, you need IGEN functions per machine type, then they need to call out to per-machine-type functions in .c files. It unnecessarily blows up the size of the code. This is especially true for MIPS32/MIPS64 CP0, which _should_ be CP0 as specified, with possibly a tiny bit of deviation per machine either for allowed customizations (e.g. VA size, subsetting, etc.) or for defects. Trying to do M32/M64 CP0 on a per-machine basis would just be crazy. * added instructions There are easily enough handled as "ASE"-ish additional machine type specifiers. * bugs. This should, IMO, be handled on a case-by-case basis. If a machine has a few of them, they should probably be handled with if()s on the machine type. If it's got a _lot_, they should probably be handled by not using the generic mips32/mips64 instructions, and littering the new machine's name all over as need be. * Pre-MIPS32/MIPS64 is a bit trickier, because in addition to the above cases where people are implementing on top of a standard ISA, you run into stuff like: * vastly diverent CP0 implementations. * instructions _removed_ from a base ISA (e.g. no ll/sc), or instructions with same encoding but different behaviour. * divergent requirements re: hazards (i.e., load to use, LO/HI, etc.) that it's desirable to have the simulator warn about (so as to be sure the compiler is avoiding them adequately). I believe both should be evaluated on a case-by-case basis, the latter obviously moreso than the former. (Odds are, if a MIPS CPU has _that_ many bugs, it's not gonna pass conformance testing or ship in any significant volume. IMO, variations among pre-MIPS32/MIPS64 chips are likely to be greater than post-, but thankfully going forward we should see fewer of them being added to the simulator!) anyway, that having been said, I think i should now state my opinion re: evaluating this particular case, i.e. Richard's patch to add vr5500 support (in particular the conditional in check_mf_cycles) as modified by my subsequent patch: * the check in check_mf_cycles will evaluate to a constant true/false if not a multi-machine simulator, * the vr5500 run-time conditionals are not many in number (uh, 1 8-), and therefore aren't intrusive in that manner (i.e., they don't make the code harder to read), * based on the above 2 points, that runtime check will not hurt code performance in the case where we care most about performance (single-machine), and since it's just one check shouldn't hurt much at all even if multi-machine. You've suggested the alternative of changing check_mf_cycles to be a machine-dependent function. I do believe (as you've stated) that _that_ would require adding vr5500 tags scattered on every instruction used by the vr5500, i.e., every mipsIV instruction, with the _only_ difference between vr5500 and mipsIV (over the set of insn/fns used by mipsIV) being check_mf_cycles. IMO, that would: * have no significant performance benefit, but * make the patch to add the vr5500 support harder to understand (i.e. cluttered with Lots of lines of diffs to just add vr5500 tags), and finally and most importantly: * hurt the maintainability of the code. Instead of vr5500 differences being clearly marked as with Richard's code, one would _have_ to use e.g. compare_igen_models to tell the difference, and even that would say only that each had different check_mf_cycles fns (i.e., not make the difference clear). (I also think that doing it the way you suggest would set the wrong precedent for future additions, which if MIPS32/MIPS64 based really almost certainly should be done w/ conditional checks.) In other words, IMO, doing it the way Richard has done it is basically all gain, no pain, and that compared to that tagging every mipsIV insn w/ vr5500 is much pain, no gain. 8-) chris