On Tue, Jun 26, 2012 at 5:37 AM, Jia Liu <pro...@gmail.com> wrote:
> 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.
That's the whole point. Overflow and division by zero jumps here only
in case SR_OVE is cleared, i.e. we don't want exception. I haven't found
in openrisc documentation what result l.div produces in this case.
I guess there can be two options: either the result is undefined, or
it's some special value, like INF/NAN in the floating point world.
> 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.
Actually you did this with three brcondi in the else branch. The only thing
left is not to crash in case guest doesn't want range exception.
> I'm working on it to find a suitable path, a little difficult to me.
--
Thanks.
-- Max
