[RFC PATCH-for-10.1 07/19] target/hppa: Replace TARGET_LONG_BITS -> target_long_bits()

Philippe Mathieu-Daudé posted 19 patches 7 months, 2 weeks ago
[RFC PATCH-for-10.1 07/19] target/hppa: Replace TARGET_LONG_BITS -> target_long_bits()
Posted by Philippe Mathieu-Daudé 7 months, 2 weeks ago
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/hppa/mem_helper.c | 3 ++-
 target/hppa/translate.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 554d7bf4d14..03cd103f284 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "qemu/target_info.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/cputlb.h"
@@ -101,7 +102,7 @@ static void hppa_flush_tlb_ent(CPUHPPAState *env, HPPATLBEntry *ent,
 
     tlb_flush_range_by_mmuidx(cs, ent->itree.start,
                               ent->itree.last - ent->itree.start + 1,
-                              HPPA_MMU_FLUSH_MASK, TARGET_LONG_BITS);
+                              HPPA_MMU_FLUSH_MASK, target_long_bits());
 
     /* Never clear BTLBs, unless forced to do so. */
     is_btlb = ent < &env->tlb[HPPA_BTLB_ENTRIES(env)];
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 14f38333222..81f535589cf 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/target_info.h"
 #include "cpu.h"
 #include "qemu/host-utils.h"
 #include "exec/exec-all.h"
@@ -3731,7 +3732,7 @@ static bool trans_shrp_imm(DisasContext *ctx, arg_shrp_imm *a)
     t2 = load_gpr(ctx, a->r2);
     if (a->r1 == 0) {
         tcg_gen_extract_i64(dest, t2, sa, width - sa);
-    } else if (width == TARGET_LONG_BITS) {
+    } else if (width == target_long_bits()) {
         tcg_gen_extract2_i64(dest, t2, cpu_gr[a->r1], sa);
     } else {
         assert(!a->d);
-- 
2.47.1


Re: [RFC PATCH-for-10.1 07/19] target/hppa: Replace TARGET_LONG_BITS -> target_long_bits()
Posted by Paolo Bonzini 7 months ago
On 4/4/25 01:49, Philippe Mathieu-Daudé wrote:
> @@ -101,7 +102,7 @@ static void hppa_flush_tlb_ent(CPUHPPAState *env, HPPATLBEntry *ent,
>   
>       tlb_flush_range_by_mmuidx(cs, ent->itree.start,
>                                 ent->itree.last - ent->itree.start + 1,
> -                              HPPA_MMU_FLUSH_MASK, TARGET_LONG_BITS);
> +                              HPPA_MMU_FLUSH_MASK, target_long_bits());

This ignores the fact that TCG is still using the target_long type, so 
there's no real hope at this point of differentiating TARGET_LONG_BITS 
this way.

I think that you really need to make sure that target_long/target_ulong 
are little more than a shortcut used by TCG backends to invoke *_tl 
functions.  Basically remove all uses outside target/ and tcg/ (a lot of 
uses in hw/ are for sPAPR devices and they can be replaced easily with 
uint64_t; there are actually not many).

Then let cpu.h define target_long/target_ulong so that you can remove 
the uses in tcg/ as well, and poison TARGET_LONG_BITS outside target/. 
This way TARGET_LONG_BITS does not need to be part of the TargetInfo.

On the other hand, if I'm missing something in your plan just tell me.

Paolo


Re: [RFC PATCH-for-10.1 07/19] target/hppa: Replace TARGET_LONG_BITS -> target_long_bits()
Posted by Pierrick Bouvier 7 months, 2 weeks ago
On 4/3/25 16:49, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/hppa/mem_helper.c | 3 ++-
>   target/hppa/translate.c  | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 554d7bf4d14..03cd103f284 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -19,6 +19,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
> +#include "qemu/target_info.h"
>   #include "cpu.h"
>   #include "exec/exec-all.h"
>   #include "exec/cputlb.h"
> @@ -101,7 +102,7 @@ static void hppa_flush_tlb_ent(CPUHPPAState *env, HPPATLBEntry *ent,
>   
>       tlb_flush_range_by_mmuidx(cs, ent->itree.start,
>                                 ent->itree.last - ent->itree.start + 1,
> -                              HPPA_MMU_FLUSH_MASK, TARGET_LONG_BITS);
> +                              HPPA_MMU_FLUSH_MASK, target_long_bits());
>   
>       /* Never clear BTLBs, unless forced to do so. */
>       is_btlb = ent < &env->tlb[HPPA_BTLB_ENTRIES(env)];
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 14f38333222..81f535589cf 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -18,6 +18,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/target_info.h"
>   #include "cpu.h"
>   #include "qemu/host-utils.h"
>   #include "exec/exec-all.h"
> @@ -3731,7 +3732,7 @@ static bool trans_shrp_imm(DisasContext *ctx, arg_shrp_imm *a)
>       t2 = load_gpr(ctx, a->r2);
>       if (a->r1 == 0) {
>           tcg_gen_extract_i64(dest, t2, sa, width - sa);
> -    } else if (width == TARGET_LONG_BITS) {
> +    } else if (width == target_long_bits()) {
>           tcg_gen_extract2_i64(dest, t2, cpu_gr[a->r1], sa);
>       } else {
>           assert(!a->d);

The temptation is good, but please do not touch any target code at this 
point. We want to focus on defining the API first, and we can perform 
codebase changes as a second step, without letting any occurrences of 
the old macros/functions, instead of just adding "another way to do it".
Re: [RFC PATCH-for-10.1 07/19] target/hppa: Replace TARGET_LONG_BITS -> target_long_bits()
Posted by Philippe Mathieu-Daudé 7 months, 2 weeks ago
On 4/4/25 18:48, Pierrick Bouvier wrote:
> On 4/3/25 16:49, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/hppa/mem_helper.c | 3 ++-
>>   target/hppa/translate.c  | 3 ++-
>>   2 files changed, 4 insertions(+), 2 deletions(-)


> The temptation is good, but please do not touch any target code at this 
> point. We want to focus on defining the API first, and we can perform 
> codebase changes as a second step, without letting any occurrences of 
> the old macros/functions, instead of just adding "another way to do it".

I meant to remove these patch before posting, to focus on ARM, but
apparently forgot to do so...