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~