From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47370)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1V9H9Y-00055z-Rg
	for qemu-devel@nongnu.org; Tue, 13 Aug 2013 12:09:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1V9H9Q-0001CI-Ea
	for qemu-devel@nongnu.org; Tue, 13 Aug 2013 12:09:00 -0400
Received: from mail-qe0-x22d.google.com ([2607:f8b0:400d:c02::22d]:38068)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1V9H9Q-0001Bu-AQ
	for qemu-devel@nongnu.org; Tue, 13 Aug 2013 12:08:52 -0400
Received: by mail-qe0-f45.google.com with SMTP id x7so4391414qeu.32
	for <qemu-devel@nongnu.org>; Tue, 13 Aug 2013 09:08:52 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
Message-ID: <520A5A10.1050508@twiddle.net>
Date: Tue, 13 Aug 2013 09:08:48 -0700
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
References: <CANd1uZ=GNDWNGJED+uTsy5CKSxrD6iUEJp4nMYv_a1pHhSntwg@mail.gmail.com>
In-Reply-To: <CANd1uZ=GNDWNGJED+uTsy5CKSxrD6iUEJp4nMYv_a1pHhSntwg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC 01/14] tcg-aarch64: Allow immediate operands
 to add and sub
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: Jay Foad <jay.foad@gmail.com>
Cc: qemu-devel@nongnu.org

On 08/13/2013 01:57 AM, Jay Foad wrote:
>> -static inline void tcg_out_addi(TCGContext *s, int ext,
>> -                                TCGReg rd, TCGReg rn, unsigned int aimm)
>> +static void tcg_out_addi(TCGContext *s, int ext, TCGReg rd, TCGReg rn,
>> +                         tcg_target_long aimm)
>>  {
>> -    /* add immediate aimm unsigned 12bit value (with LSL 0 or 12) */
>> -    /* using ADD 0x11000000 | (ext) | (aimm << 10) | (rn << 5) | rd */
>> -    unsigned int base = ext ? 0x91000000 : 0x11000000;
>> +    enum aarch64_arith_opc opc = ARITH_ADDI;
>> +    tcg_target_long lo, hi;
>>
>> -    if (aimm <= 0xfff) {
>> -        aimm <<= 10;
>> -    } else {
>> -        /* we can only shift left by 12, on assert we cannot represent */
>> -        assert(!(aimm & 0xfff));
>> -        assert(aimm <= 0xfff000);
>> -        base |= 1 << 22; /* apply LSL 12 */
>> -        aimm >>= 2;
>> +    if (aimm < 0) {
>> +        aimm = -aimm;
>> +        opc = ARITH_SUBI;
>>      }
>> +    hi = aimm & 0xfff000;
>> +    lo = aimm & 0xfff;
>> +    assert(aimm == hi + lo);
> 
> Won't this fail if aimm was -0x1000000 on entry?

Yep, off-by-one thinko on the constraint test.
Should only allow -0xffffff <= x <= 0xffffff.


r~