Re: [PULL 03/13] target/rx: TCG translation

2020-04-03 Thread Richard Henderson 
On 4/3/20 9:41 AM, Peter Maydell wrote:
> On Tue, 17 Mar 2020 at 16:52, Philippe Mathieu-Daudé  wrote:
>>
>> From: Yoshinori Sato 
>>
>> This part only supported RXv1 instructions.
>>
>> Instruction manual:
>>   
>> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01us0032ej0120_rxsm.pdf
> 
> Hi; Coverity points out a possible missing 'break' here
> (CID 142):
> 
>> +static void rx_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>> +{
>> +DisasContext *ctx = container_of(dcbase, DisasContext, base);
>> +
>> +switch (ctx->base.is_jmp) {
>> +case DISAS_NEXT:
>> +case DISAS_TOO_MANY:
>> +gen_goto_tb(ctx, 0, dcbase->pc_next);
>> +break;
>> +case DISAS_JUMP:
>> +if (ctx->base.singlestep_enabled) {
>> +gen_helper_debug(cpu_env);
>> +} else {
>> +tcg_gen_lookup_and_goto_ptr();
>> +}
>> +break;
>> +case DISAS_UPDATE:
>> +tcg_gen_movi_i32(cpu_pc, ctx->base.pc_next);
> 
> Should this have a "break" or a "/* fall through */" comment ?
> 
>> +case DISAS_EXIT:
>> +tcg_gen_exit_tb(NULL, 0);
>> +break;

This one should be a fall through.


r~

Re: [PULL 03/13] target/rx: TCG translation

2020-04-03 Thread Peter Maydell 
On Tue, 17 Mar 2020 at 16:52, Philippe Mathieu-Daudé  wrote:
>
> From: Yoshinori Sato 
>
> This part only supported RXv1 instructions.
>
> Instruction manual:
>   
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01us0032ej0120_rxsm.pdf

Hi; Coverity points out a possible missing 'break' here
(CID 142):

> +static void rx_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
> +{
> +DisasContext *ctx = container_of(dcbase, DisasContext, base);
> +
> +switch (ctx->base.is_jmp) {
> +case DISAS_NEXT:
> +case DISAS_TOO_MANY:
> +gen_goto_tb(ctx, 0, dcbase->pc_next);
> +break;
> +case DISAS_JUMP:
> +if (ctx->base.singlestep_enabled) {
> +gen_helper_debug(cpu_env);
> +} else {
> +tcg_gen_lookup_and_goto_ptr();
> +}
> +break;
> +case DISAS_UPDATE:
> +tcg_gen_movi_i32(cpu_pc, ctx->base.pc_next);

Should this have a "break" or a "/* fall through */" comment ?

> +case DISAS_EXIT:
> +tcg_gen_exit_tb(NULL, 0);
> +break;
> +case DISAS_NORETURN:
> +break;
> +default:
> +g_assert_not_reached();
> +}
> +}

thanks
-- PMM