On 3/20/19 7:16 AM, Yoshinori Sato wrote:
> +void rx_cpu_unpack_psw(CPURXState *env, int all)
> +{
> + if (env->psw_pm == 0) {
> + env->psw_ipl = extract32(env->psw, PSW_IPL, 4);
> + if (all) {
> + env->psw_pm = extract32(env->psw, PSW_PM, 1);
> + }
> + env->psw_u = extract32(env->psw, PSW_U, 1);
> + env->psw_i = extract32(env->psw, PSW_I, 1);
> + }
> + env->psw_o = extract32(env->psw, PSW_O, 1) << 31;
> + env->psw_s = extract32(env->psw, PSW_S, 1) << 31;
> + env->psw_z = extract32(env->psw, PSW_Z, 1) ? 0 : 1;
> + env->psw_c = extract32(env->psw, PSW_C, 1);
> +}
I think this function should take the psw value to extract as an argument.
Otherwise, if psw_pm == 1, you're leaving garbage values in env->psw, and I
think that can be confusing.
In fact, since pack_psw does not read env->psw at all, I would remove that
variable.
> +uint32_t helper_mvfc(CPURXState *env, uint32_t cr)
> +{
> + switch (cr) {
> + case 0:
> + return pack_psw(env);
> + case 2:
> + return env->psw_u ? env->regs[0] : env->usp;
> + case 3:
> + return env->fpsw;
> + case 8:
> + return env->bpsw;
> + case 9:
> + return env->bpc;
> + case 10:
> + return env->psw_u ? env->isp : env->regs[0];
> + case 11:
> + return env->fintv;
> + case 12:
> + return env->intb;
> + default:
> + g_assert_not_reached();
> + return -1;
This should only assert if it is caught in translate.c and rejected as an
invalid instruction. I don't see that happening though.
I think the best option is to move all of this into translate.c. You have
defined tcg temporaries for (almost) all of these, although many are currently
unused. It's easy to handle any without tcg temporaries with
tcg_gen_ld_i32(tmp, cpu_env, offsetof(CPURXState, field));
At which point your default case can simply return false to signal an illegal
instruction.
I now see that PC is a control register number 1, not handled here, and that
"MVFC PC, rN" should use ctx->pc.
> +void helper_mvtc(CPURXState *env, uint32_t cr, uint32_t val)
> +{
> + switch (cr) {
...
> + default:
> + g_assert_not_reached();
> + }
> +}
Likewise.
> +void helper_unpack_psw(CPURXState *env)
> +{
> + uint32_t prev_u;
> + prev_u = env->psw_u;
> + rx_cpu_unpack_psw(env, 1);
> + if (prev_u != env->psw_u) {
> + if (env->psw_u) {
> + env->isp = env->regs[0];
> + env->regs[0] = env->usp;
> + } else {
> + env->usp = env->regs[0];
> + env->regs[0] = env->isp;
> + }
> + }
> +}
Again, I think this should take the full psw as an argument.
> +/* fp operations */
> +static void update_fpsw(CPURXState *env, float32 ret, uintptr_t retaddr)
> +{
> + int xcpt, cause, enable;
> +
> + env->psw_z = (*((uint32_t *)&ret) == 0); \
> + env->psw_s = (*((uint32_t *)&ret) >= 0x80000000UL); \
(1) You do not need this casting, float32 == uint32_t,
(2) The result is wrong for -0.0.
env->psw_s = ret; /* sign in bit 31 */
env->psw_z = ret & 0x7fffffff; /* remove sign from -0.0 */
> + xcpt = get_float_exception_flags(&env->fp_status);
> +
> + /* Clear the cause entries */
> + env->fpsw = deposit32(env->fpsw, FPSW_CAUSE, 5, 0);
> +
> + if (unlikely(xcpt)) {
> + if (xcpt & float_flag_invalid) {
> + env->fpsw = deposit32(env->fpsw, FPSW_CAUSE_V, 1, 1);
> + env->fpsw = deposit32(env->fpsw, FPSW_FLAG_V, 1, 1);
> + }
> + if (xcpt & float_flag_divbyzero) {
> + env->fpsw = deposit32(env->fpsw, FPSW_CAUSE_Z, 1, 1);
> + env->fpsw = deposit32(env->fpsw, FPSW_FLAG_Z, 1, 1);
> + }
> + if (xcpt & float_flag_overflow) {
> + env->fpsw = deposit32(env->fpsw, FPSW_CAUSE_O, 1, 1);
> + env->fpsw = deposit32(env->fpsw, FPSW_FLAG_O, 1, 1);
> + }
> + if (xcpt & float_flag_underflow) {
> + env->fpsw = deposit32(env->fpsw, FPSW_CAUSE_U, 1, 1);
> + env->fpsw = deposit32(env->fpsw, FPSW_FLAG_U, 1, 1);
> + }
> + if (xcpt & float_flag_inexact) {
> + env->fpsw = deposit32(env->fpsw, FPSW_CAUSE_X, 1, 1);
> + env->fpsw = deposit32(env->fpsw, FPSW_FLAG_X, 1, 1);
> + }
What I don't see here is the "unimplemented processing" exception that you get
for denormal inputs or outputs, and DN = 0.
There is support for this in softfloat, although it looks funny.
You must always have
set_flush_to_zero(1, &env->fp_status);
set_flush_inputs_to_zero(1, &env->fp_status);
Do this during reset, I think.
This enables within softfloat the float_flag_{input,output}_denormal bits.
Here in update_fpsw you would do
if ((xcpt & (float_flag_input_denormal
| float_flag_output_denormal))
&& !(env->fpsw & FPSW_DN)) {
env->fpsw = deposit32(env->fpsw, FPSW_CAUSE_E, 1, 1);
}
> + /* Generate an exception if enabled */
> + cause = extract32(env->fpsw, FPSW_CAUSE, 5);
> + enable = extract32(env->fpsw, FPSW_ENABLE, 5);
/* Cause E, unimplemented processing, cannot be disabled. */
enable |= (1 << 5);
> + if (cause & enable) {
> + raise_exception(env, 21, retaddr);
> + }
> + }
> +}
> +
> +static void set_fpmode(CPURXState *env, uint32_t val)
> +{
> + static const int roundmode[] = {
> + float_round_nearest_even,
> + float_round_to_zero,
> + float_round_up,
> + float_round_down,
> + };
> + env->fpsw = val & FPSW_MASK;
> + set_float_rounding_mode(roundmode[val & FPSW_RM_MASK],
> + &env->fp_status);
> + set_flush_to_zero((val & FPSW_DN) != 0, &env->fp_status);
As discussed above, do not set_flush_to_zero here.
> +#define FLOATOP(op, func) \
> +float32 helper_##op(CPURXState *env, float32 t0, float32 t1) \
> +{ \
> + float32 ret; \
> + ret = func(t0, t1, &env->fp_status); \
> + update_fpsw(env, *(uint32_t *)&ret, GETPC()); \
No casting required.
> +void helper_fcmp(CPURXState *env, float32 t0, float32 t1)
> +{
> + int st;
> + st = float32_compare(t0, t1, &env->fp_status);
> + update_fpsw(env, 0, GETPC());
> + env->psw_z = env->psw_s = env->psw_o = 0;
env->psw_z = 1, because Z == !psw_z.
> + switch (st) {
> + case float_relation_equal:
> + env->psw_z = 0;
> + break;
> + case float_relation_less:
> + env->psw_s = -1;
> + break;
> + case float_relation_unordered:
> + env->psw_o = 1 << 31;
-1 works here too.
> +void helper_sstr(CPURXState *env, uint32_t sz)
> +{
> + while (env->regs[3] != 0) {
> + switch (sz) {
> + case 0:
> + cpu_stb_data_ra(env, env->regs[1], env->regs[2], GETPC());
> + break;
> + case 1:
> + cpu_stw_data_ra(env, env->regs[1], env->regs[2], GETPC());
> + break;
> + case 2:
> + cpu_stl_data_ra(env, env->regs[1], env->regs[2], GETPC());
> + break;
> + }
> + env->regs[1] += (1 << sz);
> + env->regs[3]--;
> + }
This should probably be split into 3 functions, so that the switch does not
have to be re-evaluated within the loop.
> +static void rx_search(uint32_t mode, int sz, CPURXState *env)
> +{
> + uint32_t tmp;
> +
> + while (env->regs[3] != 0) {
> + tmp = ld[sz](env, env->regs[1], env->pc);
> + env->regs[1] += 1 << (mode % 4);
This increment doesn't look right for SWHILE.
> + env->regs[3]--;
> + if ((mode == OP_SWHILE && tmp != env->regs[2]) ||
> + (mode == OP_SUNTIL && tmp == env->regs[2])) {
> + break;
> + }
> + }
> + env->psw_z = (mode == OP_SUNTIL) ?
> + (tmp - env->regs[2]) : env->regs[3];
> + env->psw_c = (tmp <= env->regs[2]);
> +}
I think this whole function would be clearer split between SWHILE and SUNTIL.
You're missing the initial R3 == 0 check (if executed with R3 == 0, no effect
on registers or flags).
> +/* accumlator operations */
> +void helper_rmpa(CPURXState *env, uint32_t sz)
> +{
> + uint64_t result_l, prev;
> + int32_t result_h;
> + int64_t tmp0, tmp1;
> +
> + if (env->regs[3] == 0) {
> + return;
> + }
> + result_l = env->regs[5];
> + result_l <<= 32;
> + result_l |= env->regs[4];
> + result_h = env->regs[6];
> + env->psw_o = 0;
> +
> + while (env->regs[3] != 0) {
> + tmp0 = ld[sz](env, env->regs[1], env->pc);
> + tmp1 = ld[sz](env, env->regs[2], env->pc);
You need to use signed load functions here.
> +void helper_satr(CPURXState *env)
> +{
> + if (env->psw_o >> 31) {
> + if ((int)env->psw_s < 0) {
> + env->regs[4] = 0x00000000;
> + env->regs[5] = 0x7fffffff;
> + env->regs[6] = 0xffffffff;
> + } else {
> + env->regs[4] = 0xffffffff;
> + env->regs[5] = 0x80000000;
> + env->regs[6] = 0x00000000;
These constants are in the wrong order.
> +/* div */
> +uint32_t helper_div(CPURXState *env, uint32_t num, uint32_t den)
> +{
> + uint32_t ret = num;
> + if (den != 0) {
> + ret = (int32_t)num / (int32_t)den;
> + }
> + env->psw_o = ((num == INT_MIN && den == -1) || den == 0) << 31;
Because of x86 host, which will raise SIGFPE for INT_MIN/-1, this needs to be
if (den == 0 || (num == INT_MIN && den == -1)) {
ret = num; /* undefined */
env->psw_o = -1;
} else {
ret = (int32_t)num / (int32_t)den;
env->psw_o = 0;
}
r~
