| Message ID | 20200707070021.10031-5-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/avr: Few fixes | expand |
On 7/7/20 12:00 AM, Philippe Mathieu-Daudé wrote: > @@ -1385,7 +1385,7 @@ static bool trans_SBRC(DisasContext *ctx, arg_SBRC *a) > { > TCGv Rr = cpu_r[a->rr]; > > - ctx->skip_cond = TCG_COND_EQ; > + ctx->skip_cond = TCG_COND_NE; > ctx->skip_var0 = tcg_temp_new(); > ctx->free_skip_var0 = true; This is wrong. The next line is > tcg_gen_andi_tl(ctx->skip_var0, Rr, 1 << a->bit); So we compute "var = R & bit", which should be zero for "Skip if Bit in Register Clear". Thus "var EQ 0" is the correct test. > @@ -1401,7 +1401,7 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a) > { > TCGv Rr = cpu_r[a->rr]; > > - ctx->skip_cond = TCG_COND_NE; > + ctx->skip_cond = TCG_COND_EQ; > ctx->skip_var0 = tcg_temp_new(); > ctx->free_skip_var0 = true; Similarly this is "var NE 0". r~
On 7/7/20 6:34 PM, Richard Henderson wrote: > On 7/7/20 12:00 AM, Philippe Mathieu-Daudé wrote: >> @@ -1385,7 +1385,7 @@ static bool trans_SBRC(DisasContext *ctx, arg_SBRC *a) >> { >> TCGv Rr = cpu_r[a->rr]; >> >> - ctx->skip_cond = TCG_COND_EQ; >> + ctx->skip_cond = TCG_COND_NE; >> ctx->skip_var0 = tcg_temp_new(); >> ctx->free_skip_var0 = true; > > This is wrong. The next line is > >> tcg_gen_andi_tl(ctx->skip_var0, Rr, 1 << a->bit); > > So we compute "var = R & bit", which should be zero for "Skip if Bit in > Register Clear". Thus "var EQ 0" is the correct test. Thanks for verifying. If TCG is correct, then some hardware register might have a bit flipped. I couldn't run Sarah's test suite on Fedora 30: /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find crtatmega2560.o: No such file or directory /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -lm /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -lc /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -latmega2560 collect2: error: ld returned 1 exit status I'll try on some Debian based host. > >> @@ -1401,7 +1401,7 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a) >> { >> TCGv Rr = cpu_r[a->rr]; >> >> - ctx->skip_cond = TCG_COND_NE; >> + ctx->skip_cond = TCG_COND_EQ; >> ctx->skip_var0 = tcg_temp_new(); >> ctx->free_skip_var0 = true; > > Similarly this is "var NE 0".
On 7/7/20 9:58 AM, Philippe Mathieu-Daudé wrote: > I couldn't run Sarah's test suite on Fedora 30: > > /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find > crtatmega2560.o: No such file or directory > /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -lm > /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -lc > /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -latmega2560 > collect2: error: ld returned 1 exit status > > I'll try on some Debian based host. I believe the debian avr-libc package will have those, and should be pulled in by gcc-avr. r~
diff --git a/target/avr/translate.c b/target/avr/translate.c index fe03e676df..2f77fe3ba7 100644 --- a/target/avr/translate.c +++ b/target/avr/translate.c @@ -1385,7 +1385,7 @@ static bool trans_SBRC(DisasContext *ctx, arg_SBRC *a) { TCGv Rr = cpu_r[a->rr]; - ctx->skip_cond = TCG_COND_EQ; + ctx->skip_cond = TCG_COND_NE; ctx->skip_var0 = tcg_temp_new(); ctx->free_skip_var0 = true; @@ -1401,7 +1401,7 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a) { TCGv Rr = cpu_r[a->rr]; - ctx->skip_cond = TCG_COND_NE; + ctx->skip_cond = TCG_COND_EQ; ctx->skip_var0 = tcg_temp_new(); ctx->free_skip_var0 = true;
SBRC/SBRS instructions seem to be inverted. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- target/avr/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)