Re: [Qemu-devel] [PATCH RFC v20 5/8] target/avr: Add instruction translation

[Michael Rolnik]

Got it.
Thanks for the explanation

Sent from my cell phone, please ignore typos

On Mon, Jun 3, 2019, 6:31 PM Richard Henderson <address@hidden>
wrote:

> On 6/1/19 10:44 PM, Michael Rolnik wrote:
> > Hi Richard.
> >
> > these instructions are not branches or jumps they all do skip.
>
> Of course they're not all branches.  I used the example of a branch to
> show a
> situation in which your translation is wrong.
>
> > however, if you think it's important I change it, I will, just show me an
> > example or explain.
>
> Ok, let's take HPPA as a very similar example.
>
> Many HPPA instructions may "nullify" the next instruction.  The language is
> different, but it's the same thing as the AVR "skip".
>
> Now, I spent quite a bit of effort in target/hppa using conditional move
> instructions to implement this.  But you need not go that far.
>
> Now, AVR is differs from HPPA in that there is a nullify bit as part of the
> process state.  Since AVR does not have this, we will need to keep the
> "skip"
> state entirely internal to the qemu implementation.
>
> I suggest:
>
> (1) Add "bool skipping;" to CPUAVRState.
>
> Because TranslationBlocks can be ended at any instruction boundary, we need
> some way to preserve the skipping state across TB's.
>
> (2) Include "skipping" into the flags for cpu_get_tb_cpu_state,
> TB_FLAGS_SKIPPING.
>
> (3) Include "skipping" into the computation of cpu_interrupts_enabled.
>
> Because "skipping" is not part of the architectural state of the CPU, we
> cannot
> allow an interrupt to come between the two instructions.  Therefore, while
> "skipping" is true, disable interrupts.
>
> (4) Within the instructions that skip the next, issue the branch but
> record the
> label as DisasContext->skip_label.  This will allow the main loop (and
> other
> instructions) know that skipping is active.
>
> (5a) In gen_intermediate_code, if TB_FLAGS_SKIPPING, decode but do not
> translate the insn, then clear TB_FLAGS_SKIPPING and store 0 into
> env->skipping.
>
> (5b) In gen_intermediate_code, if !TB_FLAGS_SKIPPING, copy
> DisasContext->skip_label into a local variable, this_skip_label and zero.
>
> We need to prepare for skip of skip, so do not allow the label of the first
> skip to be clobbered by the label of the second skip.
>
> (5c) After translate(), if this_skip_label is non-null, emit the label.
>
> (6) Reverse the sense of your conditional branches.
>
> Currently you generate
>
>   brcond(xxx, yyy, zzz, true_label);
>   goto npc
> true_label:
>   goto true_pc
>
> which is fine until we have skip labels.  We now want to emit
>
>   brcond(!xxx, yyy, zzz, false_label);
>   goto true_pc
> false_label:
> skip_label:
>   goto npc
>
> which you can do by issuing only the branch, goto, label, and then setting
> ctx->bstate to BS_STOP, so that the skip_label is emitted by the main
> loop, and
> the goto npc is also issued by the main loop.
>
> (7) At the end of the loop in gen_intermedite_code, if
> DisasContext->skip_label
> is non-null, then we ended the TB with a skipping instruction and we need
> to
> preserve that within env.
>
>     TCGLabel *finish = NULL;
>
>     if (ctx.skip_label) {
>         finish = gen_new_label();
>     }
>
>     if (tb->cflags & CF_LAST_IO) {
>     ...
>
>
>     if (ctx.skip_label) {
>         TCGv_i32 one;
>
>         gen_set_label(ctx.skip_label);
>         one = tcg_const_i32(1);
>         tcg_gen_st8_i32(one, cpu_env, offsetof(CPUAVRState, skipping));
>         tcg_temp_free_i32(one);
>         tcg_gen_br(finish);
>     }
>
>  done_generating:
>    gen_tb_end(tb, num_insns);
>
>
> r~
>