diff mbox series

Hexagon (target/hexagon) remove unused functions

Message ID 1619667142-29636-1-git-send-email-tsimpson@quicinc.com
State New
Headers show
Series Hexagon (target/hexagon) remove unused functions | expand

Commit Message

Taylor Simpson April 29, 2021, 3:32 a.m. UTC 
Remove gen_read_reg and gen_set_byte

Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/genptr.c | 11 -----------
 1 file changed, 11 deletions(-)

Comments

Philippe Mathieu-Daudé April 29, 2021, 4:49 a.m. UTC | #1 
Hi Taylor,

On 4/29/21 5:32 AM, Taylor Simpson wrote:
> Remove gen_read_reg and gen_set_byte
> 
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---

To help git-tools (and reviewers), please use the 'Based-on' tag
the next time you send a patch depending on another one:
Based-on: <1617930474-31979-1-git-send-email-tsimpson@quicinc.com>

>  target/hexagon/genptr.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index 55c7cd8..f93f895 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -28,12 +28,6 @@
>  #undef QEMU_GENERATE
>  #include "gen_tcg.h"
>  
> -static inline TCGv gen_read_reg(TCGv result, int num)
> -{
> -    tcg_gen_mov_tl(result, hex_gpr[num]);
> -    return result;
> -}

But what about:

target/hexagon/macros.h:26:#define READ_REG(dest, NUM)
gen_read_reg(dest, NUM)
target/hexagon/macros.h:29:#define READ_REG(NUM)
(env->gpr[(NUM)])
target/hexagon/macros.h:360:#define fREAD_LR() (READ_REG(HEX_REG_LR))
target/hexagon/macros.h:366:#define fREAD_SP() (READ_REG(HEX_REG_SP))
target/hexagon/macros.h:367:#define fREAD_LC0 (READ_REG(HEX_REG_LC0))
target/hexagon/macros.h:368:#define fREAD_LC1 (READ_REG(HEX_REG_LC1))
target/hexagon/macros.h:369:#define fREAD_SA0 (READ_REG(HEX_REG_SA0))
target/hexagon/macros.h:370:#define fREAD_SA1 (READ_REG(HEX_REG_SA1))
target/hexagon/macros.h:371:#define fREAD_FP() (READ_REG(HEX_REG_FP))
target/hexagon/macros.h:375:    (insn->extension_valid ? 0 :
READ_REG(HEX_REG_GP))
target/hexagon/macros.h:377:#define fREAD_GP() READ_REG(HEX_REG_GP)
target/hexagon/macros.h:379:#define fREAD_PC() (READ_REG(HEX_REG_PC))
target/hexagon/macros.h:577:#define fGET_FRAMEKEY()
READ_REG(HEX_REG_FRAMEKEY)

and:

target/hexagon/genptr.c:37:static inline TCGv gen_read_preg(TCGv pred,
uint8_t num)
target/hexagon/macros.h:27:#define READ_PREG(dest, NUM)
gen_read_preg(dest, (NUM))

I'd rather send a "!fixup Hexagon (target/hexagon) circular addressing"
patch (so Richard squashes it there) with:

-- >8 --
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index b726c3b7917..bf0e5ae92bb 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -22,16 +22,11 @@
 #include "hex_regs.h"
 #include "reg_fields.h"

-#ifdef QEMU_GENERATE
-#define READ_REG(dest, NUM)              gen_read_reg(dest, NUM)
-#define READ_PREG(dest, NUM)             gen_read_preg(dest, (NUM))
-#else
 #define READ_REG(NUM)                    (env->gpr[(NUM)])
 #define READ_PREG(NUM)                   (env->pred[NUM])

 #define WRITE_RREG(NUM, VAL)             log_reg_write(env, NUM, VAL, slot)
 #define WRITE_PREG(NUM, VAL)             log_pred_write(env, NUM, VAL)
-#endif

 #define PCALIGN 4
 #define PCALIGN_MASK (PCALIGN - 1)
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 55c7cd86a4e..42f25f6f462 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -28,18 +28,6 @@
 #undef QEMU_GENERATE
 #include "gen_tcg.h"

-static inline TCGv gen_read_reg(TCGv result, int num)
-{
-    tcg_gen_mov_tl(result, hex_gpr[num]);
-    return result;
-}
-
-static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
-{
-    tcg_gen_mov_tl(pred, hex_pred[num]);
-    return pred;
-}
-
 static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int
slot)
 {
     TCGv zero = tcg_const_tl(0);
@@ -319,11 +307,6 @@ static inline void gen_set_half_i64(int N, TCGv_i64
result, TCGv src)
     tcg_temp_free_i64(src64);
 }

-static inline void gen_set_byte(int N, TCGv result, TCGv src)
-{
-    tcg_gen_deposit_tl(result, result, src, N * 8, 8);
-}
-
 static void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src)
 {
     TCGv_i64 src64 = tcg_temp_new_i64();
---

NB: untested :)

Regards,

Phil.
Philippe Mathieu-Daudé April 29, 2021, 5:13 a.m. UTC | #2 
On Thu, Apr 29, 2021 at 6:49 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Taylor,
>
> On 4/29/21 5:32 AM, Taylor Simpson wrote:
> > Remove gen_read_reg and gen_set_byte
> >
> > Reported-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> > ---
>
> To help git-tools (and reviewers), please use the 'Based-on' tag
> the next time you send a patch depending on another one:
> Based-on: <1617930474-31979-1-git-send-email-tsimpson@quicinc.com>

Sorry I forgot to link the explanation:
https://wiki.qemu.org/Contribute/SubmitAPatch#Base_patches_against_current_git_master

>
> >  target/hexagon/genptr.c | 11 -----------
> >  1 file changed, 11 deletions(-)
Taylor Simpson April 29, 2021, 5:25 a.m. UTC | #3 
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Wednesday, April 28, 2021 11:49 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; ale@rev.ng; Brian Cain
> <bcain@quicinc.com>
> Subject: Re: [PATCH] Hexagon (target/hexagon) remove unused functions
> 
> Hi Taylor,
> 
> On 4/29/21 5:32 AM, Taylor Simpson wrote:
> > Remove gen_read_reg and gen_set_byte
> >
> > Reported-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> > ---
> 
> To help git-tools (and reviewers), please use the 'Based-on' tag
> the next time you send a patch depending on another one:
> Based-on: <1617930474-31979-1-git-send-email-tsimpson@quicinc.com>
> 
> >  target/hexagon/genptr.c | 11 -----------
> >  1 file changed, 11 deletions(-)
> >
> > diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> > index 55c7cd8..f93f895 100644
> > --- a/target/hexagon/genptr.c
> > +++ b/target/hexagon/genptr.c
> > @@ -28,12 +28,6 @@
> >  #undef QEMU_GENERATE
> >  #include "gen_tcg.h"
> >
> > -static inline TCGv gen_read_reg(TCGv result, int num)
> > -{
> > -    tcg_gen_mov_tl(result, hex_gpr[num]);
> > -    return result;
> > -}
> 
> But what about:
> 
> target/hexagon/macros.h:26:#define READ_REG(dest, NUM)
> gen_read_reg(dest, NUM)

I should remove this to avoid confusion.

> target/hexagon/macros.h:29:#define READ_REG(NUM)
> (env->gpr[(NUM)])
> target/hexagon/macros.h:360:#define fREAD_LR()
> (READ_REG(HEX_REG_LR))
> target/hexagon/macros.h:366:#define fREAD_SP()
> (READ_REG(HEX_REG_SP))
> target/hexagon/macros.h:367:#define fREAD_LC0
> (READ_REG(HEX_REG_LC0))
> target/hexagon/macros.h:368:#define fREAD_LC1
> (READ_REG(HEX_REG_LC1))
> target/hexagon/macros.h:369:#define fREAD_SA0
> (READ_REG(HEX_REG_SA0))
> target/hexagon/macros.h:370:#define fREAD_SA1
> (READ_REG(HEX_REG_SA1))
> target/hexagon/macros.h:371:#define fREAD_FP()
> (READ_REG(HEX_REG_FP))
> target/hexagon/macros.h:375:    (insn->extension_valid ? 0 :
> READ_REG(HEX_REG_GP))
> target/hexagon/macros.h:377:#define fREAD_GP()
> READ_REG(HEX_REG_GP)
> target/hexagon/macros.h:379:#define fREAD_PC()
> (READ_REG(HEX_REG_PC))
> target/hexagon/macros.h:577:#define fGET_FRAMEKEY()
> READ_REG(HEX_REG_FRAMEKEY)

These are not guarded by QEMU_GENERATE, so they are needed and reference the other version of READ_REG

> and:
> 
> target/hexagon/genptr.c:37:static inline TCGv gen_read_preg(TCGv pred,
> uint8_t num)
> target/hexagon/macros.h:27:#define READ_PREG(dest, NUM)
> gen_read_preg(dest, (NUM))

This is needed.  It reads a predicate register.  The gen_read_reg functions reads a general purpose register.

> I'd rather send a "!fixup Hexagon (target/hexagon) circular addressing"
> patch (so Richard squashes it there) with:

Richard said he could cherry pick a single patch.
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg05968.html

> 
> -- >8 --
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> index b726c3b7917..bf0e5ae92bb 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -22,16 +22,11 @@
>  #include "hex_regs.h"
>  #include "reg_fields.h"
> 
> -#ifdef QEMU_GENERATE
> -#define READ_REG(dest, NUM)              gen_read_reg(dest, NUM)
> -#define READ_PREG(dest, NUM)             gen_read_preg(dest, (NUM))
> -#else
>  #define READ_REG(NUM)                    (env->gpr[(NUM)])
>  #define READ_PREG(NUM)                   (env->pred[NUM])
> 
>  #define WRITE_RREG(NUM, VAL)             log_reg_write(env, NUM, VAL, slot)
>  #define WRITE_PREG(NUM, VAL)             log_pred_write(env, NUM, VAL)
> -#endif
> 
>  #define PCALIGN 4
>  #define PCALIGN_MASK (PCALIGN - 1)
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index 55c7cd86a4e..42f25f6f462 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -28,18 +28,6 @@
>  #undef QEMU_GENERATE
>  #include "gen_tcg.h"
> 
> -static inline TCGv gen_read_reg(TCGv result, int num)
> -{
> -    tcg_gen_mov_tl(result, hex_gpr[num]);
> -    return result;
> -}
> -
> -static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
> -{
> -    tcg_gen_mov_tl(pred, hex_pred[num]);
> -    return pred;
> -}
> -
>  static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int
> slot)
>  {
>      TCGv zero = tcg_const_tl(0);
> @@ -319,11 +307,6 @@ static inline void gen_set_half_i64(int N, TCGv_i64
> result, TCGv src)
>      tcg_temp_free_i64(src64);
>  }
> 
> -static inline void gen_set_byte(int N, TCGv result, TCGv src)
> -{
> -    tcg_gen_deposit_tl(result, result, src, N * 8, 8);
> -}
> -
>  static void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src)
>  {
>      TCGv_i64 src64 = tcg_temp_new_i64();
> ---
> 
> NB: untested :)
> 
> Regards,
> 
> Phil.
diff mbox series

Patch

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 55c7cd8..f93f895 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -28,12 +28,6 @@ 
 #undef QEMU_GENERATE
 #include "gen_tcg.h"
 
-static inline TCGv gen_read_reg(TCGv result, int num)
-{
-    tcg_gen_mov_tl(result, hex_gpr[num]);
-    return result;
-}
-
 static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
 {
     tcg_gen_mov_tl(pred, hex_pred[num]);
@@ -319,11 +313,6 @@  static inline void gen_set_half_i64(int N, TCGv_i64 result, TCGv src)
     tcg_temp_free_i64(src64);
 }
 
-static inline void gen_set_byte(int N, TCGv result, TCGv src)
-{
-    tcg_gen_deposit_tl(result, result, src, N * 8, 8);
-}
-
 static void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src)
 {
     TCGv_i64 src64 = tcg_temp_new_i64();