Re: [Qemu-devel] [PATCH v14 22/33] target-tilegx: Implement system and memory management instructions

[Richard Henderson]

On 08/29/2015 02:21 PM, Peter Maydell wrote:
> > +    case OE_RR_X1(SWINT3):
> > +        ret = TILEGX_EXCP_OPCODE_UNKNOWN;
> > +        mnemonic = "swint3";
> > +        goto done0;
> > +    done0:
>
> goto to immediately following label ?

Sometimes nicer than fallthrough when future patches might interpose extra code.

> > @@ -284,8 +339,12 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
> > opext,
> >           dc->jmp.cond = TCG_COND_ALWAYS;
> >           dc->jmp.dest = tcg_temp_new();
> >           tcg_gen_andi_tl(dc->jmp.dest, load_gr(dc, srca), ~7);
>
> Should this really fall through into the added check ?
>
> > +    done1:
> > +        if (dest && ret == TILEGX_EXCP_NONE) {
> > +            ret = TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
> > +        }
...
> >       case OE_RR_X1(LNK):
> >       case OE_RR_Y1(LNK):
> >           if (srca) {
> > -            return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
> > +            ret = TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>
> This code change seems unrelated to the rest of the patch?
> Also we'll end up printing the disassembly as "lnk" rather
> than whatever we should print for undefined instructions.

I'm not really sure what to do with reserved operand fields on this 
architecture.  Raising an exception is what Chen started with, but the encoding 
for brk suggests that they're ignored.

Suggestions?


r~