Re: [Qemu-devel] [PATCH v14 14/33] target-tilegx: Handle simple logical operations

[Peter Maydell]

On 24 August 2015 at 17:17, Richard Henderson <address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target-tilegx/translate.c | 99 
> +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
> index a2d597d..066d351 100644
> --- a/target-tilegx/translate.c
> +++ b/target-tilegx/translate.c
> @@ -106,9 +106,64 @@ static void gen_exception(DisasContext *dc, TileExcp num)
>      dc->exit_tb = true;
>  }
>
> +static bool check_gr(DisasContext *dc, uint8_t reg)
> +{
> +    if (likely(reg < TILEGX_R_COUNT)) {
> +        return true;
> +    }
> +
> +    switch (reg) {
> +    case TILEGX_R_SN:
> +    case TILEGX_R_ZERO:
> +        break;
> +    case TILEGX_R_IDN0:
> +    case TILEGX_R_IDN1:
> +        gen_exception(dc, TILEGX_EXCP_REG_IDN_ACCESS);
> +        break;
> +    case TILEGX_R_UDN0:
> +    case TILEGX_R_UDN1:
> +    case TILEGX_R_UDN2:
> +    case TILEGX_R_UDN3:
> +        gen_exception(dc, TILEGX_EXCP_REG_UDN_ACCESS);

Why does this function generate an exception immediately
rather than returning a TILEGX_EXCP_* code the way the
decode framework seems to be set up to work?

> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    return false;
> +}
> +
> +static TCGv load_zero(DisasContext *dc)
> +{
> +    if (TCGV_IS_UNUSED_I64(dc->zero)) {
> +        dc->zero = tcg_const_i64(0);
> +    }
> +    return dc->zero;
> +}

The ARM frontend has made me a bit suspicious of TCGv
and target_long rather than specifically using TCGv_i32
or i64 and uint32_t/uint64_t, but I guess it makes sense
here.

Rest of the patch looks OK.

thanks
-- PMM