Re: [PATCH v5 13/14] Hexagon (target/hexagon) Reduce manipulation of slot_cancelled

[Anton Johansson via]

On 1/31/23 23:56, Taylor Simpson wrote:

  /* Called during packet commit when there are two scalar stores */
-void HELPER(probe_pkt_scalar_store_s0)(CPUHexagonState *env, int mmu_idx)
+void HELPER(probe_pkt_scalar_store_s0)(CPUHexagonState *env, int args)
  {
-    probe_store(env, 0, mmu_idx);
+    int mmu_idx = args & 0x3;
+    bool is_predicated = (args >> 2) & 1;
+    probe_store(env, 0, mmu_idx, is_predicated);
  }

Can we use bitmasks for the fields of args?

  void HELPER(probe_hvx_stores)(CPUHexagonState *env, int mmu_idx)
@@ -489,12 +492,14 @@ void HELPER(probe_pkt_scalar_hvx_stores)(CPUHexagonState 
*env, int mask,
      bool has_st0        = (mask >> 0) & 1;
      bool has_st1        = (mask >> 1) & 1;
      bool has_hvx_stores = (mask >> 2) & 1;
+    bool s0_is_pred     = (mask >> 3) & 1;
+    bool s1_is_pred     = (mask >> 4) & 1;

also used here

      if (has_st0) {
-        probe_store(env, 0, mmu_idx);
+        probe_store(env, 0, mmu_idx, s0_is_pred);
      }
      if (has_st1) {
-        probe_store(env, 1, mmu_idx);
+        probe_store(env, 1, mmu_idx, s1_is_pred);
      }
      if (has_hvx_stores) {
          HELPER(probe_hvx_stores)(env, mmu_idx);
@@ -1444,12 +1449,6 @@ void HELPER(vwhist128qm)(CPUHexagonState *env, int32_t 
uiV)
      }
  }

-void cancel_slot(CPUHexagonState *env, uint32_t slot)

-{
-    HEX_DEBUG_LOG("Slot %d cancelled\n", slot);
-    env->slot_cancelled |= (1 << slot);
-}
-
  /* These macros can be referenced in the generated helper functions */
  #define warn(...) /* Nothing */
  #define fatal(...) g_assert_not_reached();
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 53fd935db7..6ee8784910 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -248,7 +248,16 @@ static bool check_for_attrib(Packet *pkt, int attrib)

static bool need_slot_cancelled(Packet *pkt)

  {
-    return check_for_attrib(pkt, A_CONDEXEC);
+    /* We only need slot_cancelled for conditional store and HVX instructions 
*/
+    for (int i = 0; i < pkt->num_insns; i++) {
+        uint16_t opcode = pkt->insn[i].opcode;
+        if (GET_ATTRIB(opcode, A_CONDEXEC) &&
+            (GET_ATTRIB(opcode, A_STORE) ||
+             GET_ATTRIB(opcode, A_CVI))) {
+            return true;
+        }
+    }
+    return false;
  }

static bool need_pred_written(Packet *pkt)

@@ -860,6 +869,12 @@ static void gen_commit_packet(DisasContext *ctx)
              if (has_hvx_store) {
                  mask |= (1 << 2);
              }
+            if (has_store_s0 && slot_is_predicated(pkt, 0)) {
+                mask |= (1 << 3);
+            }
+            if (has_store_s1 && slot_is_predicated(pkt, 1)) {
+                mask |= (1 << 4);
+            }
              mask_tcgv = tcg_constant_tl(mask);
              gen_helper_probe_pkt_scalar_hvx_stores(cpu_env, mask_tcgv, 
mem_idx);
          }

and here

@@ -868,8 +883,12 @@ static void gen_commit_packet(DisasContext *ctx)
           * process_store_log will execute the slot 1 store first,
           * so we only have to probe the store in slot 0
           */
-        TCGv mem_idx = tcg_constant_tl(ctx->mem_idx);
-        gen_helper_probe_pkt_scalar_store_s0(cpu_env, mem_idx);
+        int args = ctx->mem_idx;
+        if (slot_is_predicated(pkt, 0)) {
+            args |= (1 << 2);
+        }
+        TCGv args_tcgv = tcg_constant_tl(args);
+        gen_helper_probe_pkt_scalar_store_s0(cpu_env, args_tcgv);
      }

and here

Otherwise,

Reviewed-by: Anton Johansson <a...@rev.ng>