On 12/09/2013 07:47 AM, Tom Musta wrote: > + /* does the result fit in 32 bits? */ > \ > + tcg_gen_brcondi_i64(TCG_COND_LT, cpu_gpr[rD(ctx->opcode)], > INT32_MIN, \ > + lbl_ov); > \ > + tcg_gen_brcondi_i64(TCG_COND_GT, cpu_gpr[rD(ctx->opcode)], > INT32_MAX, \ > + lbl_ov); > \
Better with an extend and single brcond. > + tcg_gen_shli_i64(ra, cpu_gpr[rA(ctx->opcode)], 32); > \ > + /* check for MIN div -1 */ > \ > + int l3 = gen_new_label(); > \ > + tcg_gen_brcondi_i64(TCG_COND_NE, rb, -1l, l3); > \ > + tcg_gen_brcondi_i64(TCG_COND_EQ, ra, INT64_MIN, lbl_ov); > \ The second test can never be true, since ra has 32 zero bits. Thus the first test is also pointless. > + gen_set_label(lbl_ov); /* overflow handling */ > \ > + > \ > + if (signed) { > \ > + tcg_gen_sari_i64(cpu_gpr[rD(ctx->opcode)], ra, 63); > \ > + } else { > \ > + tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 0); > \ > + } > \ Divide by zero from the signed case reads an uninitialized ra here. While it's true that the result is undefined, I don't think we want to expose uninitialized reads to the TCG optimizer. Although... what is that sari for anyway? The result is undefined in the non-div-by-zero overflow case as well. We might as well use 0, or even 0xdeadbeef, all the time, no? r~