[PATCH 0/6] misc hexagon patches

[PATCH 0/6] misc hexagon patches

[Brian Cain]

While preparing the system emulation patches, these ones stuck out as
not-strictly-related to sysemu.  We can review and apply them independently
of those.

Brian Cain (6):
  target/hexagon: handle .new values
  target/hexagon: Fix badva reference, delete CAUSE
  target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof
  target/hexagon: s/pkt_has_store/pkt_has_scalar_store
  target/hexagon: Remove unreachable
  target/hexagon: Add memory order definition

 target/hexagon/idef-parser/README.rst       |  2 +-
 target/hexagon/cpu-param.h                  |  5 +++
 target/hexagon/insn.h                       |  4 +--
 target/hexagon/macros.h                     |  8 ++---
 target/hexagon/cpu.c                        |  3 +-
 target/hexagon/decode.c                     | 10 ++++--
 target/hexagon/genptr.c                     |  3 +-
 target/hexagon/idef-parser/parser-helpers.c |  4 +--
 target/hexagon/op_helper.c                  |  4 +--
 target/hexagon/translate.c                  |  9 +++---
 target/hexagon/gen_helper_funcs.py          |  2 +-
 target/hexagon/hex_common.py                | 34 ++++++++++++++++-----
 12 files changed, 59 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH 1/6] target/hexagon: handle .new values

[Brian Cain]

From: Brian Cain <bcain@quicinc.com>

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/hex_common.py | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index 758e5fd12d..242dee3731 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -349,6 +349,12 @@ def helper_arg(self):
             self.reg_tcg(),
             f"{self.helper_arg_type()} {self.helper_arg_name()}"
         )
+    def from_subtype(self, subtype):
+        if subtype == "":
+            return self
+        raise Exception(
+            f"unknown subtype '{subtype}' on generic Register class")
+
 
 #
 # Every register is either Single or Pair or Hvx
@@ -1070,11 +1076,22 @@ def init_registers():
     for reg in new_regs:
         new_registers[f"{reg.regtype}{reg.regid}"] = reg
 
-def get_register(tag, regtype, regid):
-    if f"{regtype}{regid}V" in semdict[tag]:
-        return registers[f"{regtype}{regid}"]
-    else:
-        return new_registers[f"{regtype}{regid}"]
+def is_new_reg(tag, regid):
+    if regid[0] in "NO":
+        return True
+    return regid[0] == "P" and \
+           f"{regid}N" in semdict[tag] and \
+           f"{regid}V" not in semdict[tag]
+
+def get_register(tag, regtype, regid, subtype=""):
+    regid = f"{regtype}{regid}"
+    is_new = is_new_reg(tag, regid)
+    try:
+        reg = new_registers[regid] if is_new else registers[regid]
+    except KeyError:
+        raise Exception(f"Unknown {'new ' if is_new else ''}register {regid}" +\
+                        f"from '{tag}' with syntax '{semdict[tag]}'") from None
+    return reg.from_subtype(subtype)
 
 def helper_ret_type(tag, regs):
     ## If there is a scalar result, it is the return type
-- 
2.34.1

Re: [PATCH 1/6] target/hexagon: handle .new values

[Matheus Tavares Bernardino]

On Thu,  3 Apr 2025 19:51:58 -0700 Brian Cain <brian.cain@oss.qualcomm.com> wrote:
>
> From: Brian Cain <bcain@quicinc.com>

Perhaps it would be best to reset the autorship here to
brian.cain@oss.qualcomm.com?

> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
>  target/hexagon/hex_common.py | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
> index 758e5fd12d..242dee3731 100755
> --- a/target/hexagon/hex_common.py
> +++ b/target/hexagon/hex_common.py
> @@ -349,6 +349,12 @@ def helper_arg(self):
>              self.reg_tcg(),
>              f"{self.helper_arg_type()} {self.helper_arg_name()}"
>          )
> +    def from_subtype(self, subtype):
> +        if subtype == "":
> +            return self
> +        raise Exception(
> +            f"unknown subtype '{subtype}' on generic Register class")
> +

We use this method for other reg types downstream (HVX). Since, in this patch
series, we are not really using from_subtype (get_register is always called
with subtype == ""), I think we could either exclude it from this series or
evaluate how to also upstream its use for HVX.

Re: [PATCH 1/6] target/hexagon: handle .new values

[Brian Cain]

On 4/4/2025 8:25 AM, Matheus Tavares Bernardino wrote:
> On Thu,  3 Apr 2025 19:51:58 -0700 Brian Cain <brian.cain@oss.qualcomm.com> wrote:
>> From: Brian Cain <bcain@quicinc.com>
> Perhaps it would be best to reset the autorship here to
> brian.cain@oss.qualcomm.com?


Good catch -- will do.


>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> ---
>>   target/hexagon/hex_common.py | 27 ++++++++++++++++++++++-----
>>   1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
>> index 758e5fd12d..242dee3731 100755
>> --- a/target/hexagon/hex_common.py
>> +++ b/target/hexagon/hex_common.py
>> @@ -349,6 +349,12 @@ def helper_arg(self):
>>               self.reg_tcg(),
>>               f"{self.helper_arg_type()} {self.helper_arg_name()}"
>>           )
>> +    def from_subtype(self, subtype):
>> +        if subtype == "":
>> +            return self
>> +        raise Exception(
>> +            f"unknown subtype '{subtype}' on generic Register class")
>> +
> We use this method for other reg types downstream (HVX). Since, in this patch
> series, we are not really using from_subtype (get_register is always called
> with subtype == ""), I think we could either exclude it from this series or
> evaluate how to also upstream its use for HVX.

[PATCH 2/6] target/hexagon: Fix badva reference, delete CAUSE

[Brian Cain]

From: Brian Cain <bcain@quicinc.com>

The BADVA reg is referred to with the wrong identifier.  The
CAUSE reg field of SSR is not yet modeled.

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/cpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 766b678651..62f1fe15b8 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -216,8 +216,7 @@ static void hexagon_dump(CPUHexagonState *env, FILE *f, int flags)
     qemu_fprintf(f, "  cs0 = 0x00000000\n");
     qemu_fprintf(f, "  cs1 = 0x00000000\n");
 #else
-    print_reg(f, env, HEX_REG_CAUSE);
-    print_reg(f, env, HEX_REG_BADVA);
+    print_reg(f, env, HEX_SREG_BADVA);
     print_reg(f, env, HEX_REG_CS0);
     print_reg(f, env, HEX_REG_CS1);
 #endif
-- 
2.34.1

[PATCH 3/6] target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof

[Brian Cain]

From: Brian Cain <bcain@quicinc.com>

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/hex_common.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index 242dee3731..85eabc9876 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -247,8 +247,11 @@ def need_next_PC(tag):
 
 
 def need_pkt_has_multi_cof(tag):
-    return "A_COF" in attribdict[tag]
-
+    return (
+        "A_JUMP" in attribdict[tag]
+        or "A_CALL" in attribdict[tag]
+        or "J2_rte" == tag
+    ) and tag != "J2_hintjumpr"
 
 def need_pkt_need_commit(tag):
     return 'A_IMPLICIT_WRITES_USR' in attribdict[tag]
-- 
2.34.1

[PATCH 4/6] target/hexagon: s/pkt_has_store/pkt_has_scalar_store

[Brian Cain]

From: Brian Cain <bcain@quicinc.com>

To remove any confusion with HVX or other potential store instructions,
we'll qualify this context var with "scalar".

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/idef-parser/README.rst       | 2 +-
 target/hexagon/insn.h                       | 4 ++--
 target/hexagon/macros.h                     | 8 ++++----
 target/hexagon/decode.c                     | 4 ++--
 target/hexagon/genptr.c                     | 3 ++-
 target/hexagon/idef-parser/parser-helpers.c | 4 ++--
 target/hexagon/op_helper.c                  | 4 ++--
 target/hexagon/translate.c                  | 9 +++++----
 target/hexagon/gen_helper_funcs.py          | 2 +-
 9 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/target/hexagon/idef-parser/README.rst b/target/hexagon/idef-parser/README.rst
index 7199177ee3..235e3debee 100644
--- a/target/hexagon/idef-parser/README.rst
+++ b/target/hexagon/idef-parser/README.rst
@@ -637,7 +637,7 @@ tinycode for the Hexagon ``add`` instruction
 ::
 
    ---- 00021094
-   mov_i32 pkt_has_store_s1,$0x0
+   mov_i32 pkt_has_scalar_store_s1,$0x0
    add_i32 tmp0,r2,r2
    mov_i32 loc2,tmp0
    mov_i32 new_r1,loc2
diff --git a/target/hexagon/insn.h b/target/hexagon/insn.h
index 24dcf7fe9f..5d59430da9 100644
--- a/target/hexagon/insn.h
+++ b/target/hexagon/insn.h
@@ -66,8 +66,8 @@ struct Packet {
 
     bool pkt_has_dczeroa;
 
-    bool pkt_has_store_s0;
-    bool pkt_has_store_s1;
+    bool pkt_has_scalar_store_s0;
+    bool pkt_has_scalar_store_s1;
 
     bool pkt_has_hvx;
     Insn *vhist_insn;
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index ee3d4c88e7..b6e5c8aae2 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -82,7 +82,7 @@
  */
 #define CHECK_NOSHUF(VA, SIZE) \
     do { \
-        if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
+        if (insn->slot == 0 && ctx->pkt->pkt_has_scalar_store_s1) { \
             probe_noshuf_load(VA, SIZE, ctx->mem_idx); \
             process_store(ctx, 1); \
         } \
@@ -93,11 +93,11 @@
         TCGLabel *noshuf_label = gen_new_label(); \
         tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
         GET_EA; \
-        if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
+        if (insn->slot == 0 && ctx->pkt->pkt_has_scalar_store_s1) { \
             probe_noshuf_load(EA, SIZE, ctx->mem_idx); \
         } \
         gen_set_label(noshuf_label); \
-        if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
+        if (insn->slot == 0 && ctx->pkt->pkt_has_scalar_store_s1) { \
             process_store(ctx, 1); \
         } \
     } while (0)
@@ -524,7 +524,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int shift)
 
 #define fLOAD(NUM, SIZE, SIGN, EA, DST) \
     do { \
-        check_noshuf(env, pkt_has_store_s1, slot, EA, SIZE, GETPC()); \
+        check_noshuf(env, pkt_has_scalar_store_s1, slot, EA, SIZE, GETPC()); \
         DST = (size##SIZE##SIGN##_t)MEM_LOAD##SIZE(env, EA, GETPC()); \
     } while (0)
 #endif
diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index 23deba2426..b5ece60450 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -236,9 +236,9 @@ static void decode_set_insn_attr_fields(Packet *pkt)
             if (GET_ATTRIB(opcode, A_SCALAR_STORE) &&
                 !GET_ATTRIB(opcode, A_MEMSIZE_0B)) {
                 if (pkt->insn[i].slot == 0) {
-                    pkt->pkt_has_store_s0 = true;
+                    pkt->pkt_has_scalar_store_s0 = true;
                 } else {
-                    pkt->pkt_has_store_s1 = true;
+                    pkt->pkt_has_scalar_store_s1 = true;
                 }
             }
         }
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 2c5e15cfcf..7c73772e40 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -395,7 +395,8 @@ static inline void gen_store_conditional8(DisasContext *ctx,
 #ifndef CONFIG_HEXAGON_IDEF_PARSER
 static TCGv gen_slotval(DisasContext *ctx)
 {
-    int slotval = (ctx->pkt->pkt_has_store_s1 & 1) | (ctx->insn->slot << 1);
+    int slotval =
+        (ctx->pkt->pkt_has_scalar_store_s1 & 1) | (ctx->insn->slot << 1);
     return tcg_constant_tl(slotval);
 }
 #endif
diff --git a/target/hexagon/idef-parser/parser-helpers.c b/target/hexagon/idef-parser/parser-helpers.c
index a7dcd85fe4..3316c230f8 100644
--- a/target/hexagon/idef-parser/parser-helpers.c
+++ b/target/hexagon/idef-parser/parser-helpers.c
@@ -1725,7 +1725,7 @@ void gen_cancel(Context *c, YYLTYPE *locp)
 
 void gen_load_cancel(Context *c, YYLTYPE *locp)
 {
-    OUT(c, locp, "if (insn->slot == 0 && pkt->pkt_has_store_s1) {\n");
+    OUT(c, locp, "if (insn->slot == 0 && pkt->pkt_has_scalar_store_s1) {\n");
     OUT(c, locp, "ctx->s1_store_processed = false;\n");
     OUT(c, locp, "process_store(ctx, 1);\n");
     OUT(c, locp, "}\n");
@@ -1750,7 +1750,7 @@ void gen_load(Context *c, YYLTYPE *locp, HexValue *width,
 
     /* Lookup the effective address EA */
     find_variable(c, locp, ea, ea);
-    OUT(c, locp, "if (insn->slot == 0 && pkt->pkt_has_store_s1) {\n");
+    OUT(c, locp, "if (insn->slot == 0 && pkt->pkt_has_scalar_store_s1) {\n");
     OUT(c, locp, "probe_noshuf_load(", ea, ", ", width, ", ctx->mem_idx);\n");
     OUT(c, locp, "process_store(ctx, 1);\n");
     OUT(c, locp, "}\n");
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 6da8db8ea5..6ff37680d9 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -463,11 +463,11 @@ void HELPER(probe_pkt_scalar_hvx_stores)(CPUHexagonState *env, int mask)
  * If the load is in slot 0 and there is a store in slot1 (that
  * wasn't cancelled), we have to do the store first.
  */
-static void check_noshuf(CPUHexagonState *env, bool pkt_has_store_s1,
+static void check_noshuf(CPUHexagonState *env, bool pkt_has_scalar_store_s1,
                          uint32_t slot, target_ulong vaddr, int size,
                          uintptr_t ra)
 {
-    if (slot == 0 && pkt_has_store_s1 &&
+    if (slot == 0 && pkt_has_scalar_store_s1 &&
         ((env->slot_cancelled & (1 << 1)) == 0)) {
         probe_read(env, vaddr, size, MMU_USER_IDX, ra);
         commit_store(env, 1, ra);
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 5271c4e022..aca77dfdb1 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -705,11 +705,11 @@ static void process_store_log(DisasContext *ctx)
      *  the memory accesses overlap.
      */
     Packet *pkt = ctx->pkt;
-    if (pkt->pkt_has_store_s1) {
+    if (pkt->pkt_has_scalar_store_s1) {
         g_assert(!pkt->pkt_has_dczeroa);
         process_store(ctx, 1);
     }
-    if (pkt->pkt_has_store_s0) {
+    if (pkt->pkt_has_scalar_store_s0) {
         g_assert(!pkt->pkt_has_dczeroa);
         process_store(ctx, 0);
     }
@@ -834,8 +834,9 @@ static void gen_commit_packet(DisasContext *ctx)
      * involved in committing the packet.
      */
     Packet *pkt = ctx->pkt;
-    bool has_store_s0 = pkt->pkt_has_store_s0;
-    bool has_store_s1 = (pkt->pkt_has_store_s1 && !ctx->s1_store_processed);
+    bool has_store_s0 = pkt->pkt_has_scalar_store_s0;
+    bool has_store_s1 =
+        (pkt->pkt_has_scalar_store_s1 && !ctx->s1_store_processed);
     bool has_hvx_store = pkt_has_hvx_store(pkt);
     if (pkt->pkt_has_dczeroa) {
         /*
diff --git a/target/hexagon/gen_helper_funcs.py b/target/hexagon/gen_helper_funcs.py
index c1f806ac4b..a9c0e27a80 100755
--- a/target/hexagon/gen_helper_funcs.py
+++ b/target/hexagon/gen_helper_funcs.py
@@ -69,7 +69,7 @@ def gen_helper_function(f, tag, tagregs, tagimms):
     if hex_common.need_slot(tag):
         if "A_LOAD" in hex_common.attribdict[tag]:
             f.write(hex_common.code_fmt(f"""\
-                bool pkt_has_store_s1 = slotval & 0x1;
+                bool pkt_has_scalar_store_s1 = slotval & 0x1;
             """))
         f.write(hex_common.code_fmt(f"""\
             uint32_t slot = slotval >> 1;
-- 
2.34.1

[PATCH 5/6] target/hexagon: Remove unreachable

[Brian Cain]

From: Brian Cain <bcain@quicinc.com>

We should raise an exception in the event that we encounter a packet
that can't be correctly decoded, not fault.

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/decode.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index b5ece60450..1db7f1950f 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -489,7 +489,6 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
             insn->iclass = iclass_bits(encoding);
             return 1;
         }
-        g_assert_not_reached();
     } else {
         uint32_t iclass = get_duplex_iclass(encoding);
         unsigned int slot0_subinsn = get_slot0_subinsn(encoding);
@@ -512,6 +511,11 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
         }
         g_assert_not_reached();
     }
+    /*
+     * invalid/unrecognized opcode; return 1 and let gen_insn() raise an
+     * exception when it sees this empty insn.
+     */
+    return 1;
 }
 
 static void decode_add_endloop_insn(Insn *insn, int loopnum)
-- 
2.34.1

[PATCH 6/6] target/hexagon: Add memory order definition

[Brian Cain]

From: Brian Cain <bcain@quicinc.com>

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/cpu-param.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/hexagon/cpu-param.h b/target/hexagon/cpu-param.h
index 45ee7b4640..ccaf6a9d28 100644
--- a/target/hexagon/cpu-param.h
+++ b/target/hexagon/cpu-param.h
@@ -23,4 +23,9 @@
 #define TARGET_PHYS_ADDR_SPACE_BITS 36
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
+/*
+ * Hexagon processors have a strong memory model.
+ */
+#define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL)
+
 #endif
-- 
2.34.1

Re: [PATCH 6/6] target/hexagon: Add memory order definition

[Richard Henderson]

On 4/3/25 19:52, Brian Cain wrote:
> From: Brian Cain <bcain@quicinc.com>
> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
>   target/hexagon/cpu-param.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/hexagon/cpu-param.h b/target/hexagon/cpu-param.h
> index 45ee7b4640..ccaf6a9d28 100644
> --- a/target/hexagon/cpu-param.h
> +++ b/target/hexagon/cpu-param.h
> @@ -23,4 +23,9 @@
>   #define TARGET_PHYS_ADDR_SPACE_BITS 36
>   #define TARGET_VIRT_ADDR_SPACE_BITS 32
>   
> +/*
> + * Hexagon processors have a strong memory model.
> + */
> +#define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL)
> +
>   #endif

Excellent, then we have that covered with

https://patchew.org/QEMU/20250321181549.3331-1-philmd@linaro.org/20250321181549.3331-2-philmd@linaro.org/

and the follow-up

https://patchew.org/QEMU/20250321181549.3331-1-philmd@linaro.org/20250321181549.3331-8-philmd@linaro.org/

which moves that macro to a field in TCGCPUOps.


r~

Re: [PATCH 6/6] target/hexagon: Add memory order definition

[Brian Cain]

On 4/4/2025 9:33 AM, Richard Henderson wrote:
> On 4/3/25 19:52, Brian Cain wrote:
>> From: Brian Cain <bcain@quicinc.com>
>>
>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> ---
>>   target/hexagon/cpu-param.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/hexagon/cpu-param.h b/target/hexagon/cpu-param.h
>> index 45ee7b4640..ccaf6a9d28 100644
>> --- a/target/hexagon/cpu-param.h
>> +++ b/target/hexagon/cpu-param.h
>> @@ -23,4 +23,9 @@
>>   #define TARGET_PHYS_ADDR_SPACE_BITS 36
>>   #define TARGET_VIRT_ADDR_SPACE_BITS 32
>>   +/*
>> + * Hexagon processors have a strong memory model.
>> + */
>> +#define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL)
>> +
>>   #endif
>
> Excellent, then we have that covered with
>
> https://patchew.org/QEMU/20250321181549.3331-1-philmd@linaro.org/20250321181549.3331-2-philmd@linaro.org/ 
>
>
> and the follow-up
>
> https://patchew.org/QEMU/20250321181549.3331-1-philmd@linaro.org/20250321181549.3331-8-philmd@linaro.org/ 
>
>
> which moves that macro to a field in TCGCPUOps.
>

Oh, I see -- thanks!  Will drop this patch from my series, then.


>
> r~