Re: [PATCH v1 6/8] s390x/tcg: Implement MULTIPLY SINGLE (MSC, MSGC, MSGRKC, MSRKC)

[Richard Henderson <richard.henderson@linaro.org>]

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.

> +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.

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)

Otherwise, looks good.


r~