From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41156)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <laurent@vivier.eu>) id 1c4ZOC-0006Iv-TM
	for qemu-devel@nongnu.org; Wed, 09 Nov 2016 15:22:33 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <laurent@vivier.eu>) id 1c4ZO7-0000B6-T7
	for qemu-devel@nongnu.org; Wed, 09 Nov 2016 15:22:32 -0500
Received: from mout.kundenserver.de ([212.227.126.187]:63053)
	by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
	(Exim 4.71) (envelope-from <laurent@vivier.eu>) id 1c4ZO7-0000Ar-JR
	for qemu-devel@nongnu.org; Wed, 09 Nov 2016 15:22:27 -0500
References: <1478712603-18286-1-git-send-email-laurent@vivier.eu>
	<c530a2ff-ffdc-2e39-2957-261db4046ebf@twiddle.net>
	<a762c9d1-e8f1-73b7-c52f-f837d217911b@twiddle.net>
From: Laurent Vivier <laurent@vivier.eu>
Message-ID: <dd2297cd-ffd3-74ae-a44b-b0a0fe4e5d64@vivier.eu>
Date: Wed, 9 Nov 2016 21:22:20 +0100
MIME-Version: 1.0
In-Reply-To: <a762c9d1-e8f1-73b7-c52f-f837d217911b@twiddle.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH] target-m68k: add rol/ror/roxl/roxr
 instructions
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: Richard Henderson <rth@twiddle.net>, qemu-devel@nongnu.org

Le 09/11/2016 à 20:39, Richard Henderson a écrit :
> On 11/09/2016 07:47 PM, Richard Henderson wrote:
>> On 11/09/2016 06:30 PM, Laurent Vivier wrote:
>>> +        /* create [src:X:..] */
>>> +
>>> +        tcg_gen_deposit_i32(t0, QREG_CC_X, src, 1, size);
>>> +        tcg_gen_shli_i32(t0, t0, 31 - size);
>>> +
>>> +        /* rotate */
>>> +
>>> +        tcg_gen_rotl_i32(t0, t0, shift);
>>> +
>>> +        /* result is [src:..:src:X] */
>>> +
>>> +        tcg_gen_andi_i32(X, t0, 1);
>>> +        tcg_gen_shri_i32(t0, t0, 1);
>>
>> I don't see how this is supposed to work.  If you form [src:x:...],
>> and rotate
>> by 0, then X gets garbage.  Of course, you're actually forming

The result is ignored in the case of a rotate by 0 (see movcond in
rotate_reg()).

>> [0:src:x].  But
>> for a rol of 2, the lsb of src gets 0's instead of the msb of src.

for a rol of 2 on an 8bit value [12345678]:

    tcg_gen_deposit_i32(t0, QREG_CC_X, src, 1, size);
    tcg_gen_shli_i32(t0, t0, 31 - size);

t0 = [12345678x00000000000000000000000]

    tcg_gen_rotl_i32(t0, t0, shift);

t0 = [345678x0000000000000000000000012]

    tcg_gen_andi_i32(X, t0, 1);

X = 2

    tcg_gen_shri_i32(t0, t0, 1);

t0 = [0345678x000000000000000000000001]

    tcg_gen_shri_i32(t1, t0, 31 - size);

t1 = [000000000000000000000000345678x0]

    tcg_gen_or_i32(dest, t0, t1);

dest = [0345678x0000000000000000345678x1]

->     we keep only 8 bits: [345678x1]

Where am I wrong?

> Feh, actually that only works for rot8, since you need 2*size+1 bits
> available for this.  Why not just use shifts.  Something like
> 
>   int shl = shift % 17;
>   int shr = shl ^ (size - 1);
>   int shx = shl ? shl - 1 : 16;
> 
>   res32 = (data << shl) | (data >> shr) | (x << shx);
>   x = (res32 >> 16) & 1;


If you think it is better, I can do like that.

Thanks,
Laurent