Hi Max,
On Wed, Jun 27, 2012 at 7:03 PM, Max Filippov <jcmvb...@gmail.com> wrote:
> On Wed, Jun 27, 2012 at 1:54 PM, Jia Liu <pro...@gmail.com> wrote:
>> Add OpenRISC instruction tanslation routines.
>>
>> Signed-off-by: Jia Liu <pro...@gmail.com>
>
> [...]
>
>> + case 0x0009:
>> + switch (op1) {
>> + case 0x03: /* l.div */
>> + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb);
>> + {
>> + int lab0 = gen_new_label();
>> + int lab1 = gen_new_label();
>> + int lab2 = gen_new_label();
>> + int lab3 = gen_new_label();
>> + TCGv_i32 sr_ove = tcg_temp_local_new_i32();
>> + if (rb == 0) {
>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0);
>> + gen_exception(dc, EXCP_RANGE);
>> + gen_set_label(lab0);
>> + } else {
>> + tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb],
>> + 0x00000000, lab1);
>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra],
>> + 0x80000000, lab2);
>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb],
>> + 0xffffffff, lab2);
>> + gen_set_label(lab1);
>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
>> + tcg_gen_brcondi_tl(TCG_COND_EQ, sr_ove, SR_OVE, lab3);
>
> You used to have
>
> tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab2)
>
> here in previous series, why do you change NE to EQ?
>
According to the OpenRISC Arch Manual.
OV: Overflow flag
0 No overflow occured during last arithmetic operation
1 Overflow occured during last arithmetic operation
--------------------------------------------------------------------
OVE:Overflow flag Exception
0 Overflow flag does not cause an exception
1 Overflow flag causes range exception
It will raise a exception, when sr_ove is 1. 0, not.
If there was not a exception, that is, sr_ove is 0, we should use NE
and jump to lab2, compute safety.
but if there was a exception, that is, sr_ove is 1, we have to jump to
lab3 to avoid crash QEMU.
Keep it NE will run the test fine, too. I think maybe EQ is better.
How it to be in your eyes?
>> + gen_exception(dc, EXCP_RANGE);
>> + gen_set_label(lab2);
>> + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
>> + gen_set_label(lab3);
>> + }
>> + tcg_temp_free_i32(sr_ove);
>> + }
>> + break;
>> +
>> + default:
>> + gen_illegal_exception(dc);
>> + break;
>> + }
>> + break;
>> +
>> + case 0x000a:
>> + switch (op1) {
>> + case 0x03: /* l.divu */
>> + LOG_DIS("l.divu r%d, r%d, r%d\n", rd, ra, rb);
>> + {
>> + int lab0 = gen_new_label();
>> + int lab1 = gen_new_label();
>> + int lab2 = gen_new_label();
>> + TCGv_i32 sr_ove = tcg_temp_local_new_i32();
>> + if (rb == 0) {
>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
>> + tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0);
>> + gen_exception(dc, EXCP_RANGE);
>> + gen_set_label(lab0);
>> + } else {
>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb],
>> + 0x00000000, lab1);
>> + tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
>> + tcg_gen_brcondi_tl(TCG_COND_EQ, sr_ove, SR_OVE, lab2);
>
> Same here.
>
>> + gen_exception(dc, EXCP_RANGE);
>> + gen_set_label(lab1);
>> + tcg_gen_divu_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
>> + gen_set_label(lab2);
>> + }
>> + tcg_temp_free_i32(sr_ove);
>> + }
>> + break;
>
> --
> Thanks.
> -- Max
Regards,
Jia.
