[PATCH 5/5] target/loongarch: div if x/0 set dividend to 0

Song Gao posted 5 patches 3 years, 4 months ago
Maintainers: Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>
[PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
Posted by Song Gao 3 years, 4 months ago
div.d, div.du, div,w, div.wu, the LoongArch host if x/0  the result is 0.
So we set the divisor to 1 and the dividend to 0.

Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 target/loongarch/insn_trans/trans_arith.c.inc | 34 +++++++++++++++----
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/target/loongarch/insn_trans/trans_arith.c.inc b/target/loongarch/insn_trans/trans_arith.c.inc
index 8e45eadbc8..c97afb16f9 100644
--- a/target/loongarch/insn_trans/trans_arith.c.inc
+++ b/target/loongarch/insn_trans/trans_arith.c.inc
@@ -147,12 +147,28 @@ static void prep_divisor_du(TCGv ret, TCGv src2)
     tcg_gen_movcond_tl(TCG_COND_EQ, ret, src2, zero, one, src2);
 }
 
+static void prep_div(TCGv divisor, TCGv dividend, TCGv src1, TCGv src2)
+{
+    TCGv zero = tcg_constant_tl(0);
+    TCGv one = tcg_constant_tl(1);
+
+    /*
+     * If x / 0, set the diviend to 0 set the divisor to 1
+     * this is the same with LoongArch host.
+     */
+    tcg_gen_movcond_tl(TCG_COND_EQ, dividend, src2, zero, zero, src1);
+    tcg_gen_movcond_tl(TCG_COND_EQ, divisor, src2, zero, one, src2);
+}
+
 static void gen_div_d(TCGv dest, TCGv src1, TCGv src2)
 {
     TCGv t0 = tcg_temp_new();
-    prep_divisor_d(t0, src1, src2);
-    tcg_gen_div_tl(dest, src1, t0);
+    TCGv t1 = tcg_temp_new();
+
+    prep_div(t0, t1, src1, src2);
+    tcg_gen_div_tl(dest, t1, t0);
     tcg_temp_free(t0);
+    tcg_temp_free(t1);
 }
 
 static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2)
@@ -166,9 +182,11 @@ static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2)
 static void gen_div_du(TCGv dest, TCGv src1, TCGv src2)
 {
     TCGv t0 = tcg_temp_new();
-    prep_divisor_du(t0, src2);
-    tcg_gen_divu_tl(dest, src1, t0);
+    TCGv t1 = tcg_temp_new();
+    prep_div(t0, t1, src1, src2);
+    tcg_gen_divu_tl(dest, t1, t0);
     tcg_temp_free(t0);
+    tcg_temp_free(t1);
 }
 
 static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2)
@@ -182,10 +200,12 @@ static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2)
 static void gen_div_w(TCGv dest, TCGv src1, TCGv src2)
 {
     TCGv t0 = tcg_temp_new();
-    /* We need not check for integer overflow for div_w. */
-    prep_divisor_du(t0, src2);
-    tcg_gen_div_tl(dest, src1, t0);
+    TCGv t1 = tcg_temp_new();
+
+    prep_div(t0, t1, src1, src2);
+    tcg_gen_div_tl(dest, t1, t0);
     tcg_temp_free(t0);
+    tcg_temp_free(t1);
 }
 
 static void gen_rem_w(TCGv dest, TCGv src1, TCGv src2)
-- 
2.31.1
Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
Posted by Qi Hu 3 years, 4 months ago
On 2022/9/17 15:59, Song Gao wrote:
> div.d, div.du, div,w, div.wu, the LoongArch host if x/0  the result is 0.

The message has a typo: "div,w" => "div.w"

Also I don't know why we need to do this, since the manual say: "When 
the divisor is 0, the result can be any value".

> So we set the divisor to 1 and the dividend to 0.
>
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/insn_trans/trans_arith.c.inc | 34 +++++++++++++++----
>   1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/target/loongarch/insn_trans/trans_arith.c.inc b/target/loongarch/insn_trans/trans_arith.c.inc
> index 8e45eadbc8..c97afb16f9 100644
> --- a/target/loongarch/insn_trans/trans_arith.c.inc
> +++ b/target/loongarch/insn_trans/trans_arith.c.inc
> @@ -147,12 +147,28 @@ static void prep_divisor_du(TCGv ret, TCGv src2)
>       tcg_gen_movcond_tl(TCG_COND_EQ, ret, src2, zero, one, src2);
>   }
>   
> +static void prep_div(TCGv divisor, TCGv dividend, TCGv src1, TCGv src2)
> +{
> +    TCGv zero = tcg_constant_tl(0);
> +    TCGv one = tcg_constant_tl(1);
> +
> +    /*
> +     * If x / 0, set the diviend to 0 set the divisor to 1
> +     * this is the same with LoongArch host.
> +     */
> +    tcg_gen_movcond_tl(TCG_COND_EQ, dividend, src2, zero, zero, src1);
> +    tcg_gen_movcond_tl(TCG_COND_EQ, divisor, src2, zero, one, src2);
> +}
> +
>   static void gen_div_d(TCGv dest, TCGv src1, TCGv src2)
>   {
>       TCGv t0 = tcg_temp_new();
> -    prep_divisor_d(t0, src1, src2);
> -    tcg_gen_div_tl(dest, src1, t0);
> +    TCGv t1 = tcg_temp_new();
> +
> +    prep_div(t0, t1, src1, src2);
> +    tcg_gen_div_tl(dest, t1, t0);
>       tcg_temp_free(t0);
> +    tcg_temp_free(t1);
>   }
>   
>   static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2)
> @@ -166,9 +182,11 @@ static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2)
>   static void gen_div_du(TCGv dest, TCGv src1, TCGv src2)
>   {
>       TCGv t0 = tcg_temp_new();
> -    prep_divisor_du(t0, src2);
> -    tcg_gen_divu_tl(dest, src1, t0);
> +    TCGv t1 = tcg_temp_new();
> +    prep_div(t0, t1, src1, src2);
> +    tcg_gen_divu_tl(dest, t1, t0);
>       tcg_temp_free(t0);
> +    tcg_temp_free(t1);
>   }
>   
>   static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2)
> @@ -182,10 +200,12 @@ static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2)
>   static void gen_div_w(TCGv dest, TCGv src1, TCGv src2)
>   {
>       TCGv t0 = tcg_temp_new();
> -    /* We need not check for integer overflow for div_w. */
> -    prep_divisor_du(t0, src2);
> -    tcg_gen_div_tl(dest, src1, t0);
> +    TCGv t1 = tcg_temp_new();
> +
> +    prep_div(t0, t1, src1, src2);
> +    tcg_gen_div_tl(dest, t1, t0);
>       tcg_temp_free(t0);
> +    tcg_temp_free(t1);
>   }
>   
>   static void gen_rem_w(TCGv dest, TCGv src1, TCGv src2)
Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
Posted by gaosong 3 years, 4 months ago
在 2022/9/17 下午4:59, Qi Hu 写道:
>
> On 2022/9/17 15:59, Song Gao wrote:
>> div.d, div.du, div,w, div.wu, the LoongArch host if x/0  the result 
>> is 0.
>
> The message has a typo: "div,w" => "div.w"
>
> Also I don't know why we need to do this, since the manual say: "When 
> the divisor is 0, the result can be any value".
>
I tested on LoongArch host,   the result is always 0.

>> So we set the divisor to 1 and the dividend to 0.
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   target/loongarch/insn_trans/trans_arith.c.inc | 34 +++++++++++++++----
>>   1 file changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/loongarch/insn_trans/trans_arith.c.inc 
>> b/target/loongarch/insn_trans/trans_arith.c.inc
>> index 8e45eadbc8..c97afb16f9 100644
>> --- a/target/loongarch/insn_trans/trans_arith.c.inc
>> +++ b/target/loongarch/insn_trans/trans_arith.c.inc
>> @@ -147,12 +147,28 @@ static void prep_divisor_du(TCGv ret, TCGv src2)
>>       tcg_gen_movcond_tl(TCG_COND_EQ, ret, src2, zero, one, src2);
>>   }
>>   +static void prep_div(TCGv divisor, TCGv dividend, TCGv src1, TCGv 
>> src2)
>> +{
>> +    TCGv zero = tcg_constant_tl(0);
>> +    TCGv one = tcg_constant_tl(1);
>> +
>> +    /*
>> +     * If x / 0, set the diviend to 0 set the divisor to 1
>> +     * this is the same with LoongArch host.
>> +     */
>> +    tcg_gen_movcond_tl(TCG_COND_EQ, dividend, src2, zero, zero, src1);
>> +    tcg_gen_movcond_tl(TCG_COND_EQ, divisor, src2, zero, one, src2);
>> +}
>> +
>>   static void gen_div_d(TCGv dest, TCGv src1, TCGv src2)
>>   {
>>       TCGv t0 = tcg_temp_new();
>> -    prep_divisor_d(t0, src1, src2);
>> -    tcg_gen_div_tl(dest, src1, t0);
>> +    TCGv t1 = tcg_temp_new();
>> +
>> +    prep_div(t0, t1, src1, src2);
>> +    tcg_gen_div_tl(dest, t1, t0);
>>       tcg_temp_free(t0);
>> +    tcg_temp_free(t1);
>>   }
>>     static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2)
>> @@ -166,9 +182,11 @@ static void gen_rem_d(TCGv dest, TCGv src1, TCGv 
>> src2)
>>   static void gen_div_du(TCGv dest, TCGv src1, TCGv src2)
>>   {
>>       TCGv t0 = tcg_temp_new();
>> -    prep_divisor_du(t0, src2);
>> -    tcg_gen_divu_tl(dest, src1, t0);
>> +    TCGv t1 = tcg_temp_new();
>> +    prep_div(t0, t1, src1, src2);
>> +    tcg_gen_divu_tl(dest, t1, t0);
>>       tcg_temp_free(t0);
>> +    tcg_temp_free(t1);
>>   }
>>     static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2)
>> @@ -182,10 +200,12 @@ static void gen_rem_du(TCGv dest, TCGv src1, 
>> TCGv src2)
>>   static void gen_div_w(TCGv dest, TCGv src1, TCGv src2)
>>   {
>>       TCGv t0 = tcg_temp_new();
>> -    /* We need not check for integer overflow for div_w. */
>> -    prep_divisor_du(t0, src2);
>> -    tcg_gen_div_tl(dest, src1, t0);
>> +    TCGv t1 = tcg_temp_new();
>> +
>> +    prep_div(t0, t1, src1, src2);
>> +    tcg_gen_div_tl(dest, t1, t0);
>>       tcg_temp_free(t0);
>> +    tcg_temp_free(t1);
>>   }
>>     static void gen_rem_w(TCGv dest, TCGv src1, TCGv src2)
>


Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
Posted by Richard Henderson 3 years, 4 months ago
On 9/17/22 11:12, gaosong wrote:
> 
> 在 2022/9/17 下午4:59, Qi Hu 写道:
>>
>> On 2022/9/17 15:59, Song Gao wrote:
>>> div.d, div.du, div,w, div.wu, the LoongArch host if x/0  the result is 0.
>>
>> The message has a typo: "div,w" => "div.w"
>>
>> Also I don't know why we need to do this, since the manual say: "When the divisor is 0, 
>> the result can be any value".
>>
> I tested on LoongArch host,   the result is always 0.

But it is legal for a different loongarch host implementation to return some other value. 
  Therefore the test itself is not correct.


r~

Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
Posted by gaosong 3 years, 4 months ago
在 2022/9/17 下午6:12, Richard Henderson 写道:
> On 9/17/22 11:12, gaosong wrote:
>>
>> 在 2022/9/17 下午4:59, Qi Hu 写道:
>>>
>>> On 2022/9/17 15:59, Song Gao wrote:
>>>> div.d, div.du, div,w, div.wu, the LoongArch host if x/0  the result 
>>>> is 0.
>>>
>>> The message has a typo: "div,w" => "div.w"
>>>
>>> Also I don't know why we need to do this, since the manual say: 
>>> "When the divisor is 0, the result can be any value".
>>>
>> I tested on LoongArch host,   the result is always 0.
>
> But it is legal for a different loongarch host implementation to 
> return some other value.  Therefore the test itself is not correct.
>
I think the manual maybe not correct,  the hardware engineer said that 
they need to comfirm  whether the result is always 0.

Thanks.
Song Gao
> r~


Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
Posted by Qi Hu 3 years, 4 months ago
On 2022/9/19 19:45, gaosong wrote:
>
> 在 2022/9/17 下午6:12, Richard Henderson 写道:
>> On 9/17/22 11:12, gaosong wrote:
>>>
>>> 在 2022/9/17 下午4:59, Qi Hu 写道:
>>>>
>>>> On 2022/9/17 15:59, Song Gao wrote:
>>>>> div.d, div.du, div,w, div.wu, the LoongArch host if x/0  the 
>>>>> result is 0.
>>>>
>>>> The message has a typo: "div,w" => "div.w"
>>>>
>>>> Also I don't know why we need to do this, since the manual say: 
>>>> "When the divisor is 0, the result can be any value".
>>>>
>>> I tested on LoongArch host,   the result is always 0.
>>
>> But it is legal for a different loongarch host implementation to 
>> return some other value.  Therefore the test itself is not correct.
>>
> I think the manual maybe not correct,  the hardware engineer said that 
> they need to comfirm  whether the result is always 0.
>
> Thanks.
> Song Gao

Hi,

The hardware designers suggested that 0 should not be used as the 
default value when "div 0" occurs. The behavior is not guaranteed in 
future processors.

So I think there are some ways to solve this:

- Remove this case("div 0") from risu test.

- Keep this patch by yourself. If you want to do risu test, patch it. :-)


regards,

Qi

>> r~
>