| Message ID | 20221001162318.153420-33-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement FEAT_HAFDBS | expand |
On Sat, 1 Oct 2022 at 17:42, Richard Henderson <richard.henderson@linaro.org> wrote: > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > 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 <peter.maydell@linaro.org> thanks -- PMM
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~
On Fri, 7 Oct 2022 at 16:37, Richard Henderson <richard.henderson@linaro.org> 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
On 10/7/22 09:11, Peter Maydell wrote: > On Fri, 7 Oct 2022 at 16:37, Richard Henderson > <richard.henderson@linaro.org> 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~
On Fri, 7 Oct 2022 at 17:13, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 10/7/22 09:11, Peter Maydell wrote: > > On Fri, 7 Oct 2022 at 16:37, Richard Henderson > > <richard.henderson@linaro.org> 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
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, }; }
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/internals.h | 2 ++ target/arm/helper.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)