Message ID | 1405331763-57126-2-git-send-email-yongbok.kim@imgtec.com |
---|---|
State | New |
Headers | show |
Hi, On 14/07/14 10:55, Yongbok Kim wrote: > +union wr_t { > + int8_t b[MSA_WRLEN/8]; > + int16_t h[MSA_WRLEN/16]; > + int32_t w[MSA_WRLEN/32]; > + int64_t d[MSA_WRLEN/64]; This is incorrect on a big endian host. The least significant bits of the lowest indexed element should always alias. With a compiler for little endian this will work fine since b[0] will alias the least significant bits of h[0], w[0], and d[0], whereas with a compiler for big endian, b[0] will alias the upper byte of h[0], w[0], and d[0]. > diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h > index 9dfa516..11722bb 100644 > --- a/target-mips/mips-defs.h > +++ b/target-mips/mips-defs.h > @@ -41,6 +41,7 @@ > #define ASE_MT 0x00020000 > #define ASE_SMARTMIPS 0x00040000 > #define ASE_MICROMIPS 0x00080000 > +#define ASE_MSA 0x00100000 inconsistent whitespace... though maybe it was already incorrect. Cheers James
Hi, On 14/07/14 10:55, Yongbok Kim wrote: > +typedef struct CPUMIPSMSAContext CPUMIPSMSAContext; > +struct CPUMIPSMSAContext { > + int32_t msair; > + int32_t msacsr; > + int32_t msaaccess; > + int32_t msasave; > + int32_t msamodify; > + int32_t msarequest; > + int32_t msamap; > + int32_t msaunmap; > + > + float_status fp_status; > +}; > + > typedef union fpr_t fpr_t; > union fpr_t { > float64 fd; /* ieee double precision */ > float32 fs[2];/* ieee single precision */ > uint64_t d; /* binary double fixed-point */ > uint32_t w[2]; /* binary single fixed-point */ > +/* FPU/MSA register mapping is not tested on big-endian hosts. */ > + wr_t wr; /* vector data */ > }; > /* define FP_ENDIAN_IDX to access the same location > * in the fpr_t union regardless of the host endianness > @@ -175,6 +237,7 @@ typedef struct CPUMIPSState CPUMIPSState; > struct CPUMIPSState { > TCState active_tc; > CPUMIPSFPUContext active_fpu; > + CPUMIPSMSAContext active_msa; According to the manual, only the msair register is shared between thread contexts, each thread context has its own version of the rest of the msa registers, so most of this should be TCState I think. Cheers James
Hi, On 22/10/2014 12:35, James Hogan wrote: > +union wr_t { > + int8_t b[MSA_WRLEN/8]; > + int16_t h[MSA_WRLEN/16]; > + int32_t w[MSA_WRLEN/32]; > + int64_t d[MSA_WRLEN/64]; > This is incorrect on a big endian host. The least significant bits of > the lowest indexed element should always alias. > > With a compiler for little endian this will work fine since b[0] will > alias the least significant bits of h[0], w[0], and d[0], whereas with a > compiler for big endian, b[0] will alias the upper byte of h[0], w[0], > and d[0]. Yes it wouldn't work for a big endian host. However this MSA feature has been fully verified for big and little endian targets on a little endian host. I can see that the dsp_helper.c has similar problem as well. MSA could be forcibly turned off in a big endian host or might be leaved as it is just like DSP. If we need to implement for big endian host it would take longer time... What do you guys think about that? Regards, Yongbok
On 24/10/2014 10:35, Yongbok Kim wrote: > Hi, > > On 22/10/2014 12:35, James Hogan wrote: >> +union wr_t { >> + int8_t b[MSA_WRLEN/8]; >> + int16_t h[MSA_WRLEN/16]; >> + int32_t w[MSA_WRLEN/32]; >> + int64_t d[MSA_WRLEN/64]; >> This is incorrect on a big endian host. The least significant bits of >> the lowest indexed element should always alias. >> >> With a compiler for little endian this will work fine since b[0] will >> alias the least significant bits of h[0], w[0], and d[0], whereas with a >> compiler for big endian, b[0] will alias the upper byte of h[0], w[0], >> and d[0]. > > Yes it wouldn't work for a big endian host. > However this MSA feature has been fully verified for big and little > endian targets on a little endian host. > I can see that the dsp_helper.c has similar problem as well. > MSA could be forcibly turned off in a big endian host or might be leaved > as it is just like DSP. > If we need to implement for big endian host it would take longer time... > > What do you guys think about that? I don't think it would be reasonable to reject these patches only because of big-endian host limitation as these patches contain huge amount of work already (moreover, MSA is quite isolated). However, this version of patchset has other issues pointed out by me and James in previous emails - they need to be fixed / cleaned up. Thanks, Leon
diff --git a/target-mips/cpu.h b/target-mips/cpu.h index c81dfac..9a6b77c 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -51,12 +51,74 @@ struct CPUMIPSTLBContext { }; #endif +/* MSA Context */ + +#define MSA_WRLEN (128) + +typedef union wr_t wr_t; +union wr_t { + int8_t b[MSA_WRLEN/8]; + int16_t h[MSA_WRLEN/16]; + int32_t w[MSA_WRLEN/32]; + int64_t d[MSA_WRLEN/64]; +}; + +typedef struct CPUMIPSMSAContext CPUMIPSMSAContext; +struct CPUMIPSMSAContext { + +#define MSAIR_REGISTER 0 +#define MSACSR_REGISTER 1 +#define MSAACCESS_REGISTER 2 +#define MSASAVE_REGISTER 3 +#define MSAMODIFY_REGISTER 4 +#define MSAREQUEST_REGISTER 5 +#define MSAMAP_REGISTER 6 +#define MSAUNMAP_REGISTER 7 + + int32_t msair; + +#define MSAIR_WRP_POS 16 +#define MSAIR_WRP_BIT (1 << MSAIR_WRP_POS) + + int32_t msacsr; + +#define MSACSR_RM_POS 0 +#define MSACSR_RM_MASK (0x3 << MSACSR_RM_POS) + +#define MSACSR_CAUSE_ENABLE_FLAGS_POS 2 +#define MSACSR_CAUSE_ENABLE_FLAGS_MASK \ + (0xffff << MSACSR_CAUSE_ENABLE_FLAGS_POS) + +#define MSACSR_NX_POS 18 +#define MSACSR_NX_BIT (1 << MSACSR_NX_POS) + +#define MSACSR_FS_POS 24 +#define MSACSR_FS_BIT (1 << MSACSR_FS_POS) + +#define MSACSR_BITS \ + (MSACSR_RM_MASK | \ + MSACSR_CAUSE_ENABLE_FLAGS_MASK | \ + MSACSR_FS_BIT | \ + MSACSR_NX_BIT) + + int32_t msaaccess; + int32_t msasave; + int32_t msamodify; + int32_t msarequest; + int32_t msamap; + int32_t msaunmap; + + float_status fp_status; +}; + typedef union fpr_t fpr_t; union fpr_t { float64 fd; /* ieee double precision */ float32 fs[2];/* ieee single precision */ uint64_t d; /* binary double fixed-point */ uint32_t w[2]; /* binary single fixed-point */ +/* FPU/MSA register mapping is not tested on big-endian hosts. */ + wr_t wr; /* vector data */ }; /* define FP_ENDIAN_IDX to access the same location * in the fpr_t union regardless of the host endianness @@ -175,6 +237,7 @@ typedef struct CPUMIPSState CPUMIPSState; struct CPUMIPSState { TCState active_tc; CPUMIPSFPUContext active_fpu; + CPUMIPSMSAContext active_msa; uint32_t current_tc; uint32_t current_fpu; @@ -362,6 +425,7 @@ struct CPUMIPSState { #define CP0C2_SA 0 int32_t CP0_Config3; #define CP0C3_M 31 +#define CP0C3_MSAP 28 #define CP0C3_ISA_ON_EXC 16 #define CP0C3_ULRI 13 #define CP0C3_DSPP 10 @@ -431,7 +495,7 @@ struct CPUMIPSState { int error_code; uint32_t hflags; /* CPU State */ /* TMASK defines different execution modes */ -#define MIPS_HFLAG_TMASK 0x1807FF +#define MIPS_HFLAG_TMASK 0x5807FF #define MIPS_HFLAG_MODE 0x00007 /* execution modes */ /* The KSU flags must be the lowest bits in hflags. The flag order must be the same as defined for CP0 Status. This allows to use @@ -475,6 +539,7 @@ struct CPUMIPSState { #define MIPS_HFLAG_DSPR2 0x100000 /* Enable access to MIPS DSPR2 resources. */ /* Extra flag about HWREna register. */ #define MIPS_HFLAG_HWRENA_ULR 0x200000 /* ULR bit from HWREna is set. */ +#define MIPS_HFLAG_MSA 0x400000 target_ulong btarget; /* Jump / branch target */ target_ulong bcond; /* Branch condition (if needed) */ @@ -628,8 +693,10 @@ enum { EXCP_C2E, EXCP_CACHE, /* 32 */ EXCP_DSPDIS, + EXCP_MSADIS, + EXCP_MSAFPE, - EXCP_LAST = EXCP_DSPDIS, + EXCP_LAST = EXCP_MSAFPE, }; /* Dummy exception for conditional stores. */ #define EXCP_SC 0x100 @@ -726,7 +793,8 @@ static inline void compute_hflags(CPUMIPSState *env) { env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 | MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU | - MIPS_HFLAG_UX | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2); + MIPS_HFLAG_UX | MIPS_HFLAG_DSP | MIPS_HFLAG_DSPR2 | + MIPS_HFLAG_MSA); if (!(env->CP0_Status & (1 << CP0St_EXL)) && !(env->CP0_Status & (1 << CP0St_ERL)) && !(env->hflags & MIPS_HFLAG_DM)) { @@ -784,6 +852,11 @@ static inline void compute_hflags(CPUMIPSState *env) env->hflags |= MIPS_HFLAG_COP1X; } } + if (env->insn_flags & ASE_MSA) { + if (env->CP0_Config5 & (1 << CP0C5_MSAEn)) { + env->hflags |= MIPS_HFLAG_MSA; + } + } } #endif /* !defined (__MIPS_CPU_H__) */ diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h index 9dfa516..11722bb 100644 --- a/target-mips/mips-defs.h +++ b/target-mips/mips-defs.h @@ -41,6 +41,7 @@ #define ASE_MT 0x00020000 #define ASE_SMARTMIPS 0x00040000 #define ASE_MICROMIPS 0x00080000 +#define ASE_MSA 0x00100000 /* Chip specific instructions. */ #define INSN_LOONGSON2E 0x20000000 diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c index 27651a4..75f8af8 100644 --- a/target-mips/op_helper.c +++ b/target-mips/op_helper.c @@ -1512,6 +1512,7 @@ void helper_mtc0_config5(CPUMIPSState *env, target_ulong arg1) { env->CP0_Config5 = (env->CP0_Config5 & (~env->CP0_Config5_rw_bitmask)) | (arg1 & env->CP0_Config5_rw_bitmask); + compute_hflags(env); } void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1)
add defines and data structure for MIPS SIMD Architecture Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> --- target-mips/cpu.h | 79 +++++++++++++++++++++++++++++++++++++++++++++-- target-mips/mips-defs.h | 1 + target-mips/op_helper.c | 1 + 3 files changed, 78 insertions(+), 3 deletions(-)