On 22.3.19. 19:04, Aleksandar Markovic wrote:
@@ -1511,9 +1518,11 @@ void helper_msa_insert_df(CPUMIPSState *env, uint32_t 
df, uint32_t wd,
       case DF_WORD:
           pwd->w[n] = (int32_t)rs;
           break;
+#ifdef TARGET_MIPS64
       case DF_DOUBLE:
           pwd->d[n] = (int64_t)rs;
           break;
+#endif
       default:
           assert(0);
       }
You are right that this case should be under ifdef the way you did.

In fact, this code should be impossible to reach, since there is a check
for MIPS32/64 in translate.c before invoking this helper, so technically
there is no bug. However, it is a latent bag, and also an instance of
"dead code" (for MIPS32). So, you are rightfully removing this case
for MIPS32.

May I just ask you to put this in a separate patch, since this has nothing
to do with endianess etc. (with the title, let's say:

"target/mips: Remove handling of nonexistent flavor of INSERT for MIPS32",

and the commit message

"INSERT.D is present in MIPS64 MSA only. [1] page <XXX>

[1] <insert here the latest MSA MIPS64 doc>"

)?
You are right, it has nothing to do with the endianness problem. I will
remove that in v2, and add another patch for that.

Mateja, I noticed that FILL.D has almost identical problem.

Could you create a separate patch for FILL.D too, dealing with the same
issue?

And also analyze all MIPS64-only MSA instructions in this regard?

Glancing at the MIPS64 MSA doc, they are:

COPY_S.D
COPY_U.W (this is not a typo, it is W, not D)
FILL.D
INSERT.D
Will do.

Yours,
Aleksandar
Regards,
Mateja
Thanks,
Aleksandar
Thanks,
Mateja

Reply via email to