Re: [PATCH v3 32/42] target/arm: Extract HA and HD in aa64_va_parameters

2022-10-07 Thread Peter Maydell 
On Fri, 7 Oct 2022 at 17:13, Richard Henderson
 wrote:
>
> On 10/7/22 09:11, Peter Maydell wrote:
> > On Fri, 7 Oct 2022 at 16:37, Richard Henderson
> >  wrote:
> >>
> >> On 10/7/22 02:24, Peter Maydell wrote:
>  +.ha = ha,
>  +.hd = ha & hd,
> >>>
> >>> This is a bitwise operation on two bools, should be && ?
> >>
> >> Bitwise works fine, but I can use boolean if you like.
> >>
> >> I'd be surprised (and filing a missed optimization bug) if the compiler 
> >> treated these two
> >> operations differently in this case (simple bool operands with no side 
> >> effects).
> >
> > The different treatment I would expect would be that in the '&'
> > case it warns you about using a bitwise operation on a boolean :-)
>
> Oh, well, no compiler should ever do that, because bool implicitly converts 
> to int for any
> arithmetic, just like char.

Yeah, but -Wbool-operation is there to catch bugs where the bitwise
operation was unintended and the wrong behaviour.

-- PMM

Re: [PATCH v3 32/42] target/arm: Extract HA and HD in aa64_va_parameters

2022-10-07 Thread Richard Henderson 

On 10/7/22 02:24, Peter Maydell wrote:

+.ha = ha,
+.hd = ha & hd,


This is a bitwise operation on two bools, should be && ?


Bitwise works fine, but I can use boolean if you like.

I'd be surprised (and filing a missed optimization bug) if the compiler treated these two 
operations differently in this case (simple bool operands with no side effects).



r~

Re: [PATCH v3 32/42] target/arm: Extract HA and HD in aa64_va_parameters

2022-10-07 Thread Peter Maydell 
On Fri, 7 Oct 2022 at 16:37, Richard Henderson
 wrote:
>
> On 10/7/22 02:24, Peter Maydell wrote:
> >> +.ha = ha,
> >> +.hd = ha & hd,
> >
> > This is a bitwise operation on two bools, should be && ?
>
> Bitwise works fine, but I can use boolean if you like.
>
> I'd be surprised (and filing a missed optimization bug) if the compiler 
> treated these two
> operations differently in this case (simple bool operands with no side 
> effects).

The different treatment I would expect would be that in the '&'
case it warns you about using a bitwise operation on a boolean :-)

-- PMM

Re: [PATCH v3 32/42] target/arm: Extract HA and HD in aa64_va_parameters

2022-10-07 Thread Richard Henderson 

On 10/7/22 09:11, Peter Maydell wrote:

On Fri, 7 Oct 2022 at 16:37, Richard Henderson
 wrote:


On 10/7/22 02:24, Peter Maydell wrote:

+.ha = ha,
+.hd = ha & hd,


This is a bitwise operation on two bools, should be && ?


Bitwise works fine, but I can use boolean if you like.

I'd be surprised (and filing a missed optimization bug) if the compiler treated 
these two
operations differently in this case (simple bool operands with no side effects).


The different treatment I would expect would be that in the '&'
case it warns you about using a bitwise operation on a boolean :-)


Oh, well, no compiler should ever do that, because bool implicitly converts to int for any 
arithmetic, just like char.



r~

Re: [PATCH v3 32/42] target/arm: Extract HA and HD in aa64_va_parameters

2022-10-07 Thread Peter Maydell 
On Sat, 1 Oct 2022 at 17:42, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/internals.h | 2 ++
>  target/arm/helper.c| 8 +++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index a50189e2e4..e95b6b1b8f 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1014,6 +1014,8 @@ typedef struct ARMVAParameters {
>  bool using64k   : 1;
>  bool tsz_oob: 1;  /* tsz has been clamped to legal range */
>  bool ds : 1;
> +bool ha : 1;
> +bool hd : 1;
>  } ARMVAParameters;
>
>  ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 19a03eb200..70ae3816b9 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10280,7 +10280,7 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
> uint64_t va,
> ARMMMUIdx mmu_idx, bool data)
>  {
>  uint64_t tcr = regime_tcr(env, mmu_idx);
> -bool epd, hpd, using16k, using64k, tsz_oob, ds;
> +bool epd, hpd, using16k, using64k, tsz_oob, ds, ha, hd;
>  int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
>  ARMCPU *cpu = env_archcpu(env);
>
> @@ -10298,6 +10298,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
> uint64_t va,
>  epd = false;
>  sh = extract32(tcr, 12, 2);
>  ps = extract32(tcr, 16, 3);
> +ha = extract32(tcr, 21, 1) && cpu_isar_feature(aa64_hafs, cpu);
> +hd = extract32(tcr, 22, 1) && cpu_isar_feature(aa64_hdbs, cpu);
>  ds = extract64(tcr, 32, 1);
>  } else {
>  /*
> @@ -10322,6 +10324,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
> uint64_t va,
>  hpd = extract64(tcr, 42, 1);
>  }
>  ps = extract64(tcr, 32, 3);
> +ha = extract64(tcr, 39, 1) && cpu_isar_feature(aa64_hafs, cpu);
> +hd = extract64(tcr, 40, 1) && cpu_isar_feature(aa64_hdbs, cpu);
>  ds = extract64(tcr, 59, 1);
>  }
>
> @@ -10393,6 +10397,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
> uint64_t va,
>  .using64k = using64k,
>  .tsz_oob = tsz_oob,
>  .ds = ds,
> +.ha = ha,
> +.hd = ha & hd,

This is a bitwise operation on two bools, should be && ?

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH v3 32/42] target/arm: Extract HA and HD in aa64_va_parameters

2022-10-01 Thread Richard Henderson 
Signed-off-by: Richard Henderson 
---
 target/arm/internals.h | 2 ++
 target/arm/helper.c| 8 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index a50189e2e4..e95b6b1b8f 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1014,6 +1014,8 @@ typedef struct ARMVAParameters {
 bool using64k   : 1;
 bool tsz_oob: 1;  /* tsz has been clamped to legal range */
 bool ds : 1;
+bool ha : 1;
+bool hd : 1;
 } ARMVAParameters;
 
 ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 19a03eb200..70ae3816b9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10280,7 +10280,7 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
uint64_t va,
ARMMMUIdx mmu_idx, bool data)
 {
 uint64_t tcr = regime_tcr(env, mmu_idx);
-bool epd, hpd, using16k, using64k, tsz_oob, ds;
+bool epd, hpd, using16k, using64k, tsz_oob, ds, ha, hd;
 int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
 ARMCPU *cpu = env_archcpu(env);
 
@@ -10298,6 +10298,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
uint64_t va,
 epd = false;
 sh = extract32(tcr, 12, 2);
 ps = extract32(tcr, 16, 3);
+ha = extract32(tcr, 21, 1) && cpu_isar_feature(aa64_hafs, cpu);
+hd = extract32(tcr, 22, 1) && cpu_isar_feature(aa64_hdbs, cpu);
 ds = extract64(tcr, 32, 1);
 } else {
 /*
@@ -10322,6 +10324,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
uint64_t va,
 hpd = extract64(tcr, 42, 1);
 }
 ps = extract64(tcr, 32, 3);
+ha = extract64(tcr, 39, 1) && cpu_isar_feature(aa64_hafs, cpu);
+hd = extract64(tcr, 40, 1) && cpu_isar_feature(aa64_hdbs, cpu);
 ds = extract64(tcr, 59, 1);
 }
 
@@ -10393,6 +10397,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
uint64_t va,
 .using64k = using64k,
 .tsz_oob = tsz_oob,
 .ds = ds,
+.ha = ha,
+.hd = ha & hd,
 };
 }
 
-- 
2.34.1