From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59351)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Vipex-0000PU-KZ
	for qemu-devel@nongnu.org; Tue, 19 Nov 2013 13:04:28 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Vipes-0004ZZ-Jf
	for qemu-devel@nongnu.org; Tue, 19 Nov 2013 13:04:23 -0500
Received: from mail-la0-f44.google.com ([209.85.215.44]:51918)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Vipes-0004ZC-CR
	for qemu-devel@nongnu.org; Tue, 19 Nov 2013 13:04:18 -0500
Received: by mail-la0-f44.google.com with SMTP id ep20so6395032lab.17
	for <qemu-devel@nongnu.org>; Tue, 19 Nov 2013 10:04:16 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <528BA571.6090804@linaro.org>
References: <1380242934-20953-1-git-send-email-agraf@suse.de>
	<1380242934-20953-27-git-send-email-agraf@suse.de>
	<528B9D0E.2060107@linaro.org> <528BA571.6090804@linaro.org>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Tue, 19 Nov 2013 18:03:55 +0000
Message-ID: <CAFEAcA_1qf2iKe44CuwpR1_wpSKcXr24bQj4TK1sVCEAmjmY7g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH 26/60] AArch64: Add ADR instruction
	emulation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Claudio Fontana <claudio.fontana@linaro.org>
Cc: Michael Matz <matz@suse.de>, Alexander Graf <agraf@suse.de>, QEMU Developers <qemu-devel@nongnu.org>, Dirk Mueller <dmueller@suse.de>, Laurent Desnogues <laurent.desnogues@gmail.com>, =?UTF-8?B?QWxleCBCZW5uw6ll?= <alex.bennee@linaro.org>, Christoffer Dall <christoffer.dall@linaro.org>, Richard Henderson <rth@twiddle.net>

On 19 November 2013 17:52, Claudio Fontana <claudio.fontana@linaro.org> wrote:
> static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
> {
>     /*
>      * 31 30 29 28 27 26 25 24 23     5 4  0
>      * op immlo  1  0  0  0  0   immhi   Rd
>      */
>     unsigned int page, imm, rd, len; /* op -> page, immhi:immlo -> imm */
>     uint64_t base;
>     sint64_t offset; /* SignExtend(imm) -> offset */
>
>     page = insn & (1 << 31) ? 1 : 0;
>     imm = extract32(insn, 29, 2) + extract32(insn, 5, 19) << 2;
>     rd = extract32(insn, 0, 5);

Claiming you want sign extension and not using sextract32()
is a bit odd.

>
>     base = s->pc - 4;
>     len = 19 + 2; /* immhi:immlo */
>     offset = imm;
>
>     if (page) {
>         /* ADRP (page based) */
>         base &= ~0xfff;
>         len += 12; /* immhi:immlo:Zeros(12) */
>         offset <<= 12; /* apply Zeros */
>     }
>
>     offset = (offset << (64 - len)) >> (64 - len); /* sign extend */

Don't manually sign extend, please.

>     tcg_gen_movi_i64(cpu_reg(reg), base + offset);
> }

thanks
-- PMM