On 20 January 2011 17:16, Christophe Lyon <christophe.l...@st.com> wrote:
> Set the right overflow bit for neon 32 and 64 bit saturating add/sub.
>
> Also move the neon 64 bit saturating add/sub helpers to neon_helper.c
> for consistency with the 32 bits versions.
>
> There is probably still room for code commonalization though.
>
> Peter, this patch is based upon your patch 6f83e7d and adds the 64 bits case.
I've reviewed this patch and tested it in the usual way
and can confirm that it now sets the right saturation bit;
mostly it is OK. However...
> @@ -4233,16 +4227,20 @@ static int disas_neon_data_insn(CPUState * env,
> DisasContext *s, uint32_t insn)
> switch (op) {
> case 1: /* VQADD */
> if (u) {
> - gen_helper_neon_add_saturate_u64(CPU_V001);
> + gen_helper_neon_qadd_u64(cpu_V0, cpu_env,
> + cpu_V0, cpu_V1);
> } else {
> - gen_helper_neon_add_saturate_s64(CPU_V001);
> + gen_helper_neon_qadd_s64(cpu_V0, cpu_env,
> + cpu_V0, cpu_V1);
> }
> break;
> case 5: /* VQSUB */
> if (u) {
> - gen_helper_neon_sub_saturate_u64(CPU_V001);
> + gen_helper_neon_qsub_u64(cpu_V0, cpu_env,
> + cpu_V0, cpu_V1);
> } else {
> - gen_helper_neon_sub_saturate_s64(CPU_V001);
> + gen_helper_neon_qsub_s64(cpu_V0, cpu_env,
> + cpu_V0, cpu_V1);
> }
> break;
> case 8: /* VSHL */
the indentation in this hunk is wrong -- qemu standard is four-space.
You can check for this sort of thing by running scripts/checkpatch.pl,
which (as well as a pile of false positives because it's got confused
by the HELPER() macro) says:
WARNING: suspect code indent for conditional statements (20, 22)
#264: FILE: target-arm/translate.c:4229:
if (u) {
+ gen_helper_neon_qadd_u64(cpu_V0, cpu_env,
WARNING: suspect code indent for conditional statements (20, 22)
#275: FILE: target-arm/translate.c:4238:
if (u) {
+ gen_helper_neon_qsub_u64(cpu_V0, cpu_env,
-- PMM
