Patchwork tcg/arm: Remove unused tcg_out_addi()

login
register
mail settings
Submitter Peter Maydell
Date Sept. 12, 2011, 10:03 a.m.
Message ID <1315821825-27492-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/114306/
State New
Headers show

Comments

Peter Maydell - Sept. 12, 2011, 10:03 a.m.
Remove the unused function tcg_out_addi() from the ARM TCG backend;
this fixes a compilation failure on ARM hosts with newer gcc.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
A previous patch from Richard Henderson for this compile failure:
 http://patchwork.ozlabs.org/patch/110400/
was rejected, so here's another go. This simply removes the unused function,
in line with the approach taken for ppc/ppc64 in commits 1a2eb162414 and
c24a9c6ef94.

If this is accepted I can do the equivalent patches for tcg/ia64 and tcg/s390
(although those don't cause compile failures because the unused function
happens to be marked 'inline'.)

 tcg/arm/tcg-target.c |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)
Stefan Weil - Sept. 12, 2011, 5:04 p.m.
Am 12.09.2011 12:03, schrieb Peter Maydell:
> Remove the unused function tcg_out_addi() from the ARM TCG backend;
> this fixes a compilation failure on ARM hosts with newer gcc.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> A previous patch from Richard Henderson for this compile failure:
> http://patchwork.ozlabs.org/patch/110400/
> was rejected, so here's another go. This simply removes the unused 
> function,
> in line with the approach taken for ppc/ppc64 in commits 1a2eb162414 and
> c24a9c6ef94.
>
> If this is accepted I can do the equivalent patches for tcg/ia64 and 
> tcg/s390
> (although those don't cause compile failures because the unused function
> happens to be marked 'inline'.)
>
> tcg/arm/tcg-target.c | 15 ---------------
> 1 files changed, 0 insertions(+), 15 deletions(-)
>

Are you sure that those functions will never be needed again?
If yes, your solution is ok. I was not sure - that's the reason
why I had sent a patch using inline. #if 0 ... #endif and
a comment might also be a solution for possible
future usage.

Stefan
Peter Maydell - Sept. 12, 2011, 5:30 p.m.
On 12 September 2011 18:04, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 12.09.2011 12:03, schrieb Peter Maydell:
>> Remove the unused function tcg_out_addi() from the ARM TCG backend;
>> this fixes a compilation failure on ARM hosts with newer gcc.

> Are you sure that those functions will never be needed again?
> If yes, your solution is ok. I was not sure - that's the reason
> why I had sent a patch using inline. #if 0 ... #endif and
> a comment might also be a solution for possible
> future usage.

Well, if we do need it again we'll have to resurrect the ppc version.
I dislike #if 0...#endif code.

I think the underlying problem here is that we don't have a well
defined API which the tcg backends have to provide to the generic
tcg code. So we can't really say whether tcg_out_addi() ought to
be part of it or not. (Ideally all the functions provided by the
backend ought to have a consistent naming scheme, eg tcg_target_*,
but sadly this isn't the case. And the way tcg.c #includes the
backend means you can't use presence/absence of 'static' to
identify the exposed API either... Does that really gain us much,
or should we just build tcg-target.c as a separate translation unit?)

I think I would argue that tcg.c ought to do addition via tcg_out_op,
since the target has to provide addition via that route anyway.
That would be cleaner than requiring backends to implement an extra
interface for "emit code to add things together". If you buy this
line of reasoning then deleting the current tcg_out_addi() on
platforms that don't need it internally anyhow is clearly the
Right Thing :-)

-- PMM
Richard Henderson - Sept. 14, 2011, 7:34 p.m.
On 09/12/2011 03:03 AM, Peter Maydell wrote:
> Remove the unused function tcg_out_addi() from the ARM TCG backend;
> this fixes a compilation failure on ARM hosts with newer gcc.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <rth@twiddle.net>

> ---
> A previous patch from Richard Henderson for this compile failure:
>  http://patchwork.ozlabs.org/patch/110400/
> was rejected, so here's another go. This simply removes the unused function,
> in line with the approach taken for ppc/ppc64 in commits 1a2eb162414 and
> c24a9c6ef94.
> 
> If this is accepted I can do the equivalent patches for tcg/ia64 and tcg/s390
> (although those don't cause compile failures because the unused function
> happens to be marked 'inline'.)

If we ever need this function again, we can revive it from git.
I'm in favor of deleting all of these functions.


r~
Peter Maydell - Sept. 23, 2011, 10:07 a.m.
Ping? Would be nice to get this committed since it's a compile failure
fix...

-- PMM

On 12 September 2011 11:03, Peter Maydell <peter.maydell@linaro.org> wrote:
> Remove the unused function tcg_out_addi() from the ARM TCG backend;
> this fixes a compilation failure on ARM hosts with newer gcc.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> A previous patch from Richard Henderson for this compile failure:
>  http://patchwork.ozlabs.org/patch/110400/
> was rejected, so here's another go. This simply removes the unused function,
> in line with the approach taken for ppc/ppc64 in commits 1a2eb162414 and
> c24a9c6ef94.
>
> If this is accepted I can do the equivalent patches for tcg/ia64 and tcg/s390
> (although those don't cause compile failures because the unused function
> happens to be marked 'inline'.)
>
>  tcg/arm/tcg-target.c |   15 ---------------
>  1 files changed, 0 insertions(+), 15 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index 93eb0f1..ce4760d 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -1820,21 +1820,6 @@ static inline void tcg_out_st(TCGContext *s, TCGType type, int arg,
>     tcg_out_st32(s, COND_AL, arg, arg1, arg2);
>  }
>
> -static void tcg_out_addi(TCGContext *s, int reg, tcg_target_long val)
> -{
> -    if (val > 0)
> -        if (val < 0x100)
> -            tcg_out_dat_imm(s, COND_AL, ARITH_ADD, reg, reg, val);
> -        else
> -            tcg_abort();
> -    else if (val < 0) {
> -        if (val > -0x100)
> -            tcg_out_dat_imm(s, COND_AL, ARITH_SUB, reg, reg, -val);
> -        else
> -            tcg_abort();
> -    }
> -}
> -
>  static inline void tcg_out_mov(TCGContext *s, TCGType type, int ret, int arg)
>  {
>     tcg_out_dat_reg(s, COND_AL, ARITH_MOV, ret, 0, arg, SHIFT_IMM_LSL(0));
> --
> 1.7.1
Blue Swirl - Oct. 1, 2011, 12:04 p.m.
Thanks, applied.

On Mon, Sep 12, 2011 at 10:03 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Remove the unused function tcg_out_addi() from the ARM TCG backend;
> this fixes a compilation failure on ARM hosts with newer gcc.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> A previous patch from Richard Henderson for this compile failure:
>  http://patchwork.ozlabs.org/patch/110400/
> was rejected, so here's another go. This simply removes the unused function,
> in line with the approach taken for ppc/ppc64 in commits 1a2eb162414 and
> c24a9c6ef94.
>
> If this is accepted I can do the equivalent patches for tcg/ia64 and tcg/s390
> (although those don't cause compile failures because the unused function
> happens to be marked 'inline'.)
>
>  tcg/arm/tcg-target.c |   15 ---------------
>  1 files changed, 0 insertions(+), 15 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index 93eb0f1..ce4760d 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -1820,21 +1820,6 @@ static inline void tcg_out_st(TCGContext *s, TCGType type, int arg,
>     tcg_out_st32(s, COND_AL, arg, arg1, arg2);
>  }
>
> -static void tcg_out_addi(TCGContext *s, int reg, tcg_target_long val)
> -{
> -    if (val > 0)
> -        if (val < 0x100)
> -            tcg_out_dat_imm(s, COND_AL, ARITH_ADD, reg, reg, val);
> -        else
> -            tcg_abort();
> -    else if (val < 0) {
> -        if (val > -0x100)
> -            tcg_out_dat_imm(s, COND_AL, ARITH_SUB, reg, reg, -val);
> -        else
> -            tcg_abort();
> -    }
> -}
> -
>  static inline void tcg_out_mov(TCGContext *s, TCGType type, int ret, int arg)
>  {
>     tcg_out_dat_reg(s, COND_AL, ARITH_MOV, ret, 0, arg, SHIFT_IMM_LSL(0));
> --
> 1.7.1
>
>
>

Patch

diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index 93eb0f1..ce4760d 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -1820,21 +1820,6 @@  static inline void tcg_out_st(TCGContext *s, TCGType type, int arg,
     tcg_out_st32(s, COND_AL, arg, arg1, arg2);
 }
 
-static void tcg_out_addi(TCGContext *s, int reg, tcg_target_long val)
-{
-    if (val > 0)
-        if (val < 0x100)
-            tcg_out_dat_imm(s, COND_AL, ARITH_ADD, reg, reg, val);
-        else
-            tcg_abort();
-    else if (val < 0) {
-        if (val > -0x100)
-            tcg_out_dat_imm(s, COND_AL, ARITH_SUB, reg, reg, -val);
-        else
-            tcg_abort();
-    }
-}
-
 static inline void tcg_out_mov(TCGContext *s, TCGType type, int ret, int arg)
 {
     tcg_out_dat_reg(s, COND_AL, ARITH_MOV, ret, 0, arg, SHIFT_IMM_LSL(0));