[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 25/61] target/riscv: vector single-width averaging add and
Re: [PATCH v6 25/61] target/riscv: vector single-width averaging add and subtract
Sat, 28 Mar 2020 23:37:08 +0800
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0
On 2020/3/28 9:22, Richard Henderson wrote:
On 3/27/20 6:07 PM, LIU Zhiwei wrote:
On 2020/3/28 8:32, Richard Henderson wrote:
On 3/18/20 8:46 PM, LIU Zhiwei wrote:
+static inline int32_t asub32(CPURISCVState *env, int vxrm, int32_t a, int32_t
+ int64_t res = (int64_t)a - b;
+ uint8_t round = get_round(vxrm, res, 1);
+ return (res >> 1) + round;
I find a corner case here. As the spec said in Section 13.2
"There can be no overflow in the result".
If the a is 0x7fffffff, b is 0x80000000, and the round mode is round to
then the result is (0x7fffffff - 0x80000000 + 1) >> 1, equals 0x80000000,
according the v0.7.1
That's why we used int64_t as the intermediate type:
0x000000007fffffff - 0xffffffff80000000 + 1
= 0x000000007fffffff + 0x0000000080000000 + 1
= 0x00000000ffffffff + 1
Shift that right by 1 and you do indeed get 0x80000000.
There's no saturation involved.
The minuend 0x7fffffff is INT32_MAX, and the subtrahend 0x80000000 is INT32_MIN.
The difference between the minuend and the subtrahend should be a positive
number. But the result here is 0x80000000.
So it is overflow. However, according to the spec, it should not overflow.
Unless I'm missing something, the spec is wrong about "there can be no
overflow", the above being a counter-example.
Do you have hardware to compare against? Perhaps it is in fact "overflow is
ignored", as the 0.8 spec says for vasubu?
Agree! the overflow is just ignored. The code in the patch is OK now.
I discussed it with hardware coworker and software coworker.
The hardware coworker points that it is an error in spec.
The software coworker think overflow will make this instruction some
as the shift and round should protect the result from overflow.
Like vasubu, overflow ignore is much better for vasub in this case.
I wouldn't add saturation, because the spec says nothing about saturation, and
does mention truncation, at least for vasubu.
[PATCH v6 26/61] target/riscv: vector single-width fractional multiply with rounding and saturation, LIU Zhiwei, 2020/03/17
[PATCH v6 27/61] target/riscv: vector widening saturating scaled multiply-add, LIU Zhiwei, 2020/03/17
[PATCH v6 28/61] target/riscv: vector single-width scaling shift instructions, LIU Zhiwei, 2020/03/17
[PATCH v6 29/61] target/riscv: vector narrowing fixed-point clip instructions, LIU Zhiwei, 2020/03/17
[PATCH v6 30/61] target/riscv: vector single-width floating-point add/subtract instructions, LIU Zhiwei, 2020/03/17
- [PATCH v6 23/61] target/riscv: vector integer merge and move instructions, (continued)