On 28.02.22 11:14, Thomas Huth wrote:
> On 24/02/2022 00.43, Richard Henderson wrote:
>> On 2/23/22 12:31, David Miller wrote:
>>> +#define F_EPI "stg %%r0, %[res] " : [res] "+m" (res) : : "r0", "r2", "r3"
>>> +
>>> +#define F_PRO asm ( \
>>> + "llihf %%r0,801\n" \
>>> + "lg %%r2, %[a]\n" \
>>> + "lg %%r3, %[b] " \
>>> + : : [a] "m" (a), \
>>> + [b] "m" (b) \
>>> + : "r2", "r3")
>>> +
>>> +#define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
>>> +{ uint64_t res = 0; F_PRO; ASM; return res; }
>>> +
>>> +/* AND WITH COMPLEMENT */
>>> +FbinOp(_ncrk, asm("ncrk %%r0, %%r3, %%r2\n" F_EPI))
>>> +FbinOp(_ncgrk, asm("ncgrk %%r0, %%r3, %%r2\n" F_EPI))
>>
>> Better written as
>>
>> asm("ncrk %0, %3, %2" : "=&r"(res) : "r"(a), "r"(b) : "cc");
>
> I agree with Richard, especially since it's kind of "dangerous" to chain
> multiple asm() statements (without "volatile") and hoping that the compiler
> keeps the values in the registers in between (without reordering the
> statements).
>
> Anyway, since I'll be away for most the rest of the week and we already have
> soft-freeze next week, I'd like to get this fixed for my pull request that I
> plan later for today or tomorrow, so I now went ahead and modified the code
> to look like this:
>
> #define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
> { \
> uint64_t res = 0; \
> asm ("llihf %[res],801\n" ASM \
> : [res]"=&r"(res) : [a]"r"(a), [b]"r"(b) : "cc"); \
> return res; \
> }
>
> /* AND WITH COMPLEMENT */
> FbinOp(_ncrk, ".insn rrf, 0xB9F50000, %[res], %[b], %[a], 0\n")
> FbinOp(_ncgrk, ".insn rrf, 0xB9E50000, %[res], %[b], %[a], 0\n")
>
> /* NAND */
> FbinOp(_nnrk, ".insn rrf, 0xB9740000, %[res], %[b], %[a], 0\n")
> FbinOp(_nngrk, ".insn rrf, 0xB9640000, %[res], %[b], %[a], 0\n")
>
> /* NOT XOR */
> FbinOp(_nxrk, ".insn rrf, 0xB9770000, %[res], %[b], %[a], 0\n")
> FbinOp(_nxgrk, ".insn rrf, 0xB9670000, %[res], %[b], %[a], 0\n")
>
> /* NOR */
> FbinOp(_nork, ".insn rrf, 0xB9760000, %[res], %[b], %[a], 0\n")
> FbinOp(_nogrk, ".insn rrf, 0xB9660000, %[res], %[b], %[a], 0\n")
>
> /* OR WITH COMPLEMENT */
> FbinOp(_ocrk, ".insn rrf, 0xB9750000, %[res], %[b], %[a], 0\n")
> FbinOp(_ocgrk, ".insn rrf, 0xB9650000, %[res], %[b], %[a], 0\n")
>
> Full patch can be seen here:
>
> https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473
>
> I hope that's ok for everybody?
Fine with me.
--
Thanks,
David / dhildenb
