Re: [PATCH] Teach VRP about __builtin_{ffs,parity,popcount,clz,ctz,clrsb}{,l,ll,imax} (PR target/29776)

[Richard Biener <rguenther at suse dot de>]

Jakub Jelinek <jakub@redhat.com> wrote:

>Hi!
>
>Attached are two versions of a patch to teach VRP about the int bitop
>builtins.  Both patches are identical for all builtins but
>__builtin_c[lt]z*, which are the only two from these that are
>documented
>to have undefined behavior on some argument (0).
>
>The first version is strict, it assumes __builtin_c[lt]z* (0) doesn't
>happen
>in valid programs, while the second one attempts to be less strict to
>avoid
>breaking stuff too much.
>
>The reason for writing the second patch is that longlong.h on various
>targets has stuff like:
>#ifdef __alpha_cix__
>#define count_leading_zeros(COUNT,X)    ((COUNT) = __builtin_clzl (X))
>#define count_trailing_zeros(COUNT,X)   ((COUNT) = __builtin_ctzl (X))
>#define COUNT_LEADING_ZEROS_0 64
>#else
>and documents that if COUNT_LEADING_ZEROS_0 is defined, then
>count_leading_zeros (cnt, 0) should be well defined and set
>cnt to COUNT_LEADING_ZEROS_0.  While neither gcc nor glibc use
>COUNT_LEADING_ZEROS_0 macro, I'm a little bit afraid some code in the
>wild
>might do, and it might even have its own copy of longlong.h, so even if
>we've removed those COUNT_LEADING_ZEROS_0 macros for targets that
>use the builtins, something could stay broken.  So, what the patch does
>is if an optab exists for the mode of the builtin's argument and
>C?Z_DEFINED_VALUE_AT_ZERO is defined, then it will add that value to
>the
>range unless VR of argument is non-zero (well, it handles only a few
>interesting commonly used values, for CLZ only precision of the mode
>(seems right now when CLZ_DEFINED_VALUE_AT_ZERO is non-zero, it sets
>it always to bitsize of the mode, and even widening or double word
>narrowing expansion should maintain this property), for CTZ -1 and
>bitsize).  If there isn't an optab for it, for CLZ it still assumes
>it might be bitsize, for CTZ it just assumes it is undefined behavior
>otherwise, because if I understand the code right, for CTZ we really
>could
>return various values for 0 without hw support for the mode, e.g. when
>CTZ is implemented using CLZ, it might return something, if we use
>wider
>mode hw CTZ and it would return bitsize, that would be bitsize of the
>wider
>mode etc.  I bet longlong.h only uses __builtin_c?z builtins for modes
>actually implemented in hw anyway (otherwise it couldn't be used safely
>in
>libgcc implementation of those libcalls).
>
>Both patches have been bootstrapped/regtested on x86_64-linux and
>i686-linux, which one do you prefer?

The less strict variant.

Thanks 
Richard.

>	Jakub