On Wed, Jun 27, 2012 at 9:41 PM, Max Filippov <[email protected]> wrote:
> On Wed, Jun 27, 2012 at 4:40 PM, Jia Liu <[email protected]> wrote:
>> Hi Max,
>>
>> On Wed, Jun 27, 2012 at 7:03 PM, Max Filippov <[email protected]> wrote:
>>> On Wed, Jun 27, 2012 at 1:54 PM, Jia Liu <[email protected]> wrote:
>>>> Add OpenRISC instruction tanslation routines.
>>>>
>>>> Signed-off-by: Jia Liu <[email protected]>
>>>
>>> [...]
>>>
>>>> + 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.
>
> Right.
>
>> If there was not a exception, that is, sr_ove is 0, we should use NE
>> and jump to lab2, compute safety.
>
> No, sr_ove does not reflect whether there was an exception or not,
> it controls whether we want an exception on overflow or not.
>
>> 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.
>
> It's strange, with my testcase you should get an exception inside
> the guest if you put EQ here. Did you observe it?
>
>> How it to be in your eyes?
>
> Condition code was all correct; when we didn't want an exception
> on overflow, i.e. sr_ove is not equal to SR_OVE, you jumped to exit,
> otherwise you raised an exception. You should keep NE here.
>
OK, you are right, I'll change them back into NE, both of them,
>>>> + 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.
