From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37917) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SAtU9-00044T-Vr for qemu-devel@nongnu.org; Thu, 22 Mar 2012 21:40:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SAtU8-0000ny-4x for qemu-devel@nongnu.org; Thu, 22 Mar 2012 21:40:09 -0400 Received: from mail-lpp01m010-f45.google.com ([209.85.215.45]:56220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SAtU7-0000mv-PP for qemu-devel@nongnu.org; Thu, 22 Mar 2012 21:40:08 -0400 Received: by lahe6 with SMTP id e6so2185025lah.4 for ; Thu, 22 Mar 2012 18:40:05 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1332423372-2391-1-git-send-email-proljc@gmail.com> Date: Fri, 23 Mar 2012 09:40:05 +0800 Message-ID: From: Jia Liu Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2 0/4] MIPS ASE DSP Support for Qemu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: sw@weilnetz.de, qemu-devel@nongnu.org, aurelien@aurel32.net On Thu, Mar 22, 2012 at 10:01 PM, Peter Maydell wrote: > On 22 March 2012 13:36, Jia Liu wrote: >> Jia Liu (4): >> =A0add MIPS DSP helpers define >> =A0add MIPS DSP helpers implement >> =A0add MIPS DSP translation >> =A0add MIPS DSP testcase > > You can't split these changes into patches like this. > Every patch needs to be complete of itself, and in particular > it has to compile cleanly at each intermediate point, not just > after all patches are applied. If you apply just patch 1/4 > then compilation fails: > =A0LINK =A0mips-softmmu/qemu-system-mips > translate.o: In function `mips_tcg_init': > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/helper.h:301: > undefined reference to `helper_absqsph' > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/helper.h:302: > undefined reference to `helper_absqsw' > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/helper.h:303: > undefined reference to `helper_addqph' > [snipped long list of similar errors] > > It would be better to break it up as patches each of > which adds support for a coherent bite-sized subset of > these instructions (so each individual patch includes > the helper function declaration, implementation and > translate.c changes for a smaller number of instructions). > Thank you. So I should split these changes into 2 patches, 0001 DSP Support, 0002 testcases. Is it OK? > You also need to test compilation on a 32 bit host, as that > is currently broken. For example: > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c: > In function =91mipsdsp_sat_add_i32=92: > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:3565: > error: integer constant is too large for =91long=92 type > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c: > In function =91mipsdsp_mul_q31_q31=92: > /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:3874: > error: integer constant is too large for =91long=92 type > [and on for another long list of errors] > > Most of these seem to be that you need the "ULL" suffix for > constants which are more than 32 bits wide. > Sorry, I'm working on a x64 machine, but I'll test it at a i386 machine. Thank you every much! > -- PMM Regards, Jia.