> From: Aleksandar Markovic
> Sent: Tuesday, January 15, 2019 10:20 PM
> To: Fredrik Noring; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: Re: [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia
> registers
>
> > From: Fredrik Noring <nor...@nocrew.org>
> > Sent: Sunday, January 13, 2019 8:03 PM
> > To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> > Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> > Subject: [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia
> > registers
> >
> > The 32 R5900 128-bit MMRs are split into two 64-bit halves: the lower
> > halves are the GPRs and the upper halves are accessible by the R5900-
> > specific multimedia instructions.
> >
> > Signed-off-by: Fredrik Noring <nor...@nocrew.org>
> > ---
Since this patch is not a subject of any disagreement, and "#ifdef"
structure is of a cosmetic nature, I will do these minor cosmetic
changes while applying, and insert this patch in the next MIPS pull
request, scheduled shortly.
Reviewed-by: Aleksandar Markovic <amarko...@wavecomp.com>
Other patches from this series will not be in the pull request.
> > target/mips/cpu.h | 2 ++
> > target/mips/translate.c | 14 ++++++++++++--
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> > index 03c03fd8c6..9ff4a68d90 100644
> > --- a/target/mips/cpu.h
> > +++ b/target/mips/cpu.h
> > @@ -180,6 +180,8 @@ struct TCState {
> > #define MXU_CR_RD_EN 1
> > #define MXU_CR_MXU_EN 0
> >
> > + /* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs
> > */
> > + uint64_t mmr[32];
> > };
> >
> > typedef struct CPUMIPSState CPUMIPSState;
> > diff --git a/target/mips/translate.c b/target/mips/translate.c
> > index a538351032..9d5150ec8b 100644
> > --- a/target/mips/translate.c
> > +++ b/target/mips/translate.c
> > @@ -2455,7 +2455,10 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
> > static TCGv_i64 fpu_f64[32];
> > static TCGv_i64 msa_wr_d[64];
> >
> > -#if !defined(TARGET_MIPS64)
> > +#if defined(TARGET_MIPS64)
> > +/* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
> > +static TCGv_i64 cpu_mmr[32];
> > +#else
>
> Naming is perfect, but:
>
> #if-#else-#endif is confusing here:
> - MMR declaration should be under it own #if defined(TARGET_MIPS64)
> - MXU declaration should be under separate #if !defined(TARGET_MIPS64)
> (we are not dealing with two variants (64/32) of the same item, but with two
> separate items, one is valid for 64-bit only, another for 32-bit only)
>
> > /* MXU registers */
> > static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
> > static TCGv mxu_CR;
> > @@ -29785,7 +29788,14 @@ void mips_tcg_init(void)
> > fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
> > offsetof(CPUMIPSState,
> > active_fpu.fcr31),
> > "fcr31");
> > -#if !defined(TARGET_MIPS64)
> > +#if defined(TARGET_MIPS64)
> > + cpu_mmr[0] = NULL;
> > + for (i = 1; i < 32; i++)
> > + cpu_mmr[i] = tcg_global_mem_new_i64(cpu_env,
> > + offsetof(CPUMIPSState,
> > + active_tc.mmr[i]),
> > + regnames[i]);
> > +#else
> > for (i = 0; i < NUMBER_OF_MXU_REGISTERS - 1; i++) {
> > mxu_gpr[i] = tcg_global_mem_new(cpu_env,
> > offsetof(CPUMIPSState,
> > --
>
> The same applies here.
>
> > 2.19.2
>
>
