Hi Max,
On Mon, Jun 25, 2012 at 5:00 PM, Max Filippov <jcmvb...@gmail.com> wrote:
> On Mon, Jun 25, 2012 at 6:50 AM, Jia Liu <pro...@gmail.com> wrote:
>> Hi Max,
>>
>> On Thu, Jun 21, 2012 at 6:24 PM, Max Filippov <jcmvb...@gmail.com> wrote:
>>> On Thu, Jun 21, 2012 at 6:58 AM, 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();
>>>> + 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_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0);
>>>> + tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
>>>> + 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],
>>>> + 0xffffffff, lab2);
>>>> + tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb],
>>>> + 0x80000000, 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_NE, sr_ove, SR_OVE, lab2);
>>>
>>> Causes host division by zero/overflow. I'd suggest to brcond to lab3 set
>>> after
>>> the final tcg_gen_div.
>>
>> Causes host division by zero/overflow? Can I handle the host code? I'm
>> confused about this.
>> May I get more comment about this? Sorry I didn't understand it.
>
> The brcondi in question jumps to the division operation although we
> know for sure
> that it's going to be a division by zero or an overflow. The host is
> not smarter than
> we are, it cannot magically divide by zero. So, if the result register value
> is
> undefined in case of division by zero/overflow than you should skip the
> division
> and brcondi to a label lab3, right after it.
>
>>>> + 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);
If we set a lab3 here, after tcg_gen_div_tl, and jump here when
(cpr_R[ra] == 0xffffffff && cpu_R[rb] == 0x80000000), it won't raise a
exception or compute.
It may not be a good way.
>
>>>> + }
>>>> + tcg_temp_free_i32(sr_ove);
>>>> + }
>>>> + break;
>
> A picture worth a thousand words is the test for this specific case.
> I think it can look like this (because SR_OVE is cleared upon reset):
>
> int main(void)
> {
> int a, b, c;
>
> b = 0x80000000;
> c = 0xffffffff;
>
> __asm
> ("l.div %0, %1, %2\n"
> : "=r"(a)
> : "r"(b), "r"(c)
> );
> return 0;
> }
>
> I believe that this test with the current div implementation would crash qemu.
>
Yeah, it crashed. It looks I should find a better way to handle l.div*.
Thank you for your test case.
0xffffffff is -1, 0x80000000 is -MAX.
-1/-MAX will raise a exception, and I've handle this.
-MAX/-1 will get a MAX, for max value of a register is -(-MAX)-1, so,
it overflowed, I didn't handle this.
I'm working on it to find a suitable path, a little difficult to me.
> BTW, shouldn't tests that detect wrong behavior do "return 1" or something
> like this to make "make check" fully automatic?
>
Thanks, I'll add a return value to all of them.
> --
> Thanks.
> -- Max
Regards,
Jia.
