Re: [RFC 08/15] target/riscv: rvb: single-bit instructions

[Richard Henderson <richard.henderson@linaro.org>]

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~