From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46210) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPbRX-0005BL-0b for qemu-devel@nongnu.org; Wed, 12 Aug 2015 15:12:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPbRT-0005k8-PT for qemu-devel@nongnu.org; Wed, 12 Aug 2015 15:12:06 -0400 Received: from mail-qg0-x229.google.com ([2607:f8b0:400d:c04::229]:33459) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPbRT-0005ji-Kg for qemu-devel@nongnu.org; Wed, 12 Aug 2015 15:12:03 -0400 Received: by qged69 with SMTP id d69so17190721qge.0 for ; Wed, 12 Aug 2015 12:12:03 -0700 (PDT) Sender: Richard Henderson References: <1439151229-27747-1-git-send-email-laurent@vivier.eu> <1439151229-27747-29-git-send-email-laurent@vivier.eu> From: Richard Henderson Message-ID: <55CB9A7F.9010704@twiddle.net> Date: Wed, 12 Aug 2015 12:11:59 -0700 MIME-Version: 1.0 In-Reply-To: <1439151229-27747-29-git-send-email-laurent@vivier.eu> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.5 28/30] m68k: shift/rotate bytes and words List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, Andreas Schwab , gerg@uclinux.org On 08/09/2015 01:13 PM, Laurent Vivier wrote: > +#define HELPER_SHL(type, bits) \ > +uint32_t HELPER(glue(glue(shl, bits), _cc))(CPUM68KState *env, \ > + uint32_t val, uint32_t shift) \ > +{ \ > + type result; \ > + uint32_t cf; \ > + shift &= 63; \ > + if (shift == 0) { \ > + result = (type)val; \ > + cf = 0; \ > + } else if (shift < bits) { \ > + result = (type)val << shift; \ > + cf = ((type)val >> (bits - shift)) & 1; \ > + } else if (shift == bits) { \ > + result = 0; \ > + cf = val & 1; \ > + } else { \ > + result = 0; \ > + cf = 0; \ > + } \ Perhaps this would be cleaner to simply operate on a 64-bit type. uint64_t res = (type)val; res <<= shift & 63; cf = (res >> bits) & 1; For shift == 0, we've not set bit BITS, so it's zero. For shift <= BITS, we've obviously got the correct data. For shift > BITS, we've shifted val all the way past and again have zero. > +#define HELPER_SHR(type, bits) \ > +uint32_t HELPER(glue(glue(shr, bits), _cc))(CPUM68KState *env, \ > + uint32_t val, uint32_t shift) \ > +{ \ > + type result; \ > + uint32_t cf; \ > + shift &= 63; \ > + if (shift == 0) { \ > + result = (type)val; \ > + cf = 0; \ > + } else if (shift < bits) { \ > + result = (type)val >> shift; \ > + cf = ((type)val >> (shift - 1)) & 1; \ > + } else if (shift == bits) { \ > + result = 0; \ > + cf = (type)val >> (bits - 1); \ > + } else { \ > + result = 0; \ > + cf = 0; \ > + } \ Similarly. > +#define HELPER_SAL(type, bits) \ > +uint32_t HELPER(glue(glue(sal, bits), _cc))(CPUM68KState *env, \ > + uint32_t val, uint32_t shift) \ > +{ \ > + type result; \ > + uint32_t cf; \ > + uint32_t vf; \ > + uint32_t m; \ > + shift &= 63; \ > + if (shift == 0) { \ > + vf = 0; \ > + } else if (shift < bits) { \ > + m = ((1llu << (shift + 1)) - 1) << (bits - shift - 1); \ > + vf = (val & m) != m && (val & m) != 0; \ > + } else { \ > + m = (1llu << bits) - 1; \ > + vf = (val & m) != 0; \ > + } \ This computation of VF seems overly complex. It's just (type)(val ^ result) < 0 for all cases. > +DISAS_INSN(shift8_im) > +DISAS_INSN(shift16_im) > DISAS_INSN(shift_im) ... > +DISAS_INSN(shift8_reg) > +DISAS_INSN(shift16_reg) > DISAS_INSN(shift_reg) ... > +DISAS_INSN(shift_mem) Surely some code could be shared here... r~