qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <rth@twiddle.net>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	patches@linaro.org, Michael Matz <matz@suse.de>,
	qemu-devel@nongnu.org,
	Claudio Fontana <claudio.fontana@linaro.org>,
	Dirk Mueller <dmueller@suse.de>,
	Will Newton <will.newton@linaro.org>,
	Laurent Desnogues <laurent.desnogues@gmail.com>,
	kvmarm@lists.cs.columbia.edu,
	Christoffer Dall <christoffer.dall@linaro.org>
Subject: Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)
Date: Tue, 10 Dec 2013 08:58:44 -0800	[thread overview]
Message-ID: <52A74844.7060902@twiddle.net> (raw)
In-Reply-To: <87wqjc6c3s.fsf@linaro.org>

On 12/10/2013 05:59 AM, Alex Bennée wrote:
>>> +    if (extend && is_signed) {
>>> +        g_assert(size < 3);
>>> +        tcg_gen_ext32u_i64(dest, dest);
>>> +    }
>>
>> Is it worth noticing in size==2 && !extend that is_signed can be forced false
>> to avoid the extra extension.
> 
> Sorry I don't quite follow, the extension only occurs if it's an explict
> extension into a 64 bit register. Or are you talking about avoiding
> using the extension logic in the generic tcg_gen_qemu_ld_i64 code?

It's not obvious from this patch, since its more about the later gpr loads.
I was thinking about size==4, signed, 32-bit.

I.e. size=10, opc=11.  But reading closer that's unallocated_encoding.  So the
answer for this patch is "no it's not worth it".

But looking forward through the other patches I see

> +        if (size == 3 && opc == 2) {
> +            /* PRFM - prefetch */
> +            return;
> +        }
> +        is_store = (opc == 0);
> +        is_signed = opc & (1<<1);
> +        is_extended = (size < 3) && (opc & 1);

whereas from the ARM I see

if opc<1> == '0' then
  // store or zero-extending load
else
  if size == '11' then
     memop = MemOp_PREFETCH;
     if opc<0> == '1' then UnallocatedEncoding();
  else
     // sign-extending load
     memop = MemOp_LOAD;
     if size == '10' && opc<0> == '1' then UnallocatedEncoding();

I.e. two undiagnosed unallocated_encoding.

BTW, I find (1<<1) harder to scan here than 2, but I wasn't going to mention
that while there was nothing else in the code to fix.


r~

  reply	other threads:[~2013-12-10 16:59 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 18:12 [Qemu-devel] [PATCH 0/9] target-arm: A64 decoder set 3: loads, stores, misc integer Peter Maydell
2013-12-09 18:12 ` [Qemu-devel] [PATCH 1/9] target-arm: A64: add support for stp (store pair) Peter Maydell
2013-12-09 20:17   ` Richard Henderson
2013-12-10 14:05     ` Alex Bennée
2013-12-09 18:12 ` [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair) Peter Maydell
2013-12-09 20:25   ` Richard Henderson
2013-12-10 13:59     ` Alex Bennée
2013-12-10 16:58       ` Richard Henderson [this message]
2013-12-10 17:41         ` Alex Bennée
2013-12-09 18:12 ` [Qemu-devel] [PATCH 3/9] target-arm: A64: add support for ld/st unsigned imm Peter Maydell
2013-12-09 21:02   ` Richard Henderson
2013-12-10 14:09     ` Alex Bennée
2013-12-09 18:12 ` [Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset Peter Maydell
2013-12-09 21:09   ` Richard Henderson
2013-12-10 14:16     ` Alex Bennée
2013-12-10 15:59       ` Richard Henderson
2013-12-11 22:01         ` Alex Bennée
2013-12-09 18:12 ` [Qemu-devel] [PATCH 5/9] target-arm: A64: add support for ld/st with index Peter Maydell
2013-12-09 18:12 ` [Qemu-devel] [PATCH 6/9] target-arm: A64: add support for add, addi, sub, subi Peter Maydell
2013-12-09 21:36   ` Richard Henderson
2013-12-09 18:12 ` [Qemu-devel] [PATCH 7/9] target-arm: A64: add support for move wide instructions Peter Maydell
2013-12-09 21:42   ` Richard Henderson
2013-12-09 18:12 ` [Qemu-devel] [PATCH 8/9] target-arm: A64: add support for 3 src data proc insns Peter Maydell
2013-12-09 21:52   ` Richard Henderson
2013-12-09 18:12 ` [Qemu-devel] [PATCH 9/9] target-arm: A64: implement SVC, BRK Peter Maydell
2013-12-09 21:58   ` Richard Henderson
2013-12-09 22:14     ` Peter Maydell
2013-12-09 18:16 ` [Qemu-devel] [PATCH 0/9] target-arm: A64 decoder set 3: loads, stores, misc integer Peter Maydell

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=52A74844.7060902@twiddle.net \
    --to=rth@twiddle.net \
    --cc=alex.bennee@linaro.org \
    --cc=christoffer.dall@linaro.org \
    --cc=claudio.fontana@linaro.org \
    --cc=dmueller@suse.de \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=laurent.desnogues@gmail.com \
    --cc=matz@suse.de \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=will.newton@linaro.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).