On 2024-05-31 10:48, Andrew Cooper wrote:
On 31/05/2024 9:34 am, Andrew Cooper wrote:
On 31/05/2024 7:56 am, Nicola Vetrini wrote:
On 2024-05-31 03:14, Stefano Stabellini wrote:
On Fri, 24 May 2024, Andrew Cooper wrote:
Perform constant-folding unconditionally, rather than having it
implemented
inconsistency between architectures.

Confirm the expected behaviour with compile time and boot time tests.

For non-constant inputs, use arch_ffs() if provided but fall back to generic_ffsl() if not.  In particular, RISC-V doesn't have a builtin
that
works in all configurations.

For x86, rename ffs() to arch_ffs() and adjust the prototype.

For PPC, __builtin_ctz() is 1/3 of the size of size of the transform to
generic_fls().  Drop the definition entirely.  ARM too benefits in
the general
case by using __builtin_ctz(), but less dramatically because it using
optimised asm().

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
This patch made me realize that we should add __builtin_ctz,
__builtin_constant_p and always_inline to
docs/misra/C-language-toolchain.rst as they don't seem to be currently
documented and they are not part of the C standard

Patch welcome :-)

I can send a patch for the builtins.
That's very kind of you.

In total by the end of this series, we've got __builtin_constant_p() 
(definitely used elsewhere already), and __builtin_{ffs,ctz,clz}{,l}() 
(3x primitives, 2x input types).

If we're going for a list of the primitive operations, lets add
__builtin_popcnt{,l}() too right away, because if it weren't for 4.19
code freeze, I'd have cleaned up the hweight() helpers too.

Oh, and it's worth noting that __builtin_{ctz,clz}{,l}() have explicit
UB if given an input of 0.  (Sadly, even on architectures where the
underlying instruction emitted is safe with a 0 input. [0])

This is why every patch in the series using them checks for nonzero input.

UBSAN (with an adequate compiler) will instrument this, and Xen has
__ubsan_handle_invalid_builtin() to diagnose these.

~Andrew

[0] It turns out that Clang has a 2-argument form of the builtin with
the second being the "value forwarded" in case the first is 0.  I've not
investigated whether GCC has the same.

Hmm, maybe then it's best if builtins are listed in a separate section in that file, for ease of browsing. Xen also uses (conditionally) __builtin_mem*, __builtin_str* and others, so if all nonstandard intrinsics should be listed (as opposed to the ones in some way relevant for MISRA violations, which was the original scope of the document), then a subset of the attached list would be needed. There are a handful only used in ppc, and since the document only covers x86 and arm, those should be ignored for the time being.

Anyway, I'll send an RFC next week to decide the best route.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
0|$ git grep -E -o "__builtin[a-z_]+" -- xen | cut -d':' -f2 | sort -u
__builtin_bswap
__builtin_clz
__builtin_clzll
__builtin_constant_p
__builtin_ctz
__builtin_ctzll
__builtin_expect
__builtin_frame_address
__builtin_has_attribute
__builtin_memchr
__builtin_memcmp
__builtin_memcpy
__builtin_memmove
__builtin_memset
__builtin_offsetof
__builtin_popcount
__builtin_popcountll
__builtin_prefetch
__builtin_return_address
__builtin_strcasecmp
__builtin_strchr
__builtin_strcmp
__builtin_strlen
__builtin_strncasecmp
__builtin_strncmp
__builtin_strrchr
__builtin_strstr
__builtin_trap
__builtin_types_compatible_p
__builtin_unreachable
__builtin_va_arg
__builtin_va_copy
__builtin_va_end
__builtin_va_list
__builtin_va_start

Reply via email to