On 26.09.20 00:06, Richard Henderson wrote:
> On 9/22/20 3:31 AM, David Hildenbrand wrote:
>> +static uint32_t cc_calc_muls_32(int64_t res)
>> +{
>> + /* Arithmetic shift with sign extension so we can compare against
>> -1ull. */
>> + const uint64_t tmp = res >> 31;
>> +
>> + if (!res) {
>> + return 0;
>> + } else if (!(!tmp || tmp == -1ull)) {
>
> Comparing signed vs unsigned. Use -1 without suffix.
tmp is also uint64_t - but I can change that to int64_t.
(and condense to "tmp && tmp != -1")
>
>> +static uint64_t cc_calc_muls_64(int64_t res_high, uint64_t res_low)
>> +{
>> + const uint8_t tmp = res_low >> 63;
>> +
>> + if (!res_high && !res_low) {
>> + return 0;
>> + } else if (!(!res_high && !tmp) || !(res_high == -1ull && tmp)) {
>
> This simplifies to res_high + tmp != 0.
Yeah, after messing up one time I decided to phrase it just as stated in
the PoP.
>
> Probably better to keep tmp as uint64_t; otherwise we're likely to have an
> unnecessary zero-extension from uint8_t to uint64_t.
> Or, drop 'tmp' altogether and use
>
> if (res_high + (res_low >> 63) != 0)
Thanks, I'll go with that.
>
> Otherwise, looks good.
>
>
> r~
>
--
Thanks,
David / dhildenb
