On Fri, Jun 04, 2010 at 11:30:38AM -0700, Richard Henderson wrote:
> On 05/24/2010 09:19 AM, Nathan Froyd wrote:
> > + int (*ldfun)(target_ulong);
> > +
> > + switch (mem_idx)
> > + {
> > + case 0: ldfun = ldl_kernel; break;
> > + case 1: ldfun = ldl_super; break;
> > + default:
> > + case 2: ldfun = ldl_user; break;
> > + }
>
> This *should* now be a compile error. The return type should
> now be "uint32_t", not "int".
Will fix, thanks for pointing that out.
> > @@ -2535,26 +2555,29 @@ static void gen_compute_branch (DisasContext *ctx,
> > uint32_t opc,
> > case OPC_JALX:
> > ctx->hflags |= MIPS_HFLAG_BX;
> > /* Fallthrough */
> > + case OPC_JALS:
> > case OPC_JAL:
> > blink = 31;
> > ctx->hflags |= MIPS_HFLAG_B;
> > - ctx->hflags |= (ctx->hflags & MIPS_HFLAG_M16
> > + ctx->hflags |= (opc == OPC_JALS
> > ? MIPS_HFLAG_BDS16
> > : MIPS_HFLAG_BDS32);
>
> Changed semantics here? You're no longer testing M16 bit.
> Or is that later handled by switching mips16 to JALS too?
It ought to be handled by switching mips16 to use JALS. I see that I
didn't do that.
> And if so, perhaps this patch should be broken into two, where
> you introduce the new opcodes and hflags changes and apply them
> as-needed to the mips16 code. Thus one can verify that the
> semantics for mips16 are the same before and after.
I don't think there are any hflags changes, but the point is
well-taken. I'll do that.
> > + if (base == 0) {
> > + tcg_gen_movi_tl(t0, 0);
> > + } else {
> > + gen_load_gpr(t0, base);
> > + }
>
> gen_load_gpr already takes care of R0.
Will fix.
> > +#if 0
> > + case 0x01:
> > + switch (minor) {
> > + case MFHI_ACC:
> > + gen_HILO(ctx, OPC_MFHI, rs);
>
> New if 0 code?
Yup, will delete.
-Nathan
