diff mbox series

[v2,2/2] target/riscv: Call check_access() before tcg_temp_new()

Message ID 20210222184940.43169-2-Alexander.Richardson@cl.cam.ac.uk
State New
Headers show
Series [v2,1/2] target/riscv: Reduce duplicated code in trans_rvh.c.inc | expand

Commit Message

Alex Richardson Feb. 22, 2021, 6:49 p.m. UTC 
---
 target/riscv/insn_trans/trans_rvh.c.inc | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Richard Henderson Feb. 24, 2021, 12:31 a.m. UTC | #1 
On 2/22/21 10:49 AM, Alex Richardson wrote:
> ---
>  target/riscv/insn_trans/trans_rvh.c.inc | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Any particular reason?

At first I thought this was a macro with a hidden return, but it isn't.  I
would understand a cleanup like

    if (check_access(ctx)) {
       TCGv t0 = tcg_temp_new();
       TCGv t1 = tcg_temp_new();
       ...

where we do not generate the rest of the code if we raised an exception.
Alternately, if the nesting seems ugly,

    if (!check_access(ctx)) {
        return true;
    }
    ...
    return true;


r~
diff mbox series

Patch

diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc
index c66268a9b0..c6bbc54d68 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -32,11 +32,11 @@  static bool gen_hlv(DisasContext *ctx, int rs1, int rd, MemOp memop)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+    check_access(ctx);
+
     TCGv t0 = tcg_temp_new();
     TCGv t1 = tcg_temp_new();
 
-    check_access(ctx);
-
     gen_get_gpr(t0, rs1);
 
     tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, memop);
@@ -55,11 +55,11 @@  static bool gen_hsv(DisasContext *ctx, int rs1, int rs2, MemOp memop)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+    check_access(ctx);
+
     TCGv t0 = tcg_temp_new();
     TCGv dat = tcg_temp_new();
 
-    check_access(ctx);
-
     gen_get_gpr(t0, rs1);
     gen_get_gpr(dat, rs2);
 
@@ -134,11 +134,11 @@  static bool trans_hlvx_hu(DisasContext *ctx, arg_hlvx_hu *a)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+    check_access(ctx);
+
     TCGv t0 = tcg_temp_new();
     TCGv t1 = tcg_temp_new();
 
-    check_access(ctx);
-
     gen_get_gpr(t0, a->rs1);
 
     gen_helper_hyp_hlvx_hu(t1, cpu_env, t0);
@@ -156,11 +156,11 @@  static bool trans_hlvx_wu(DisasContext *ctx, arg_hlvx_wu *a)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+    check_access(ctx);
+
     TCGv t0 = tcg_temp_new();
     TCGv t1 = tcg_temp_new();
 
-    check_access(ctx);
-
     gen_get_gpr(t0, a->rs1);
 
     gen_helper_hyp_hlvx_wu(t1, cpu_env, t0);