Re: [Qemu-devel] [PATCH V2 0/4] MIPS ASE DSP Support for Qemu

[Jia Liu]

On Thu, Mar 22, 2012 at 10:01 PM, Peter Maydell
<address@hidden> wrote:
> On 22 March 2012 13:36, Jia Liu <address@hidden> wrote:
>> Jia Liu (4):
>>  add MIPS DSP helpers define
>>  add MIPS DSP helpers implement
>>  add MIPS DSP translation
>>  add 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:
>  LINK  mips-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 ‘mipsdsp_sat_add_i32’:
> /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:3565:
> error: integer constant is too large for ‘long’ type
> /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:
> In function ‘mipsdsp_mul_q31_q31’:
> /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:3874:
> error: integer constant is too large for ‘long’ 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.