| Message ID | 20250323173730.3213964-11-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/avr: Increase page size | expand |
On 3/23/25 10:37, Richard Henderson wrote: > Not that AVR has memory paging traps, but it's better > form to allow the memory operation to finish before > updating the cpu register. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/avr/translate.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/target/avr/translate.c b/target/avr/translate.c > index 6bb4154dff..3446007be1 100644 > --- a/target/avr/translate.c > +++ b/target/avr/translate.c > @@ -967,40 +967,38 @@ static void gen_push_ret(DisasContext *ctx, int ret) > } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) { > TCGv t0 = tcg_constant_i32(ret & 0x00ffff); > > - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1); > - gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_BEUW); > - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1); > + gen_data_store_raw(ctx, t0, cpu_sp, -1, MO_BEUW); > + tcg_gen_subi_tl(cpu_sp, cpu_sp, 2); > } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) { > TCGv lo = tcg_constant_i32(ret & 0x0000ff); > TCGv hi = tcg_constant_i32((ret & 0xffff00) >> 8); > > gen_data_store_raw(ctx, lo, cpu_sp, 0, MO_UB); > - tcg_gen_subi_tl(cpu_sp, cpu_sp, 2); > - gen_data_store_raw(ctx, hi, cpu_sp, 0, MO_BEUW); > - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1); > + gen_data_store_raw(ctx, hi, cpu_sp, -2, MO_BEUW); > + tcg_gen_subi_tl(cpu_sp, cpu_sp, 3); > + } else { > + g_assert_not_reached(); > } > } > > static void gen_pop_ret(DisasContext *ctx, TCGv ret) > { > if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) { > + gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_UB); > tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); > - gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_UB); > } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) { > - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); > - gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_BEUW); > - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); > + gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_BEUW); > + tcg_gen_addi_tl(cpu_sp, cpu_sp, 2); > } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) { > - TCGv lo = tcg_temp_new_i32(); > TCGv hi = tcg_temp_new_i32(); > > - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); > - gen_data_load_raw(ctx, hi, cpu_sp, 0, MO_BEUW); > + gen_data_load_raw(ctx, hi, cpu_sp, 1, MO_BEUW); > + gen_data_load_raw(ctx, ret, cpu_sp, 3, MO_UB); > + tcg_gen_addi_tl(cpu_sp, cpu_sp, 3); > > - tcg_gen_addi_tl(cpu_sp, cpu_sp, 2); > - gen_data_load_raw(ctx, lo, cpu_sp, 0, MO_UB); > - > - tcg_gen_deposit_tl(ret, lo, hi, 8, 16); > + tcg_gen_deposit_tl(ret, ret, hi, 8, 16); > + } else { > + g_assert_not_reached(); > } > } > A bit hard to review with the side effect of gen_data_*_raw, but number seems to match. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
diff --git a/target/avr/translate.c b/target/avr/translate.c index 6bb4154dff..3446007be1 100644 --- a/target/avr/translate.c +++ b/target/avr/translate.c @@ -967,40 +967,38 @@ static void gen_push_ret(DisasContext *ctx, int ret) } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) { TCGv t0 = tcg_constant_i32(ret & 0x00ffff); - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1); - gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_BEUW); - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1); + gen_data_store_raw(ctx, t0, cpu_sp, -1, MO_BEUW); + tcg_gen_subi_tl(cpu_sp, cpu_sp, 2); } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) { TCGv lo = tcg_constant_i32(ret & 0x0000ff); TCGv hi = tcg_constant_i32((ret & 0xffff00) >> 8); gen_data_store_raw(ctx, lo, cpu_sp, 0, MO_UB); - tcg_gen_subi_tl(cpu_sp, cpu_sp, 2); - gen_data_store_raw(ctx, hi, cpu_sp, 0, MO_BEUW); - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1); + gen_data_store_raw(ctx, hi, cpu_sp, -2, MO_BEUW); + tcg_gen_subi_tl(cpu_sp, cpu_sp, 3); + } else { + g_assert_not_reached(); } } static void gen_pop_ret(DisasContext *ctx, TCGv ret) { if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) { + gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_UB); tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); - gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_UB); } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) { - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); - gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_BEUW); - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); + gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_BEUW); + tcg_gen_addi_tl(cpu_sp, cpu_sp, 2); } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) { - TCGv lo = tcg_temp_new_i32(); TCGv hi = tcg_temp_new_i32(); - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1); - gen_data_load_raw(ctx, hi, cpu_sp, 0, MO_BEUW); + gen_data_load_raw(ctx, hi, cpu_sp, 1, MO_BEUW); + gen_data_load_raw(ctx, ret, cpu_sp, 3, MO_UB); + tcg_gen_addi_tl(cpu_sp, cpu_sp, 3); - tcg_gen_addi_tl(cpu_sp, cpu_sp, 2); - gen_data_load_raw(ctx, lo, cpu_sp, 0, MO_UB); - - tcg_gen_deposit_tl(ret, lo, hi, 8, 16); + tcg_gen_deposit_tl(ret, ret, hi, 8, 16); + } else { + g_assert_not_reached(); } }
Not that AVR has memory paging traps, but it's better form to allow the memory operation to finish before updating the cpu register. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/avr/translate.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)