On 12/9/2013 6:26 PM, Richard Henderson wrote:
>> + 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.
Hmm. Consider the case where GPR[RA] = 0xUUUUUUUU_80000000 (U=don't care) and
GPR[RB] = 0xUUUUUUUU_FFFFFFFF. Without these checks, I do not believe overflow
will be properly detected.
>
>> + 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?
I don't disagree with the comment. I was being consistent with existing code
for
divw/divd. I suspect whomever wrote this was trying to match the hardware's
implementation. But 0 is certainly a perfectly good undefined value and would
simplify the code.
