Re: [Qemu-devel] [RFC v1 15/23] riscv: tcg-target: Add branch and jump instructions

[Alistair Francis]

On Tue, Nov 20, 2018 at 11:40 PM Richard Henderson
<address@hidden> wrote:
>
> On 11/21/18 12:49 AM, Alistair Francis wrote:
> > On Fri, Nov 16, 2018 at 1:14 AM Richard Henderson
> > <address@hidden> wrote:
> >>
> >> On 11/15/18 11:36 PM, Alistair Francis wrote:
> >>> +static void tcg_out_brcond(TCGContext *s, TCGCond cond, TCGReg arg1,
> >>> +                           TCGReg arg2, TCGLabel *l)
> >>> +{
> >>> +    RISCVInsn op = tcg_brcond_to_riscv[cond].op;
> >>> +    bool swap = tcg_brcond_to_riscv[cond].swap;
> >>> +
> >>> +    tcg_out_opc_branch(s, op, swap ? arg2 : arg1, swap ? arg1 : arg2, 0);
> >>
> >> You might want to tcg_debug_assert(op != 0) here.
> >>
> >>> +    if (l->has_value) {
> >>> +        reloc_sbimm12(s->code_ptr - 1, l->u.value_ptr);
> >>
> >> I'm concerned about the conditional branch range.  +-4K isn't much to work
> >> with.  The minimum we have for other hosts is +-32K.
> >>
> >> We have two options: (1) greatly reduce the max size of the TB for this 
> >> host;
> >> (2) be prepared to emit a 2 insn sequence: conditional branch across
> >> unconditional branch, with forward branches that turn out to be small 
> >> patched
> >> with a nop.
> >>
> >> FWIW, the first case would be done via modification of tcg_op_buf_full.  
> >> You
> >> might have to go as low as 500 opcodes, I'm not sure.
> >
> > How do we do option 2?
>
> If l->has_value, just check the actual range.  But of course backward 
> branching
> isn't that common in tcg generated code.  Most branches within the TB are 
> short
> forward branches, but we also don't know how short is short.
>
> But every TB begins with a test of env->exit_code and a conditional branch to
> the end of the block, where we place some code to return to the main loop and
> return the pointer to the TB at which we exited.  Thus every TB has a branch
> that spans the size of the entire TB.
>
> So, invent (or repurpose) an R_RISCV_FOO value.  It doesn't matter which
> because it's private within tcg/riscv/.  Just add some commentary.  (See e.g.
> tcg/sparc/ and its use of R_SPARC_13.)
>
> While generating code, emit the conditional branch as normal; leave the 
> unknown
> destination 0 for now.  Emit a nop as the second insn.
>
> When resolving R_RISCV_FOO, if the conditional branch is in range, great!  
> Just
> patch it.  If it is out of range, then you need to edit the conditional branch
> to reverse the condition (insn ^ (1 << 12)) and branch to pc+8, i.e. over the
> next instruction.  Which was a nop during generation, but you will now install
> jal r0,dest going to the real destination.

Ok, I think I have done this correctly. I have something that compiles and runs.

I'll send a second RFC out sometime this week. It won't be based on
your latest patches and it's still missing some things but I want to
keep this moving along.

Thanks so much for your help Richard!

Alistair

>
>
> r~