From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3592 invoked by alias); 17 Apr 2010 23:46:02 -0000 Received: (qmail 3577 invoked by uid 22791); 17 Apr 2010 23:46:01 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 17 Apr 2010 23:45:56 +0000 Received: (qmail 27614 invoked from network); 17 Apr 2010 23:45:55 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 17 Apr 2010 23:45:55 -0000 From: Pedro Alves To: "Pierre Muller" Subject: Re: [RFC-v2] Mingw Windows 64-bit gdbserver Date: Sat, 17 Apr 2010 23:46:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <000d01cadd79$efa9e2b0$cefda810$@muller@ics-cnrs.unistra.fr> <201004171158.08327.pedro@codesourcery.com> <003701cade84$38fbdd00$aaf39700$@muller@ics-cnrs.unistra.fr> In-Reply-To: <003701cade84$38fbdd00$aaf39700$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Message-Id: <201004180045.51495.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-04/txt/msg00538.txt.bz2 On Sunday 18 April 2010 00:17:59, Pierre Muller wrote: >=20 > > -----Message d'origine----- > > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches- > > owner@sourceware.org] De la part de Pedro Alves > > Envoy=E9 : Saturday, April 17, 2010 12:58 PM > > =C0 : Pierre Muller > > Cc : gdb-patches@sourceware.org > > Objet : Re: [RFC] Mingw Windows 64-bit gdbserver > >=20 > > On Saturday 17 April 2010 06:40:02, Pierre Muller wrote: > >=20 > > > > How about instead merging the files, like > > > > linux-x86-low.c handles both 64-bit and 32-bit? There's > > > > a lot of common stuff between both archs support, it > > > > seems. > > > > > > Of course, I agree with you that the two files > > > share a very large common portion that is identical. > > > There are only two places where they really differ: > > > For the call to the init_registers_XXX > > > and for the register mappings array. > > > > > > The main question is how should we split these parts > > > off if we want to keep a common part: > > > > > > I would propose this: > > > rename win32-i386-low.c to win-x86-low.c > >=20 > > Lets avoid someone reading this and getting religious > > against "win", and go with windows-*-low.c, just > > like gdb/windows-nat.c was renamed from win32-nat.c, and > > gdb has i386-windows-nat.c and amd64-windows-nat.c. > >=20 > > > Create win32-i386-low.h and win64-amd64-low.h > > > that would have the register mappings and > > > a macro to define their local init_registers. > >=20 > > Yes, much better, if nothing else because that's how > > gdb handles this as well. It's always good to have the > > code bases solve the same problem in the same way, so > > that we can more easily keep them in sync or merge them. > >=20 > > Take a look at gdb/amd64-windows-nat.c, it also does something > > similar to handle the common stuff, though since we have > > a win32_target_ops in gdbserver, we can put the register mappings > > array pointer directly in win32_target_ops instead of making it > > a global. > >=20 > > Let's avoid macros. Use for example the `arch_setup' callback > > in the win32_target_ops vector for this, keeping the arrays > > defined in the corresponding arch specific .c files. >=20 > Here is a new version of the patch: > I tried to minimize code duplication, but I > still needed one macro to decide which file > from win32-i386-low.c or win64-amd64-low.c=20 > should be read. Sorry, this is a no-no. Please reread what I said. Do not include ".c" files. If you want to take a route like that (have a define to select between 64-bit and 32-bit), might as well merge the i386 and amd64 bits all within the same file, just like I suggested from the beggining, and use `ifdef __x86_64__'. > The ChangeLog is a bit messy because I renamed a file > and reused the old name to create a new file... > but I didn't find another name that would fit better... > Unless I use windows- as a prefix everywhere... I think is would be simpler for both of us if you split out this patch into smaller pieces. Fix the type casts in win32-low.c, one piece. Renaming a file, and adjusting configure/makefile, another piece. Splitting out i?86 bits and amd64 windows bits to their own files, another piece (though I still wonder why not merge all of that into one file, but I admit that I no longer know what is best). This change: > -srv_amd64_regobj=3D"amd64.o x86-64-avx.o" > +srv_amd64_regobj=3D"amd64.o amd64-avx.o" looks like another piece. Is it just a cosmetic change because we use amd64 instead of x86-64 throughout? Please explain it and post it separately, if possible. Etc., etc. --=20 Pedro Alves