Re: [Qemu-devel] [PULL 1/8] target-sh4: use bit number for SR constants

2015-06-03 Thread Peter Maydell
On 3 June 2015 at 15:34, Aurelien Jarno  wrote:
> On 2015-06-03 09:09, Aurelien Jarno wrote:
>> On 2015-06-02 19:01, Christopher Covington wrote:
>> > I like using the BIT() macro for this kind of thing.
>>
>> Thanks for the hint, I'll come with an additional patch to fix that in
>> the code.
>
> Unfortunately it doesn't seem as easy as it appears. The BIT() macro uses
> long types

Yeah, I've always taken it as really only being intended for working
with the bitmap functions, on account of the weird choice of "long".

-- PMM



Re: [Qemu-devel] [PULL 1/8] target-sh4: use bit number for SR constants

2015-06-03 Thread Aurelien Jarno
On 2015-06-03 09:09, Aurelien Jarno wrote:
> On 2015-06-02 19:01, Christopher Covington wrote:
> > Hi Aurelien,
> > 
> > On 06/01/2015 05:29 PM, Aurelien Jarno wrote:
> > > Use the bit number for SR constants instead of using a bit mask. This
> > > make possible to also use the constants for shifts.
> > > 
> > > Reviewed-by: Richard Henderson 
> > > Signed-off-by: Aurelien Jarno 
> > > ---
> > >  target-sh4/cpu.c   |  3 +-
> > >  target-sh4/cpu.h   | 30 ++--
> > >  target-sh4/gdbstub.c   |  4 +--
> > >  target-sh4/helper.c| 27 +-
> > >  target-sh4/op_helper.c | 26 -
> > >  target-sh4/translate.c | 75 
> > > ++
> > >  6 files changed, 85 insertions(+), 80 deletions(-)
> > > 
> > > diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> > > index d187a2b..cccb14f 100644
> > > --- a/target-sh4/cpu.c
> > > +++ b/target-sh4/cpu.c
> > > @@ -61,7 +61,8 @@ static void superh_cpu_reset(CPUState *s)
> > >  env->fpscr = FPSCR_PR; /* value for userspace according to the 
> > > kernel */
> > >  set_float_rounding_mode(float_round_nearest_even, &env->fp_status); 
> > > /* ?! */
> > >  #else
> > > -env->sr = SR_MD | SR_RB | SR_BL | SR_I3 | SR_I2 | SR_I1 | SR_I0;
> > > +env->sr = (1u << SR_MD) | (1u << SR_RB) | (1u << SR_BL) |
> > > +  (1u << SR_I3) | (1u << SR_I2) | (1u << SR_I1) | (1u << 
> > > SR_I0);
> > 
> > I like using the BIT() macro for this kind of thing.
> 
> Thanks for the hint, I'll come with an additional patch to fix that in
> the code.

Unfortunately it doesn't seem as easy as it appears. The BIT() macro uses
long types, so you can't invert it to create a mask without casting it
first, otherwise GCC complains. For example:

| target-sh4/translate.c: In function ‘_decode_opc’:
| target-sh4/translate.c:408:42: error: large integer implicitly truncated to 
unsigned type [-Werror=overflow]
|  tcg_gen_andi_i32(cpu_sr, cpu_sr, ~BIT(SR_S));

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PULL 1/8] target-sh4: use bit number for SR constants

2015-06-03 Thread Aurelien Jarno
On 2015-06-02 19:01, Christopher Covington wrote:
> Hi Aurelien,
> 
> On 06/01/2015 05:29 PM, Aurelien Jarno wrote:
> > Use the bit number for SR constants instead of using a bit mask. This
> > make possible to also use the constants for shifts.
> > 
> > Reviewed-by: Richard Henderson 
> > Signed-off-by: Aurelien Jarno 
> > ---
> >  target-sh4/cpu.c   |  3 +-
> >  target-sh4/cpu.h   | 30 ++--
> >  target-sh4/gdbstub.c   |  4 +--
> >  target-sh4/helper.c| 27 +-
> >  target-sh4/op_helper.c | 26 -
> >  target-sh4/translate.c | 75 
> > ++
> >  6 files changed, 85 insertions(+), 80 deletions(-)
> > 
> > diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> > index d187a2b..cccb14f 100644
> > --- a/target-sh4/cpu.c
> > +++ b/target-sh4/cpu.c
> > @@ -61,7 +61,8 @@ static void superh_cpu_reset(CPUState *s)
> >  env->fpscr = FPSCR_PR; /* value for userspace according to the kernel 
> > */
> >  set_float_rounding_mode(float_round_nearest_even, &env->fp_status); /* 
> > ?! */
> >  #else
> > -env->sr = SR_MD | SR_RB | SR_BL | SR_I3 | SR_I2 | SR_I1 | SR_I0;
> > +env->sr = (1u << SR_MD) | (1u << SR_RB) | (1u << SR_BL) |
> > +  (1u << SR_I3) | (1u << SR_I2) | (1u << SR_I1) | (1u << 
> > SR_I0);
> 
> I like using the BIT() macro for this kind of thing.

Thanks for the hint, I'll come with an additional patch to fix that in
the code.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PULL 1/8] target-sh4: use bit number for SR constants

2015-06-02 Thread Christopher Covington
Hi Aurelien,

On 06/01/2015 05:29 PM, Aurelien Jarno wrote:
> Use the bit number for SR constants instead of using a bit mask. This
> make possible to also use the constants for shifts.
> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-sh4/cpu.c   |  3 +-
>  target-sh4/cpu.h   | 30 ++--
>  target-sh4/gdbstub.c   |  4 +--
>  target-sh4/helper.c| 27 +-
>  target-sh4/op_helper.c | 26 -
>  target-sh4/translate.c | 75 
> ++
>  6 files changed, 85 insertions(+), 80 deletions(-)
> 
> diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> index d187a2b..cccb14f 100644
> --- a/target-sh4/cpu.c
> +++ b/target-sh4/cpu.c
> @@ -61,7 +61,8 @@ static void superh_cpu_reset(CPUState *s)
>  env->fpscr = FPSCR_PR; /* value for userspace according to the kernel */
>  set_float_rounding_mode(float_round_nearest_even, &env->fp_status); /* 
> ?! */
>  #else
> -env->sr = SR_MD | SR_RB | SR_BL | SR_I3 | SR_I2 | SR_I1 | SR_I0;
> +env->sr = (1u << SR_MD) | (1u << SR_RB) | (1u << SR_BL) |
> +  (1u << SR_I3) | (1u << SR_I2) | (1u << SR_I1) | (1u << SR_I0);

I like using the BIT() macro for this kind of thing.

Chris

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[Qemu-devel] [PULL 1/8] target-sh4: use bit number for SR constants

2015-06-01 Thread Aurelien Jarno
Use the bit number for SR constants instead of using a bit mask. This
make possible to also use the constants for shifts.

Reviewed-by: Richard Henderson 
Signed-off-by: Aurelien Jarno 
---
 target-sh4/cpu.c   |  3 +-
 target-sh4/cpu.h   | 30 ++--
 target-sh4/gdbstub.c   |  4 +--
 target-sh4/helper.c| 27 +-
 target-sh4/op_helper.c | 26 -
 target-sh4/translate.c | 75 ++
 6 files changed, 85 insertions(+), 80 deletions(-)

diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index d187a2b..cccb14f 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -61,7 +61,8 @@ static void superh_cpu_reset(CPUState *s)
 env->fpscr = FPSCR_PR; /* value for userspace according to the kernel */
 set_float_rounding_mode(float_round_nearest_even, &env->fp_status); /* ?! 
*/
 #else
-env->sr = SR_MD | SR_RB | SR_BL | SR_I3 | SR_I2 | SR_I1 | SR_I0;
+env->sr = (1u << SR_MD) | (1u << SR_RB) | (1u << SR_BL) |
+  (1u << SR_I3) | (1u << SR_I2) | (1u << SR_I1) | (1u << SR_I0);
 env->fpscr = FPSCR_DN | FPSCR_RM_ZERO; /* CPU reset value according to SH4 
manual */
 set_float_rounding_mode(float_round_to_zero, &env->fp_status);
 set_flush_to_zero(1, &env->fp_status);
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index c8dea6c..76fda35 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -47,18 +47,18 @@
 #define TARGET_PHYS_ADDR_SPACE_BITS 32
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
-#define SR_MD (1 << 30)
-#define SR_RB (1 << 29)
-#define SR_BL (1 << 28)
-#define SR_FD (1 << 15)
-#define SR_M  (1 << 9)
-#define SR_Q  (1 << 8)
-#define SR_I3 (1 << 7)
-#define SR_I2 (1 << 6)
-#define SR_I1 (1 << 5)
-#define SR_I0 (1 << 4)
-#define SR_S  (1 << 1)
-#define SR_T  (1 << 0)
+#define SR_MD 30
+#define SR_RB 29
+#define SR_BL 28
+#define SR_FD 15
+#define SR_M  9
+#define SR_Q  8
+#define SR_I3 7
+#define SR_I2 6
+#define SR_I1 5
+#define SR_I0 4
+#define SR_S  1
+#define SR_T  0
 
 #define FPSCR_MASK (0x003f)
 #define FPSCR_FR   (1 << 21)
@@ -234,7 +234,7 @@ void cpu_load_tlb(CPUSH4State * env);
 #define MMU_USER_IDX 1
 static inline int cpu_mmu_index (CPUSH4State *env)
 {
-return (env->sr & SR_MD) == 0 ? 1 : 0;
+return (env->sr & (1u << SR_MD)) == 0 ? 1 : 0;
 }
 
 #include "exec/cpu-all.h"
@@ -339,8 +339,8 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, 
target_ulong *pc,
 *flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL
 | DELAY_SLOT_TRUE | DELAY_SLOT_CLEARME))   /* Bits  0- 3 */
 | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
-| (env->sr & (SR_MD | SR_RB))  /* Bits 29-30 */
-| (env->sr & SR_FD)/* Bit 15 */
+| (env->sr & ((1u << SR_MD) | (1u << SR_RB)))  /* Bits 29-30 */
+| (env->sr & (1u << SR_FD))/* Bit 15 */
 | (env->movcal_backup ? TB_FLAG_PENDING_MOVCA : 0); /* Bit 4 */
 }
 
diff --git a/target-sh4/gdbstub.c b/target-sh4/gdbstub.c
index df4fa2a..05ba728 100644
--- a/target-sh4/gdbstub.c
+++ b/target-sh4/gdbstub.c
@@ -31,7 +31,7 @@ int superh_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 
 switch (n) {
 case 0 ... 7:
-if ((env->sr & (SR_MD | SR_RB)) == (SR_MD | SR_RB)) {
+if ((env->sr & (1u << SR_MD)) && (env->sr & (1u << SR_RB))) {
 return gdb_get_regl(mem_buf, env->gregs[n + 16]);
 } else {
 return gdb_get_regl(mem_buf, env->gregs[n]);
@@ -83,7 +83,7 @@ int superh_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 
 switch (n) {
 case 0 ... 7:
-if ((env->sr & (SR_MD | SR_RB)) == (SR_MD | SR_RB)) {
+if ((env->sr & (1u << SR_MD)) && (env->sr & (1u << SR_RB))) {
 env->gregs[n + 16] = ldl_p(mem_buf);
 } else {
 env->gregs[n] = ldl_p(mem_buf);
diff --git a/target-sh4/helper.c b/target-sh4/helper.c
index 5811360..1cb0e8d 100644
--- a/target-sh4/helper.c
+++ b/target-sh4/helper.c
@@ -93,7 +93,7 @@ void superh_cpu_do_interrupt(CPUState *cs)
 do_exp = cs->exception_index != -1;
 do_irq = do_irq && (cs->exception_index == -1);
 
-if (env->sr & SR_BL) {
+if (env->sr & (1u << SR_BL)) {
 if (do_exp && cs->exception_index != 0x1e0) {
 cs->exception_index = 0x000; /* masked exception -> reset */
 }
@@ -165,7 +165,7 @@ void superh_cpu_do_interrupt(CPUState *cs)
 env->ssr = env->sr;
 env->spc = env->pc;
 env->sgr = env->gregs[15];
-env->sr |= SR_BL | SR_MD | SR_RB;
+env->sr |= (1u << SR_BL) | (1u << SR_MD) | (1u << SR_RB);
 
 if (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
 /* Branch instruction should be executed again before delay slot. */
@@ -182,7 +182,7 @@ void superh_cpu_do_interrupt(CPUState *cs)
 case 0x000: