qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Zack Buhman <zack@buhman.org>, qemu-devel@nongnu.org
Cc: Samuel Tardieu <sam@rfc1149.net>,
	Aurelien Jarno <aurelien@aurel32.net>,
	Yoshinori Sato <ysato@users.sourceforge.jp>
Subject: Re: [PATCH] sh4: mac.w: memory accesses are 16-bit words
Date: Tue, 2 Apr 2024 15:26:25 +0200	[thread overview]
Message-ID: <16cb6ac0-818b-4393-8b27-20879a150a79@linaro.org> (raw)
In-Reply-To: <20240402093756.27466-1-zack@buhman.org>

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);



  reply	other threads:[~2024-04-02 13:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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é [this message]
2024-04-03  6:32   ` Yoshinori Sato
2024-04-05 22:54 ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16cb6ac0-818b-4393-8b27-20879a150a79@linaro.org \
    --to=philmd@linaro.org \
    --cc=aurelien@aurel32.net \
    --cc=qemu-devel@nongnu.org \
    --cc=sam@rfc1149.net \
    --cc=ysato@users.sourceforge.jp \
    --cc=zack@buhman.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).