[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 8/8] target/ppc: add vdiv{su}{wd} vmod{su}{wd} instruction
From: |
Lijun Pan |
Subject: |
Re: [PATCH v3 8/8] target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions |
Date: |
Thu, 25 Jun 2020 16:15:20 -0500 |
> On Jun 25, 2020, at 1:37 PM, Richard Henderson <richard.henderson@linaro.org>
> wrote:
>
> On 6/25/20 10:00 AM, Lijun Pan wrote:
>> +#define VDIV_MOD_DO(name, op, element, sign, bit) \
>> + void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b) \
>> + { \
>> + int i; \
>> + \
>> + \
>> + for (i = 0; i < ARRAY_SIZE(r->element); i++) { \
>> + if (unlikely((b->element[i] == 0) || \
>> + (sign && \
>> + (b->element[i] == UINT##bit##_MAX) && \
>> + (a->element[i] == INT##bit##_MIN)))) \
>> + continue; \
>> + r->element[i] = a->element[i] op b->element[i]; \
>> + } \
>> + }
>
> Missing braces for the if. Extra blank line before the for.
No, the braces are enough. "unlikely" is to describe the whole logic,
eg. if (unlikely( (divisor == 0) || (sign && (divisor == 0xFFFFFFFF) &&
(dividend = 0x80000000) ) ) )
I will remove that blank line.
>
> I see that the ISA document says divide-by-zero produces an undefined result,
> so leaving the previous contents does seem to be within the letter of the law.
>
> However... Are you able to test what real hardware produces? It would be nice
> (but not required) to match if it is simple to do so.
>
> Whichever way we go with the undefined result, this deserves a comment.
I will add “continue; / * Undefined, No Special Registers Altered */ "
Lijun
- Re: [PATCH v3 2/8] target/ppc: add byte-reverse br[dwh] instructions, (continued)
[PATCH v3 7/8] target/ppc: add vmulh{su}d instructions, Lijun Pan, 2020/06/25