|
From: | Richard Henderson |
Subject: | Re: [PATCH v4 29/31] target/ppc: Implement cfuged instruction |
Date: | Thu, 13 May 2021 06:31:27 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 5/12/21 1:54 PM, matheus.ferst@eldorado.org.br wrote:
+ while (i) { + n = ctz64(mask); + if (n > i) { + n = i; + } + + m = (1ll << n) - 1; + if (bit) { + right = ror64(right | (src & m), n); + } else { + left = ror64(left | (src & m), n); + } + + src >>= n; + mask >>= n; + i -= n; + bit = !bit; + mask = ~mask; + } + + if (bit) { + n = ctpop64(mask); + } else { + n = 64 - ctpop64(mask); + } + + return left | (right >> n); +}
This doesn't correspond to the algorithm presented in the manual. Thus this requires lots of extra commentary.
I guess I see how you're trying to process blocks at a time, instead of single bits at a time. But I don't think the merging of data into "right" and "left" looks right. I would have expected
right = (right << n) | (src & m); and similarly for left.It doesn't look like that the ctpop at the end is correct, given how mask has been modified. I would have thought that
n = ctpop64(orig_mask); return (left << n) | right; would be the correct answer.I could be wrong about the above, but that's what the missing commentary should have helped me understand.
+static bool trans_CFUGED(DisasContext *ctx, arg_X *a) +{ + REQUIRE_64BIT(ctx); + REQUIRE_INSNS_FLAGS2(ctx, ISA310); +#if defined(TARGET_PPC64) + gen_helper_cfuged(cpu_gpr[a->ra], cpu_gpr[a->rt], cpu_gpr[a->rb]); +#else + gen_invalid(ctx); +#endif + return true; +}
Given that this helper will also be used by vcfuged, there's no point in hiding it in a TARGET_PPC64 block, and thus you can drop the ifdefs.
r~
[Prev in Thread] | Current Thread | [Next in Thread] |