* [PATCH] sh4: mac.w: memory accesses are 16-bit words
@ 2024-04-02 9:37 Zack Buhman
2024-04-02 13:26 ` Philippe Mathieu-Daudé
2024-04-05 22:54 ` Richard Henderson
0 siblings, 2 replies; 4+ messages in thread
From: Zack Buhman @ 2024-04-02 9:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Zack Buhman
Before this change, executing a code sequence such as:
mova tblm,r0
mov r0,r1
mova tbln,r0
clrs
clrmac
mac.w @r0+,@r1+
mac.w @r0+,@r1+
.align 4
tblm: .word 0x1234
.word 0x5678
tbln: .word 0x9abc
.word 0xdefg
Does not result in correct behavior:
Expected behavior:
first macw : macl = 0x1234 * 0x9abc + 0x0
mach = 0x0
second macw: macl = 0x5678 * 0xdefg + 0xb00a630
mach = 0x0
Observed behavior (qemu-sh4eb, prior to this commit):
first macw : macl = 0x5678 * 0xdefg + 0x0
mach = 0x0
second macw: (unaligned longword memory access, SIGBUS)
Various SH-4 ISA manuals also confirm that `mac.w` is a 16-bit word memory
access, not a 32-bit longword memory access.
Signed-off-by: Zack Buhman <zack@buhman.org>
---
target/sh4/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index a9b1bc7524..6643c14dde 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -816,10 +816,10 @@ static void _decode_opc(DisasContext * ctx)
TCGv arg0, arg1;
arg0 = tcg_temp_new();
tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx,
- MO_TESL | MO_ALIGN);
+ MO_TESW | MO_ALIGN);
arg1 = tcg_temp_new();
tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx,
- MO_TESL | MO_ALIGN);
+ MO_TESW | MO_ALIGN);
gen_helper_macw(tcg_env, arg0, arg1);
tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 2);
tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 2);
--
2.41.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] sh4: mac.w: memory accesses are 16-bit words
2024-04-02 9:37 [PATCH] sh4: mac.w: memory accesses are 16-bit words Zack Buhman
@ 2024-04-02 13:26 ` Philippe Mathieu-Daudé
2024-04-03 6:32 ` Yoshinori Sato
2024-04-05 22:54 ` Richard Henderson
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-02 13:26 UTC (permalink / raw)
To: Zack Buhman, qemu-devel; +Cc: Samuel Tardieu, Aurelien Jarno, Yoshinori Sato
On 2/4/24 11:37, Zack Buhman wrote:
> Before this change, executing a code sequence such as:
>
> mova tblm,r0
> mov r0,r1
> mova tbln,r0
> clrs
> clrmac
> mac.w @r0+,@r1+
> mac.w @r0+,@r1+
>
> .align 4
> tblm: .word 0x1234
> .word 0x5678
> tbln: .word 0x9abc
> .word 0xdefg
>
> Does not result in correct behavior:
>
> Expected behavior:
> first macw : macl = 0x1234 * 0x9abc + 0x0
> mach = 0x0
>
> second macw: macl = 0x5678 * 0xdefg + 0xb00a630
> mach = 0x0
>
> Observed behavior (qemu-sh4eb, prior to this commit):
>
> first macw : macl = 0x5678 * 0xdefg + 0x0
> mach = 0x0
>
> second macw: (unaligned longword memory access, SIGBUS)
>
> Various SH-4 ISA manuals also confirm that `mac.w` is a 16-bit word memory
> access, not a 32-bit longword memory access.
>
> Signed-off-by: Zack Buhman <zack@buhman.org>
> ---
> target/sh4/translate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index a9b1bc7524..6643c14dde 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -816,10 +816,10 @@ static void _decode_opc(DisasContext * ctx)
> TCGv arg0, arg1;
> arg0 = tcg_temp_new();
> tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx,
> - MO_TESL | MO_ALIGN);
> + MO_TESW | MO_ALIGN);
> arg1 = tcg_temp_new();
> tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx,
> - MO_TESL | MO_ALIGN);
> + MO_TESW | MO_ALIGN);
Apparently invalid since its introduction in commit fdf9b3e831.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> gen_helper_macw(tcg_env, arg0, arg1);
> tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 2);
> tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 2);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sh4: mac.w: memory accesses are 16-bit words
2024-04-02 13:26 ` Philippe Mathieu-Daudé
@ 2024-04-03 6:32 ` Yoshinori Sato
0 siblings, 0 replies; 4+ messages in thread
From: Yoshinori Sato @ 2024-04-03 6:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Zack Buhman, qemu-devel, Samuel Tardieu, Aurelien Jarno
On Tue, 02 Apr 2024 22:26:25 +0900,
Philippe Mathieu-Daudé wrote:
>
> On 2/4/24 11:37, Zack Buhman wrote:
> > Before this change, executing a code sequence such as:
> >
> > mova tblm,r0
> > mov r0,r1
> > mova tbln,r0
> > clrs
> > clrmac
> > mac.w @r0+,@r1+
> > mac.w @r0+,@r1+
> >
> > .align 4
> > tblm: .word 0x1234
> > .word 0x5678
> > tbln: .word 0x9abc
> > .word 0xdefg
> >
> > Does not result in correct behavior:
> >
> > Expected behavior:
> > first macw : macl = 0x1234 * 0x9abc + 0x0
> > mach = 0x0
> >
> > second macw: macl = 0x5678 * 0xdefg + 0xb00a630
> > mach = 0x0
> >
> > Observed behavior (qemu-sh4eb, prior to this commit):
> >
> > first macw : macl = 0x5678 * 0xdefg + 0x0
> > mach = 0x0
> >
> > second macw: (unaligned longword memory access, SIGBUS)
> >
> > Various SH-4 ISA manuals also confirm that `mac.w` is a 16-bit word memory
> > access, not a 32-bit longword memory access.
> >
> > Signed-off-by: Zack Buhman <zack@buhman.org>
> > ---
> > target/sh4/translate.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> > index a9b1bc7524..6643c14dde 100644
> > --- a/target/sh4/translate.c
> > +++ b/target/sh4/translate.c
> > @@ -816,10 +816,10 @@ static void _decode_opc(DisasContext * ctx)
> > TCGv arg0, arg1;
> > arg0 = tcg_temp_new();
> > tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx,
> > - MO_TESL | MO_ALIGN);
> > + MO_TESW | MO_ALIGN);
> > arg1 = tcg_temp_new();
> > tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx,
> > - MO_TESL | MO_ALIGN);
> > + MO_TESW | MO_ALIGN);
>
> Apparently invalid since its introduction in commit fdf9b3e831.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> > gen_helper_macw(tcg_env, arg0, arg1);
> > tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 2);
> > tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 2);
>
SH4 Software manual said.
https://www.renesas.com/us/en/document/mas/sh-4-software-manual
> This instruction performs signed multiplication of the 16-bit operands
> whose addresses are the contents of general registers Rm and Rn,
> adds the 32-bit result to the MAC register contents, and stores the
> result in the MAC register. Operands Rm and Rn are each incremented
> by 2 each time they are read.
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
--
Yosinori Sato
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sh4: mac.w: memory accesses are 16-bit words
2024-04-02 9:37 [PATCH] sh4: mac.w: memory accesses are 16-bit words Zack Buhman
2024-04-02 13:26 ` Philippe Mathieu-Daudé
@ 2024-04-05 22:54 ` Richard Henderson
1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2024-04-05 22:54 UTC (permalink / raw)
To: Zack Buhman, qemu-devel
On 4/1/24 23:37, Zack Buhman wrote:
> Before this change, executing a code sequence such as:
>
> mova tblm,r0
> mov r0,r1
> mova tbln,r0
> clrs
> clrmac
> mac.w @r0+,@r1+
> mac.w @r0+,@r1+
>
> .align 4
> tblm: .word 0x1234
> .word 0x5678
> tbln: .word 0x9abc
> .word 0xdefg
>
> Does not result in correct behavior:
>
> Expected behavior:
> first macw : macl = 0x1234 * 0x9abc + 0x0
> mach = 0x0
>
> second macw: macl = 0x5678 * 0xdefg + 0xb00a630
> mach = 0x0
>
> Observed behavior (qemu-sh4eb, prior to this commit):
>
> first macw : macl = 0x5678 * 0xdefg + 0x0
> mach = 0x0
>
> second macw: (unaligned longword memory access, SIGBUS)
>
> Various SH-4 ISA manuals also confirm that `mac.w` is a 16-bit word memory
> access, not a 32-bit longword memory access.
>
> Signed-off-by: Zack Buhman <zack@buhman.org>
> ---
> target/sh4/translate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Queued, thanks.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-05 22:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-02 9:37 [PATCH] sh4: mac.w: memory accesses are 16-bit words Zack Buhman
2024-04-02 13:26 ` Philippe Mathieu-Daudé
2024-04-03 6:32 ` Yoshinori Sato
2024-04-05 22:54 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).