RE: [PATCH v3 10/11] Hexagon (target/hexagon) Use direct block chaining for direct jump/branch

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, November 4, 2022 8:33 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@linaro.org; ale@rev.ng; anjo@rev.ng; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>
> Subject: Re: [PATCH v3 10/11] Hexagon (target/hexagon) Use direct block
> chaining for direct jump/branch
> 
> On 11/5/22 06:26, Taylor Simpson wrote:
> > Direct block chaining is documented here
> > https://qemu.readthedocs.io/en/latest/devel/tcg.html#direct-block-chai
> > ning
> >
> > Recall that Hexagon allows packets with multiple jumps where only the
> > first one with a true predicate will actually jump.  So, we can only
> > use direct block chaining when the packet contains a single PC-relative
> jump.
> 
> Not quite accurate.
> 
> Only the first two direct branches can use direct block chaining.  Other exits
> from the translation block could use indirect block chaining
> (tcg_gen_lookup_and_goto_ptr).  You just have to remember which is
> taken.
> 

I'll work on the wording in the commit message.  When there is a single 
PC-relative branch or jump in the packet, we use 
tcg_gen_goto_tb/tcg_gen_exit_tb.  Otherwise, we use tcg_gen_lookup_and_goto_ptr.


> That said, this is certainly an improvement.
> 
> > +    if (ctx->pkt->pkt_has_multi_cof) {
> > +        gen_write_new_pc_addr(ctx, tcg_constant_tl(dest), pred);
> > +    } else {
> > +        /* Defer this jump to the end of the TB */
> > +        g_assert(ctx->branch_cond == NULL);
> > +        ctx->has_single_direct_branch = true;
> > +        if (pred != NULL) {
> > +            ctx->branch_cond = tcg_temp_local_new();
> > +            tcg_gen_mov_tl(ctx->branch_cond, pred);
> > +        }
> > +        ctx->branch_dest = dest;
> 
> Perhaps re-use hex_branch_taken as branch_cond?

Good idea.  That will save the allocation/deallocation of the TCGv.  I'll 
change it to a TCGCond to indicate the comparison to be done (if any).  It will 
work nicely with your other suggestion to pass the branch condition along.


> 
> Anyway,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~