Re: [PATCH 02/10] target/tricore: Implement CRCN insn

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

On 8/26/23 09:02, Bastian Koppelmann wrote:
> +static uint32_t crc_div(uint32_t crc_in, uint32_t data, uint32_t gen,
> +                        uint32_t n, uint32_t m)
> +{
> +    uint32_t i;
> +
> +    data = data << n;
> +    data = deposit32(data, m, 32 - m, 0);

This is data = extract32(data, 0, m), however...

> +    for (i = 0; i < m; i++) {
> +
> +        if (crc_in & (1u << (n - 1))) {
> +            crc_in <<= 1;
> +            if (data & (1u << (m - 1))) {

You only check a single bit of data here, always bit m-1.

> +                crc_in++;
> +            }
> +            crc_in ^= gen;
> +        } else {
> +            crc_in <<= 1;
> +            if (data & (1u << (m - 1))) {
> +                crc_in++;
> +            }
> +        }
> +        data <<= 1;
> +        data = deposit32(data, m, 32 - m, 0);

So why do you need to keep bits above m clear?
I think you should just shift left and let bits fall off the left naturally.

> +    return deposit32(crc_out, n, 32 - n, 0);

extract32(crc_out, 0, n);

> +}
> +
>   uint32_t helper_shuffle(uint32_t arg0, uint32_t arg1)
>   {
>       uint32_t resb;
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index 1947733870..bb7dad75d6 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -6673,6 +6673,12 @@ static void decode_rrr_divide(DisasContext *ctx)
>           gen_helper_pack(cpu_gpr_d[r4], cpu_PSW_C, cpu_gpr_d[r3],
>                           cpu_gpr_d[r3+1], cpu_gpr_d[r1]);
>           break;
> +    case OPC2_32_RRR_CRCN:
> +        if (has_feature(ctx, TRICORE_FEATURE_162)) {
> +            gen_helper_crcn(cpu_gpr_d[r4], cpu_gpr_d[r1], cpu_gpr_d[r2],
> +                            cpu_gpr_d[r3]);
> +        }
> +        break;

trap if not feature 162.


r~