On 11/19/20 12:35 PM, Richard Henderson wrote:
> On 11/18/20 12:29 AM, frank.chang@sifive.com wrote:
>> +static bool trans_sbset(DisasContext *ctx, arg_sbset *a)
>> +{
>> + REQUIRE_EXT(ctx, RVB);
>> + return gen_arith(ctx, a, &gen_sbset);
>> +}
>> +
>> +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a)
>> +{
>> + REQUIRE_EXT(ctx, RVB);
>> + return gen_arith_shamt_tl(ctx, a, &gen_sbset);
>> +}
>> +
>> +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a)
>> +{
>> + REQUIRE_EXT(ctx, RVB);
>> + return gen_arith(ctx, a, &gen_sbclr);
>> +}
>
> Coming back to my re-use of code thing, these should use gen_shift. That
> handles the truncate of source2 to the shift amount.
>
>> +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a)
>> +{
>> + REQUIRE_EXT(ctx, RVB);
>> + return gen_arith_shamt_tl(ctx, a, &gen_sbclr);
>> +}
>> +
>> +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a)
>> +{
>> + REQUIRE_EXT(ctx, RVB);
>> + return gen_arith(ctx, a, &gen_sbinv);
>> +}
>> +
>> +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a)
>> +{
>> + REQUIRE_EXT(ctx, RVB);
>> + return gen_arith_shamt_tl(ctx, a, &gen_sbinv);
>> +}
>
> I think there ought to be a gen_shifti for these.
Hmm. I just realized that gen_shifti would have a generator callback with a
constant argument, a-la tcg_gen_shli_tl.
I don't know if it's worth duplicating gen_sbclr et al for a constant argument.
And the sloi/sroi insns besides. Perhaps a gen_shifti_var helper instead?
Let me know what you think, but at the moment we're left with an incoherent set
of helpers that seem split along lines that are less than ideal.
r~
Thanks Richard and sorry for the late reply.....
If we can have gen_shift(), gen_shifti(), gen_shiftw() and gen_shiftiw(),
then we can eliminate the needs of:
gen_arith_shamt_tl(), gen_sbop_shamt(), gen_sbopw_shamt() and gen_sbopw_common()
and most of the *w version generators can be removed, too.
For *w version, we just need to call gen_shiftw() or gen_shiftiw()
with the reused non-*w version generator.
For example:
static bool trans_sbclrw(DisasContext *ctx, arg_sbclrw *a)
{
REQUIRE_EXT(ctx, RVB);
return gen_shiftw(ctx, a, &gen_sbclr);
}
static bool trans_sbclriw(DisasContext *ctx, arg_sbclriw *a)
{
REQUIRE_EXT(ctx, RVB);
return gen_shiftiw(ctx, a, &gen_sbclr);
}
both of which can reuse gen_sbclr() generator:
static void gen_sbclr(TCGv ret, TCGv arg1, TCGv shamt)
{
TCGv t = tcg_temp_new();
tcg_gen_movi_tl(t, 1);
tcg_gen_shl_tl(t, t, shamt);
tcg_gen_andc_tl(ret, arg1, t);
tcg_temp_free(t);
}
The gen_shift*() I have now are as follow:
static bool gen_shift(DisasContext *ctx, arg_r *a,
void(*func)(TCGv, TCGv, TCGv))
{
TCGv source1 = tcg_temp_new();
TCGv source2 = tcg_temp_new();
gen_get_gpr(source1, a->rs1);
gen_get_gpr(source2, a->rs2);
tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
(*func)(source1, source1, source2);
gen_set_gpr(a->rd, source1);
tcg_temp_free(source1);
tcg_temp_free(source2);
return true;
}
static bool gen_shifti(DisasContext *ctx, arg_shift *a,
void(*func)(TCGv, TCGv, TCGv))
{
TCGv source1 = tcg_temp_new();
TCGv source2 = tcg_temp_new();
gen_get_gpr(source1, a->rs1);
tcg_gen_movi_tl(source2, a->shamt);
tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
(*func)(source1, source1, source2);
gen_set_gpr(a->rd, source1);
tcg_temp_free(source1);
tcg_temp_free(source2);
return true;
}
static bool gen_shiftw(DisasContext *ctx, arg_r *a,
void(*func)(TCGv, TCGv, TCGv))
{
TCGv source1 = tcg_temp_new();
TCGv source2 = tcg_temp_new();
gen_get_gpr(source1, a->rs1);
gen_get_gpr(source2, a->rs2);
tcg_gen_andi_tl(source2, source2, 31);
(*func)(source1, source1, source2);
tcg_gen_ext32s_tl(source1, source1);
gen_set_gpr(a->rd, source1);
tcg_temp_free(source1);
tcg_temp_free(source2);
return true;
}
static bool gen_shiftiw(DisasContext *ctx, arg_shift *a,
void(*func)(TCGv, TCGv, TCGv))
{
TCGv source1 = tcg_temp_new();
TCGv source2 = tcg_temp_new();
gen_get_gpr(source1, a->rs1);
tcg_gen_movi_tl(source2, a->shamt);
tcg_gen_andi_tl(source2, source2, 31);
(*func)(source1, source1, source2);
tcg_gen_ext32s_tl(source1, source1);
gen_set_gpr(a->rd, source1);
tcg_temp_free(source1);
tcg_temp_free(source2);
return true;
}
They may be further merged as most of them are duplicate with only the differences of:
gen_get_gpr(source2, a->rs2); vs. tcg_gen_movi_tl(source2, a->shamt);
TARGET_LONG_BITS - 1 vs. 31, and
tcg_gen_ext32s_tl(); to sign-extend the 32-bit return value for *w instructions
Any thoughts?
Frank Chang