From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4902 invoked by alias); 27 Nov 2002 00:55:33 -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 4887 invoked from network); 27 Nov 2002 00:55:31 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 27 Nov 2002 00:55:31 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 9E7B03F30; Tue, 26 Nov 2002 19:55:20 -0500 (EST) Message-ID: <3DE417F8.8030209@redhat.com> Date: Tue, 26 Nov 2002 16:55:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020824 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Richard Sandiford , cgd@broadcom.com Cc: gdb-patches@sources.redhat.com Subject: Re: sim/mips patch: add support for more NEC VR targets References: Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-11/txt/msg00668.txt.bz2 > :function:::int:check_mf_cycles:hilo_history *history, signed64 time, const char *new > { > + /* There are no timing requirements in vr5500 code. */ > + if (MIPS_MACH (SD) == bfd_mach_mips5500) > + return 1; > if (history->mf.timestamp + 3 > time) > { > sim_engine_abort (SD, CPU, CIA, "HILO: %s: %s at 0x%08lx too close to MF at 0x%08lx\n", Just this, Add: function:::int:check_mf_cycles:hilo_history *history, signed64 time, const char *new *vr5500 { return 1; } and then tag the old function with the other CPU variants. that way if (MIPS_MACH (SD) == bfd_mach_mips5500) is not needed and the compiler can (if configured with sufficient inlining) eliminate the entire function call. This post: http://sources.redhat.com/ml/gdb-patches/2002-11/msg00512.html > We have had very bad experiences with trying to make a single function > serve two different ABI's in the past. (mips_push_arguments seems to > have been cleaned up since I last looked; it was a real mess.) So > while using things like 'REGISTER_SIZE' and > 'S390_STACK_PARAMETER_ALIGNMENT' are clearly a good idea, for the sake > of the other stuff I'd like to see a separate 's390x_push_arguments' > function written that does things right for the s390x's ABI. The > helper functions like `is_simple_arg' should be duplicated, rather > than testing GDB_TARGET_IS_ESAME. Gives the thrust of the rationale. Namely, its better to, from the start, have separate independant functions and not confuse things by adding more and more if(ISA/ABI) specific gunk. As JimB noted, the MIPS is the not so shining example of how to do things (i.e., how things can go wrong). For the sim, the original gencode tried to be smart and combine isa variants. Given the age of the code, the number of CPU variants, and the number of developers making changes (the two are comparable and both are large!), it ended to end up with a total mess. Someone trying to add one ISA would [always, I'm pretty sure it was] broke the other ISAs (only discovered months later) by either also adding instructions to other ISAs or, worse, removing/changing existing instructions. Ulgh! IGEN and MIPS.IGEN gave up. Instead, the developer gets to spend a few minutes adding an ISA selector to every relevant instruction. That's in the noise when compared to the amount of time that needs to be spent auding the instruction set looking for where the vendor has an instruction variant that deviates from the official spec (how many mul/adds are there?). In the case of a variant, the instruction can cloned, taged and changed with the absolute certaintity that it won't break any other instruction, because none of the other instructions / ISAs are affected. So, while this might sux from an asthetic point of view, it definitly doesn't sux from the point of view of being able to confidently, quickly and reliably update the simulator. (cgd, does this answer your question?) Andrew