diff mbox series

[v4,16/40] target/arm/helper: use vaddr instead of target_ulong for probe_access

Message ID 20250504052914.3525365-17-pierrick.bouvier@linaro.org
State New
Headers show
Series single-binary: compile target/arm twice | expand

Commit Message

Pierrick Bouvier May 4, 2025, 5:28 a.m. UTC
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/helper.h            | 2 +-
 target/arm/tcg/op_helper.c     | 2 +-
 target/arm/tcg/translate-a64.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Richard Henderson May 4, 2025, 4:17 p.m. UTC | #1
On 5/3/25 22:28, Pierrick Bouvier wrote:
> +++ b/target/arm/tcg/translate-a64.c
> @@ -258,7 +258,7 @@ static void gen_address_with_allocation_tag0(TCGv_i64 dst, TCGv_i64 src)
>   static void gen_probe_access(DisasContext *s, TCGv_i64 ptr,
>                                MMUAccessType acc, int log2_size)
>   {
> -    gen_helper_probe_access(tcg_env, ptr,
> +    gen_helper_probe_access(tcg_env, (TCGv_vaddr) ptr,
>                               tcg_constant_i32(acc),
>                               tcg_constant_i32(get_mem_index(s)),
>                               tcg_constant_i32(1 << log2_size));

This cast is incorrect.
You need something akin to tcg_gen_trunc_i64_ptr.

Alternately, do not create TCGv_vaddr as a distinct type,
but simply a #define for either TCGv_{i32,i64}.

In this case, it'll be TCGv_i64 and everything will match.


r~
Pierrick Bouvier May 4, 2025, 4:29 p.m. UTC | #2
On 5/4/25 9:17 AM, Richard Henderson wrote:
> On 5/3/25 22:28, Pierrick Bouvier wrote:
>> +++ b/target/arm/tcg/translate-a64.c
>> @@ -258,7 +258,7 @@ static void gen_address_with_allocation_tag0(TCGv_i64 dst, TCGv_i64 src)
>>    static void gen_probe_access(DisasContext *s, TCGv_i64 ptr,
>>                                 MMUAccessType acc, int log2_size)
>>    {
>> -    gen_helper_probe_access(tcg_env, ptr,
>> +    gen_helper_probe_access(tcg_env, (TCGv_vaddr) ptr,
>>                                tcg_constant_i32(acc),
>>                                tcg_constant_i32(get_mem_index(s)),
>>                                tcg_constant_i32(1 << log2_size));
> 
> This cast is incorrect.

I'll change to i32/i64 typedef, but I wonder if it's ok in tcg code to 
do this kind of cast, when you know the dh_typecode will match behind 
the hoods?

In this case, I thought it was ok since this compilation units is only 
compiled for 64 bits hosts, thus ensuring TCGv_vaddr has the same 
storage size and dh_typecode behind the hoods.

> You need something akin to tcg_gen_trunc_i64_ptr.
> 
> Alternately, do not create TCGv_vaddr as a distinct type,
> but simply a #define for either TCGv_{i32,i64}.
> 

Ok.

> In this case, it'll be TCGv_i64 and everything will match.
> 
> 
> r~
> 
>
diff mbox series

Patch

diff --git a/target/arm/helper.h b/target/arm/helper.h
index 95b9211c6f4..0a4fc90fa8b 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -104,7 +104,7 @@  DEF_HELPER_FLAGS_1(rebuild_hflags_a32_newel, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(rebuild_hflags_a32, TCG_CALL_NO_RWG, void, env, int)
 DEF_HELPER_FLAGS_2(rebuild_hflags_a64, TCG_CALL_NO_RWG, void, env, int)
 
-DEF_HELPER_FLAGS_5(probe_access, TCG_CALL_NO_WG, void, env, tl, i32, i32, i32)
+DEF_HELPER_FLAGS_5(probe_access, TCG_CALL_NO_WG, void, env, vaddr, i32, i32, i32)
 
 DEF_HELPER_1(vfp_get_fpscr, i32, env)
 DEF_HELPER_2(vfp_set_fpscr, void, env, i32)
diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index 38d49cbb9d8..33bc595c992 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -1222,7 +1222,7 @@  uint32_t HELPER(ror_cc)(CPUARMState *env, uint32_t x, uint32_t i)
     }
 }
 
-void HELPER(probe_access)(CPUARMState *env, target_ulong ptr,
+void HELPER(probe_access)(CPUARMState *env, vaddr ptr,
                           uint32_t access_type, uint32_t mmu_idx,
                           uint32_t size)
 {
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 4f94fe179b0..395c0f5c18e 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -258,7 +258,7 @@  static void gen_address_with_allocation_tag0(TCGv_i64 dst, TCGv_i64 src)
 static void gen_probe_access(DisasContext *s, TCGv_i64 ptr,
                              MMUAccessType acc, int log2_size)
 {
-    gen_helper_probe_access(tcg_env, ptr,
+    gen_helper_probe_access(tcg_env, (TCGv_vaddr) ptr,
                             tcg_constant_i32(acc),
                             tcg_constant_i32(get_mem_index(s)),
                             tcg_constant_i32(1 << log2_size));