> From: Mateja Marjanovic <mateja.marjano...@rt-rk.com> > Subject: [PATCH 4/4] target/mips: Fix insert.<b|h|w> for MIPS big endian host > > From: Mateja Marjanovic <mateja.marjano...@rt-rk.com> > > Inserting from GPR to an element in a MSA register when > executed on a MIPS big endian CPU, didn't pick the > right element, and was behaving like on little endian. > > Signed-off-by: Mateja Marjanovic <mateja.marjano...@rt-rk.com> > ---
... > @@ -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>" )? Thanks, Aleksandar
