On 6/17/24 23:27, Richard Henderson wrote:
On 6/17/24 04:51, Chinmay Rath wrote:
Hi Richard,
On 6/17/24 00:43, Richard Henderson wrote:
On 6/13/24 02:33, Chinmay Rath wrote:
+/* EA <- (ra == 0) ? 0 : GPR[ra] */
+static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
+{
+ TCGv EA;
+ if (!ra) {
+ EA = tcg_constant_tl(0);
+ return EA;
+ }
+ EA = tcg_temp_new();
+ if (NARROW_MODE(ctx)) {
+ tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
+ } else {
+ tcg_gen_mov_tl(EA, cpu_gpr[ra]);
Why are you making a copy, rather than just returning cpu_gpr[ra]?
If you need to modify the resulting EA, then you also need to make a
copy for 0.
Please ignore my previous response.
I think do_ea_calc_ra should allow modification to the resulting EA,
hence below change appears more appropriate to me :
/* EA <- (ra == 0) ? 0 : GPR[ra] */
static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
{
TCGv EA = tcg_temp_new();
if (!ra) {
tcg_gen_movi_tl(EA, 0);
return EA;
}
if (NARROW_MODE(ctx)) {
tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
} else {
tcg_gen_mov_tl(EA, cpu_gpr[ra]);
}
return EA;
}
If that's what's needed by the callers of do_ea_calc_ra, then yes.
You can drop the first return EA and use else if instead.
Sure.
I shall stick to keeping EA modifiable, (even though it is not modified
by the callers in this patch),
to allow its proper usage by (p){lx, stx}vp insns in future.
Thanks & Regards,
Chinmay
r~
