diff mbox series

[2/2] target/hexagon: rename aliased register HEX_REG_P3_0

Message ID 20221227153447.2729-3-quic_mthiyaga@quicinc.com
State New
Headers show
Series Hexagon: fix signal context save & restore bug | expand

Commit Message

Mukilan Thiyagarajan (QUIC) Dec. 27, 2022, 3:34 p.m. UTC
The patch renames the identifier of the 32bit register
HEX_REG_P3_0 to HEX_REG_P3_0_ALIASED.

This change is to intended to provide some warning that
HEX_REG_P3_0 is an aliased register which has multiple
representations in CPU state and therefore might require
special handling in some contexts. The hope is to prevent
accidental misuse of this register e.g the issue reported
for the signals tests failure [here][1].

[1]: https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html

Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
---
 target/hexagon/cpu.c      |  6 +++---
 target/hexagon/genptr.c   | 12 ++++++------
 target/hexagon/hex_regs.h |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

Comments

Taylor Simpson Dec. 28, 2022, 6:05 p.m. UTC | #1
> -----Original Message-----
> From: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>
> Sent: Tuesday, December 27, 2022 9:35 AM
> To: qemu-devel@nongnu.org; Taylor Simpson <tsimpson@quicinc.com>;
> laurent@vivier.eu
> Cc: Brian Cain <bcain@quicinc.com>; richard.henderson@linaro.org;
> alex.bennee@linaro.org; Mukilan Thiyagarajan (QUIC)
> <quic_mthiyaga@quicinc.com>
> Subject: [PATCH 2/2] target/hexagon: rename aliased register
> HEX_REG_P3_0
> 
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> 658ca4ff78..807037c586 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -86,7 +86,7 @@ static target_ulong adjust_stack_ptrs(CPUHexagonState
> *env, target_ulong addr)
>      return addr;
>  }
> 
> -/* HEX_REG_P3_0 (aka C4) is an alias for the predicate registers */
> +/* HEX_REG_P3_0_ALIASED (aka C4) is an alias for the predicate
> +registers */

Not sure why you broke this comment into two lines, but ...
/*
 * Multiline comments should be
 * formatted like this
 */

Otherwise
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Mukilan Thiyagarajan (QUIC) Dec. 29, 2022, 9:30 a.m. UTC | #2
> Not sure why you broke this comment into two lines, but ...

I'm not sure if this issue is on my end or your mail client, but the 
formatting appears to be correct in the patchew:
https://patchew.org/QEMU/20221227153447.2729-1-quic._5Fmthiyaga@quicinc.com/20221227153447.2729-3-quic._5Fmthiyaga@quicinc.com/

Please let me know if the formatting is still off in v2 of the patch for you.

Thanks,
Mukilan

-----Original Message-----
From: Taylor Simpson <tsimpson@quicinc.com> 
Sent: Wednesday, December 28, 2022 11:35 PM
To: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>; qemu-devel@nongnu.org; laurent@vivier.eu
Cc: Brian Cain <bcain@quicinc.com>; richard.henderson@linaro.org; alex.bennee@linaro.org
Subject: RE: [PATCH 2/2] target/hexagon: rename aliased register HEX_REG_P3_0



> -----Original Message-----
> From: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>
> Sent: Tuesday, December 27, 2022 9:35 AM
> To: qemu-devel@nongnu.org; Taylor Simpson <tsimpson@quicinc.com>; 
> laurent@vivier.eu
> Cc: Brian Cain <bcain@quicinc.com>; richard.henderson@linaro.org; 
> alex.bennee@linaro.org; Mukilan Thiyagarajan (QUIC) 
> <quic_mthiyaga@quicinc.com>
> Subject: [PATCH 2/2] target/hexagon: rename aliased register
> HEX_REG_P3_0
> 
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> 658ca4ff78..807037c586 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -86,7 +86,7 @@ static target_ulong 
> adjust_stack_ptrs(CPUHexagonState *env, target_ulong addr)
>      return addr;
>  }
> 
> -/* HEX_REG_P3_0 (aka C4) is an alias for the predicate registers */
> +/* HEX_REG_P3_0_ALIASED (aka C4) is an alias for the predicate 
> +registers */

Not sure why you broke this comment into two lines, but ...
/*
 * Multiline comments should be
 * formatted like this
 */

Otherwise
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
diff mbox series

Patch

diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 658ca4ff78..807037c586 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -86,7 +86,7 @@  static target_ulong adjust_stack_ptrs(CPUHexagonState *env, target_ulong addr)
     return addr;
 }
 
-/* HEX_REG_P3_0 (aka C4) is an alias for the predicate registers */
+/* HEX_REG_P3_0_ALIASED (aka C4) is an alias for the predicate registers */
 static target_ulong read_p3_0(CPUHexagonState *env)
 {
     int32_t control_reg = 0;
@@ -102,7 +102,7 @@  static void print_reg(FILE *f, CPUHexagonState *env, int regnum)
 {
     target_ulong value;
 
-    if (regnum == HEX_REG_P3_0) {
+    if (regnum == HEX_REG_P3_0_ALIASED) {
         value = read_p3_0(env);
     } else {
         value = regnum < 32 ? adjust_stack_ptrs(env, env->gpr[regnum])
@@ -198,7 +198,7 @@  static void hexagon_dump(CPUHexagonState *env, FILE *f, int flags)
     print_reg(f, env, HEX_REG_M0);
     print_reg(f, env, HEX_REG_M1);
     print_reg(f, env, HEX_REG_USR);
-    print_reg(f, env, HEX_REG_P3_0);
+    print_reg(f, env, HEX_REG_P3_0_ALIASED);
     print_reg(f, env, HEX_REG_GP);
     print_reg(f, env, HEX_REG_UGP);
     print_reg(f, env, HEX_REG_PC);
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 6cf2e0ed43..66a968c884 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -163,7 +163,7 @@  static inline void gen_read_p3_0(TCGv control_reg)
 
 /*
  * Certain control registers require special handling on read
- *     HEX_REG_P3_0          aliased to the predicate registers
+ *     HEX_REG_P3_0_ALIASED  aliased to the predicate registers
  *                           -> concat the 4 predicate registers together
  *     HEX_REG_PC            actual value stored in DisasContext
  *                           -> assign from ctx->base.pc_next
@@ -173,7 +173,7 @@  static inline void gen_read_p3_0(TCGv control_reg)
 static inline void gen_read_ctrl_reg(DisasContext *ctx, const int reg_num,
                                      TCGv dest)
 {
-    if (reg_num == HEX_REG_P3_0) {
+    if (reg_num == HEX_REG_P3_0_ALIASED) {
         gen_read_p3_0(dest);
     } else if (reg_num == HEX_REG_PC) {
         tcg_gen_movi_tl(dest, ctx->base.pc_next);
@@ -194,7 +194,7 @@  static inline void gen_read_ctrl_reg(DisasContext *ctx, const int reg_num,
 static inline void gen_read_ctrl_reg_pair(DisasContext *ctx, const int reg_num,
                                           TCGv_i64 dest)
 {
-    if (reg_num == HEX_REG_P3_0) {
+    if (reg_num == HEX_REG_P3_0_ALIASED) {
         TCGv p3_0 = tcg_temp_new();
         gen_read_p3_0(p3_0);
         tcg_gen_concat_i32_i64(dest, p3_0, hex_gpr[reg_num + 1]);
@@ -238,7 +238,7 @@  static void gen_write_p3_0(DisasContext *ctx, TCGv control_reg)
 
 /*
  * Certain control registers require special handling on write
- *     HEX_REG_P3_0          aliased to the predicate registers
+ *     HEX_REG_P3_0_ALIASED  aliased to the predicate registers
  *                           -> break the value across 4 predicate registers
  *     HEX_REG_QEMU_*_CNT    changes in current TB in DisasContext
  *                            -> clear the changes
@@ -246,7 +246,7 @@  static void gen_write_p3_0(DisasContext *ctx, TCGv control_reg)
 static inline void gen_write_ctrl_reg(DisasContext *ctx, int reg_num,
                                       TCGv val)
 {
-    if (reg_num == HEX_REG_P3_0) {
+    if (reg_num == HEX_REG_P3_0_ALIASED) {
         gen_write_p3_0(ctx, val);
     } else {
         gen_log_reg_write(reg_num, val);
@@ -266,7 +266,7 @@  static inline void gen_write_ctrl_reg(DisasContext *ctx, int reg_num,
 static inline void gen_write_ctrl_reg_pair(DisasContext *ctx, int reg_num,
                                            TCGv_i64 val)
 {
-    if (reg_num == HEX_REG_P3_0) {
+    if (reg_num == HEX_REG_P3_0_ALIASED) {
         TCGv val32 = tcg_temp_new();
         tcg_gen_extrl_i64_i32(val32, val);
         gen_write_p3_0(ctx, val32);
diff --git a/target/hexagon/hex_regs.h b/target/hexagon/hex_regs.h
index a63c2c0fd5..bddfc28021 100644
--- a/target/hexagon/hex_regs.h
+++ b/target/hexagon/hex_regs.h
@@ -58,7 +58,7 @@  enum {
     HEX_REG_LC0              = 33,
     HEX_REG_SA1              = 34,
     HEX_REG_LC1              = 35,
-    HEX_REG_P3_0             = 36,
+    HEX_REG_P3_0_ALIASED     = 36,
     HEX_REG_M0               = 38,
     HEX_REG_M1               = 39,
     HEX_REG_USR              = 40,