diff mbox series

[08/13] target/arm/cpu: flags2 is always uint64_t

Message ID 20250318045125.759259-9-pierrick.bouvier@linaro.org
State New
Headers show
Series single-binary: start make hw/arm/ common (boot.c) | expand

Commit Message

Pierrick Bouvier March 18, 2025, 4:51 a.m. UTC
Do not rely on target dependent type, but use a fixed type instead.
Since the original type is unsigned, it should be safe to extend its
size without any side effect.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/cpu.h        | 2 +-
 target/arm/tcg/hflags.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Richard Henderson March 18, 2025, 10:40 p.m. UTC | #1
On 3/17/25 21:51, Pierrick Bouvier wrote:
> Do not rely on target dependent type, but use a fixed type instead.
> Since the original type is unsigned, it should be safe to extend its
> size without any side effect.
> 
> Signed-off-by: Pierrick Bouvier<pierrick.bouvier@linaro.org>
> ---
>   target/arm/cpu.h        | 2 +-
>   target/arm/tcg/hflags.c | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

We can also remove the comment lower down about cs_base being
based on target_ulong, since that was changed some time ago:

exec/translation-block.h:    uint64_t cs_base;



r~
Pierrick Bouvier March 19, 2025, 11:17 p.m. UTC | #2
On 3/18/25 15:40, Richard Henderson wrote:
> On 3/17/25 21:51, Pierrick Bouvier wrote:
>> Do not rely on target dependent type, but use a fixed type instead.
>> Since the original type is unsigned, it should be safe to extend its
>> size without any side effect.
>>
>> Signed-off-by: Pierrick Bouvier<pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/cpu.h        | 2 +-
>>    target/arm/tcg/hflags.c | 4 ++--
>>    2 files changed, 3 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> We can also remove the comment lower down about cs_base being
> based on target_ulong, since that was changed some time ago:
> 
> exec/translation-block.h:    uint64_t cs_base;
> 
>

Sure.

I updated the comment to this:

   * We collect these two parts in CPUARMTBFlags where they are named
   * flags and flags2 respectively.
   *
- * The flags that are shared between all execution modes, TBFLAG_ANY,
- * are stored in flags.  The flags that are specific to a given mode
- * are stores in flags2.  Since cs_base is sized on the configured
- * address size, flags2 always has 64-bits for A64, and a minimum of
- * 32-bits for A32 and M32.
+ * The flags that are shared between all execution modes, TBFLAG_ANY, 
are stored
+ * in flags. The flags that are specific to a given mode are stored in 
flags2.
+ * flags2 always has 64-bits, even though only 32-bits are used for A32 
and M32.
   *
   * The bits for 32-bit A-profile and M-profile partially overlap:
   *


> 
> r~
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 96f7801a239..27a0d4550f2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -194,7 +194,7 @@  typedef struct ARMPACKey {
 /* See the commentary above the TBFLAG field definitions.  */
 typedef struct CPUARMTBFlags {
     uint32_t flags;
-    target_ulong flags2;
+    uint64_t flags2;
 } CPUARMTBFlags;
 
 typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 8d79b8b7ae1..e51d9f7b159 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -506,8 +506,8 @@  void assert_hflags_rebuild_correctly(CPUARMState *env)
 
     if (unlikely(c.flags != r.flags || c.flags2 != r.flags2)) {
         fprintf(stderr, "TCG hflags mismatch "
-                        "(current:(0x%08x,0x" TARGET_FMT_lx ")"
-                        " rebuilt:(0x%08x,0x" TARGET_FMT_lx ")\n",
+                        "(current:(0x%08x,0x%016" PRIx64 ")"
+                        " rebuilt:(0x%08x,0x%016" PRIx64 ")\n",
                 c.flags, c.flags2, r.flags, r.flags2);
         abort();
     }