[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 03/72] qemu/host-utils: Add wrappers for carry builtins
From: |
Alex Bennée |
Subject: |
Re: [PATCH 03/72] qemu/host-utils: Add wrappers for carry builtins |
Date: |
Wed, 12 May 2021 12:17:24 +0100 |
User-agent: |
mu4e 1.5.13; emacs 28.0.50 |
Richard Henderson <richard.henderson@linaro.org> writes:
> On 5/10/21 7:57 AM, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> These builtins came in clang 3.8, but are not present in gcc through
>>> version 11. Even in clang the optimization is not ideal except for
>>> x86_64, but no worse than the hand-coding that we currently do.
>> Given this statement....
>
> I think you mis-read the "except for x86_64" part?
>
> Anyway, these are simply bugs to be filed against clang, so that
> hopefully clang-12 will do a good job with the builtin. And as I
> said, while the generated code is not ideal, it's no worse.
>
>>> +static inline uint64_t uadd64_carry(uint64_t x, uint64_t y, bool *pcarry)
>>> +{
>>> +#if __has_builtin(__builtin_addcll)
>>> + unsigned long long c = *pcarry;
>>> + x = __builtin_addcll(x, y, c, &c);
>> what happens when unsigned long long isn't the same as uint64_t?
>> Doesn't
>> C99 only specify a minimum?
>
> If you only look at C99, sure. But looking at the set of supported
> hosts, unsigned long long is always a 64-bit type.
I guess I'm worrying about a theoretical future - but we don't worry
about it for other ll builtins so no biggy.
>
>>> + *pcarry = c & 1;
>> Why do we need to clamp it here? Shouldn't the compiler
>> automatically do
>> that due to the bool?
>
> This produces a single AND insn, instead of CMP + SETcc.
Might be worth mentioning that in the commit message.
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
- [PATCH 00/72] Convert floatx80 and float128 to FloatParts, Richard Henderson, 2021/05/07
- [PATCH 05/72] tests/fp: add quad support to the benchmark utility, Richard Henderson, 2021/05/07
- [PATCH 04/72] accel/tcg: Use add/sub overflow routines in tcg-runtime-gvec.c, Richard Henderson, 2021/05/07
- [PATCH 06/72] softfloat: Move the binary point to the msb, Richard Henderson, 2021/05/07
- [PATCH 07/72] softfloat: Inline float_raise, Richard Henderson, 2021/05/07