On Wed, Dec 9, 2020 at 9:34 AM Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 12/8/20 4:56 PM, Alistair Francis wrote:
> > @@ -1053,11 +1077,11 @@ static int read_htimedelta(CPURISCVState *env, int
> > csrno, target_ulong *val)
> > return -RISCV_EXCP_ILLEGAL_INST;
> > }
> >
> > -#if defined(TARGET_RISCV32)
> > - *val = env->htimedelta & 0xffffffff;
> > -#else
> > - *val = env->htimedelta;
> > -#endif
> > + if (riscv_cpu_is_32bit(env)) {
> > + *val = env->htimedelta & 0xffffffff;
> > + } else {
> > + *val = env->htimedelta;
> > + }
> > return 0;
> > }
>
> Certainly this mask was useless before the patch, because of the ifdef and
> target_ulong.
>
> Afterward, target_ulong may be larger than uint32_t... but what does the rest
> of the value, and the register into which it is stored, represent?
>
> Are you going to keep the register values sign-extended, as-if by addw et al,
> in which case casting to int32_t would be better. Are you going to simply
> ignore the upper 32 bits, in which case dropping the masking entirely would be
> better.
We are just going to ignore the top half, so I'll drop the mask.
Alistair
>
>
> r~
