On Sat, 2016-08-06 at 14:33 +0530, Richard Henderson wrote:
> On 07/31/2016 10:43 AM, Benjamin Herrenschmidt wrote:
> >
> > Translate most conditions to TCG conditions and avoid the helper
> > for most of the common cases.
> >
> > > > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> > ---
> > target-ppc/translate.c | 168
> > ++++++++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 132 insertions(+), 36 deletions(-)
> >
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index 47eb9ed..561976f 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -3639,82 +3639,178 @@ static void gen_sc(DisasContext *ctx)
> >
> > /*** Trap
> > ***/
> >
> > -/* Check for unconditional traps (always or never) */
> > -static bool check_unconditional_trap(DisasContext *ctx)
> > +static int TO2tcg[32] = {
>
> Should be const.
Indeed.
> >
> > + -1, /* 0x0f > or = or u> or u< */
> > + -1, /* 0x13 < or u< or u> -> weird */
> > + -1, /* 0x19 < or > or u> -> weird */
> > + -1, /* 0x1a < or > or u< -> weird */
> > + -1, /* 0x1b < or > or u> or u< -> weird */
>
> Not that it matters much, but when you have both < and >, or both u< and u>,
> you can collapse to NE, no matter what else is there.
Not necessarily, if = is there too it beomes always ;-) But as
a rule I bail to the helper for anything that mixes signed and
non-signed, in part bcs I was thinking of tracing them in case
anybody uses such oddball constructs. I expect them to never
happen unless somebody has a bug.
> >
> > +#define TRAP_UNCOND (-1)
> > +#define TRAP_HELPER (-2)
> > +
> > +static int precheck_trap(DisasContext *ctx)
> > {
> > - /* Trap never */
> > - if (TO(ctx->opcode) == 0) {
> > - return true;
> > + int cond = TO2tcg[TO(ctx->opcode)];
> > +
> > + /* Weird traps go to helper */
> > + if (cond < 0) {
> > + return TRAP_HELPER;
> > }
> > - /* Trap always */
> > - if (TO(ctx->opcode) == 31) {
> > + /* Unconditionals */
> > + if (cond == TCG_COND_ALWAYS) {
> > gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
> > - return true;
> > + return TRAP_UNCOND;
> > }
> > - return false;
> > + if (cond == TCG_COND_NEVER) {
> > + return TRAP_UNCOND;
> > + }
> > + /* Invert the condition as we branch over the exception when the
> > + * condition is *not* met
> > + */
> > + return tcg_invert_cond(cond);
> > +}
> > +
> > +static void gen_trap(DisasContext *ctx)
> > +{
> > + TCGv_i32 t0, t1;
> > +
> > + t0 = tcg_const_i32(POWERPC_EXCP_PROGRAM);
> > + t1 = tcg_const_i32(POWERPC_EXCP_TRAP);
> > + gen_update_nip(ctx, ctx->nip - 4);
> > + gen_helper_raise_exception_err(cpu_env, t0, t1);
>
> gen_exception_err, as with the unconditional trap?
No, we call it from gen_tw etc... but we may also branch
around that call, so we should not exit the TB with an
exception in the context.
> >
> > static void gen_tw(DisasContext *ctx)
> > {
> > - TCGv_i32 t0;
> > + int cond = precheck_trap(ctx);
> > + TCGLabel *l1;
> > + TCGv t0;
> > + TCGv t1;
> >
> > - if (check_unconditional_trap(ctx)) {
> > + if (cond == TRAP_UNCOND) {
> > + return;
> > + } else if (cond == TRAP_HELPER) {
> > + TCGv_i32 trapop = tcg_const_i32(TO(ctx->opcode));
> > + gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
> > + cpu_gpr[rB(ctx->opcode)], trapop);
> > + tcg_temp_free_i32(trapop);
> > return;
> > }
> > - t0 = tcg_const_i32(TO(ctx->opcode));
> > - gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)],
> > cpu_gpr[rB(ctx->opcode)],
> > - t0);
> > - tcg_temp_free_i32(t0);
> > + l1 = gen_new_label();
> > + t0 = tcg_temp_new();
> > + t1 = tcg_temp_new();
> > + tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
> > + tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
> > + tcg_gen_brcond_tl(cond, t0, t1, l1);
>
> It's just as easy, and perhaps better, to truncate to 32 bits instead of
> extending to TL bits.
Ok, I'll have a look. I'm not actually *that* familiar with the various
ops in the IR, but I shall figure it out :-)
Cheers,
Ben.
>
> r~
