Le 11/11/2016 à 08:31, Richard Henderson a écrit : > On 11/10/2016 11:51 PM, Laurent Vivier wrote: >> +/* Result of rotate_x() is valid if 0 < shift < (size + 1) < 32 */ >> +static TCGv rotate_x(TCGv dest, TCGv src, TCGv shift, int left, int >> size) >> +{ >> + TCGv X, shl, shr, shx; >> + >> + shr = tcg_temp_new(); >> + shl = tcg_temp_new(); >> + shx = tcg_temp_new(); >> + if (left) { >> + tcg_gen_mov_i32(shl, shift); /* shl = shift */ >> + tcg_gen_movi_i32(shr, size + 1); >> + tcg_gen_sub_i32(shr, shr, shift); /* shr = size + 1 - shift */ >> + tcg_gen_subi_i32(shx, shift, 1); /* shx = shift - 1 */ > > You're failing to bound shx properly. If shift == 0, you produce -1 > here which is illegal. You could mask this to 31, but it's even better
Yes, but in this case the result is ignored by the following movcond. tcg/README says: * shl_i32/i64 t0, t1, t2 t0=t1 << t2. Unspecified behavior if t2 < 0 or t2 >= 32 (resp 64) * shr_i32/i64 t0, t1, t2 t0=t1 >> t2 (unsigned). Unspecified behavior if t2 < 0 or t2 >= 32 (resp 64) An operation with "unspecified behavior" shall not crash. However, the result may be one of several possibilities so may be considered an "undefined result". > to produce size, as I said before, because then you don't have to have Perhaps I've missed something (again), but I've tested with "int shr = shl ^ (size - 1);" and it produces the wrong result. for instance size = 8 and shift = 1 shr = 1 ^ 7 = 6 should be: 8 + 1 - 1 = 8 > >> + tcg_gen_movcond_i32(TCG_COND_EQ, X, >> + t1, zero, >> + QREG_CC_X, X); >> + tcg_gen_movcond_i32(TCG_COND_EQ, DREG(insn, 0), >> + t1, zero, >> + DREG(insn, 0), res); > > this at the end. A zero shift will simply leave everything where it > belongs, and you extract all the parts as normal. To do "int shx = shl ? shl - 1 : 16;", we need a "movcond" in every case, which should not be needed in the case of "rotate_im" where shift is between 1 and 8. > >> +DISAS_INSN(rotate_mem) >> +{ >> + TCGv src; >> + TCGv addr; >> + TCGv shift; >> + int left = (insn & 0x100); >> + >> + SRC_EA(env, src, OS_WORD, 0, &addr); >> + >> + shift = tcg_const_i32(1); >> + if (insn & 8) { > > It's bit 9 for memory rotate. right. I fix that. Thanks, Laurent