On Fri, 10 Nov 2023, Jakub Jelinek wrote:
> On Fri, Nov 10, 2023 at 08:09:26AM +0000, Richard Biener wrote:
> > > The following patch adds 6 new type-generic builtins,
> > > __builtin_clzg
> > > __builtin_ctzg
> > > __builtin_clrsbg
> > > __builtin_ffsg
> > > __builtin_parityg
> > > __builtin_popcountg
> > > The g at the end stands for generic because the unsuffixed variant
> > > of the builtins already have unsigned int or int arguments.
> > >
> > > The main reason to add these is to support arbitrary unsigned (for
> > > clrsb/ffs signed) bit-precise integer types and also __int128 which
> > > wasn't supported by the existing builtins, so that e.g. <stdbit.h>
> > > type-generic functions could then support not just bit-precise unsigned
> > > integer type whose width matches a standard or extended integer type,
> > > but others too.
> > >
> > > None of these new builtins promote their first argument, so the argument
> > > can be e.g. unsigned char or unsigned short or unsigned __int20 etc.
> >
> > But is that a good idea? Is that how type generic functions work in C?
>
> Most current type generic functions deal just with floating point args and
> don't promote there float to double as the ... promotions would normally do:
> __builtin_signbit
> __builtin_fpclassify
> __builtin_isfinite
> __builtin_isinf_sign
> __builtin_isinf
> __builtin_isnan
> __builtin_isnormal
> __builtin_isgreater
> __builtin_isgreaterequal
> __builtin_isless
> __builtin_islessequal
> __builtin_islessgreater
> __builtin_isunordered
> __builtin_iseqsig
> __builtin_issignaling
>
> __builtin_clear_padding is uninteresting, because the argument must be a
> pointer.
>
> Then
> __builtin_add_overflow
> __builtin_sub_overflow
> __builtin_mul_overflow
> do promote the first 2 arguments, but that doesn't really matter, because
> all we care about is the argument values, not their type.
>
> And finally
> __builtin_add_overflow_p
> __builtin_sub_overflow_p
> __builtin_mul_overflow_p
> do promote the first 2 arguments, and don't promote the third one, which is
> the only one where we care about the type, so that is the behavior that
> I've used also for the new builtins. I think if we added e.g.
> __builtin_classify_type now and not more than 3 decades ago it would behave
> like that too.
> Only not promoting the argument will make it directly usable in the
> stdc_leading_zeros, stdc_leading_ones, stdc_trailing_zeros,
> stdc_trailing_ones,
> stdc_first_leading_zero, ..., stdc_count_zeros, stdc_count_ones, ...
> C23 stdbit.h type-generic macros, otherwise one would need to play with
> _Generic and special-case there unsigned char and unsigned short (which
> normally promote to int), but e.g. unsigned _BitInt(8) doesn't.
googling doesn't find me stdc_leading_zeros - are those supposed to work
for non-_BitInt types as well and don't promote the argument in that
case?
If we are spcificially targeting those I wonder why we don't name
the builtins after those? But yes, if promotion is undesirable
for implementing them then I agree. IIRC _BitInt(n) is not subject
to integer promotions.
> I expect Joseph will have compatibility version of the macro for when these
> builtins aren't supported, but given the standard says that {un,}signed
> _BitInt
> with width matching standard/extended integer types other than bool needs to
> be handled also, either it will not use _Generic at all and just go after
> sizeof (argument), or maybe will use _Generic and for the default case will
> go after sizeof. Seems clang returns -1 for __builtin_classify_type (0uwb)
> rather than 18 GCC returns, so one can't portably distinguish bitints.
>
> > I think it introduces non-obvious/unexpected behavior in user code.
>
> If we keep the patch behavior of requiring unsigned
> standard/extended/bit-precise types other than bool for the
> clz/ctz/parity/popcount cases, the choice is between always erroring on
> __builtin_clzg ((unsigned char) 1) - where the promoted argument is signed,
> and accepting it as unsigned char case without promotion, so I think users
> would be more confused to see an error on that.
> If we'd switch to accepting both signed and unsigned
> standard/extended/bit-precise integer types other than bool for all the
> builtins, whether we promote or not doesn't matter for ctz/parity/popcount
> but does for clz.
> The clrsb/ffs cases accept signed argument on the other side, so both
> promoted and unpromoted argument would mean something and be accepted,
> in the ffs case it again doesn't really matter for the result, but for clrsb
> is significant.
> Would it help to just document it that the argument isn't promoted?
>
> We document that for __builtin_*_overflow_p:
> "The value of the third argument is ignored, just the side effects in the
> third argument
> are evaluated, and no integral argument promotions are performed on the last
> argument."
>
> > If people do not want to "compensate" for this maybe insted also add
> > __builtin_*{8,16} (like we have for the bswap variants)?
>
> Note, clang has __builtin_clzs and __builtin_ctzs for unsigned short (but
> not the other 4), but nothing for the unsigned char cases.
> I was just hoping we don't need to add further variants if we have
> type-generic ones.
>
> > Otherwise this looks reasonable. I'm not sure why we need separate
> > CFN_CLZ and CFN_BUILT_IN_CLZG? (why CFN_BUILT_IN_CLZG and not CFN_CLZG?)
> > That is, I'm confused about
> >
> > CASE_CFN_CLRSB:
> > + case CFN_BUILT_IN_CLRSBG:
> >
> > why does CASE_CFN_CLRSB not include CLRSBG? It includes IFN_CLRSB, no?
> > And IFN_CLRSB already has the two and one arg case and thus encompasses
> > some BUILT_IN_CLRSBG cases?
>
> gencfn-macros.cc is aware of just normal float suffixes (F, nothing, L;
> then under different names of the macros other variants for float
> suffixes), and int suffixes (nothing, L, LL, IMAX), it doesn't know anything
> about the G suffix. We could teach it to under a different suffix add the
> G case too, but I didn't think it was necessary because the *G builtins are
> meant to be folded away into something else as soon as possible, worst case
> during gimplification, so nothing after that ought to care about them.
> It is just the fold-const-call.cc case where in constant expressions I think
> we want to fold them into constants and having new macros just to use them
> once (and don't want to use them in the 2-6 other places depending on the
> builtin) seemed unnecessary.
>
> > Besides the above question I'd say OK (I assume Josephs reply is a
> > general ack from his side).
>
> Joseph, what are your thoughts on the above?
>
> Incremental patch to document the lack of integral argument promotion:
>
> --- gcc/doc/extend.texi.jj 2023-11-09 09:17:40.240182342 +0100
> +++ gcc/doc/extend.texi 2023-11-10 09:57:45.396215654 +0100
> @@ -14962,13 +14962,15 @@ Similar to @code{__builtin_parity}, exce
>
> @defbuiltin{int __builtin_ffsg (...)}
> Similar to @code{__builtin_ffs}, except the argument is type-generic
> -signed integer (standard, extended or bit-precise).
> +signed integer (standard, extended or bit-precise). No integral argument
> +promotions are performed on the argument.
> @enddefbuiltin
>
> @defbuiltin{int __builtin_clzg (...)}
> Similar to @code{__builtin_clz}, except the argument is type-generic
> unsigned integer (standard, extended or bit-precise) and there is
> -optional second argument with int type. If two arguments are specified,
> +optional second argument with int type. No integral argument promotions
> +are performed on the first argument. If two arguments are specified,
> and first argument is 0, the result is the second argument. If only
> one argument is specified and it is 0, the result is undefined.
> @enddefbuiltin
> @@ -14976,24 +14978,28 @@ one argument is specified and it is 0, t
> @defbuiltin{int __builtin_ctzg (...)}
> Similar to @code{__builtin_ctz}, except the argument is type-generic
> unsigned integer (standard, extended or bit-precise) and there is
> -optional second argument with int type. If two arguments are specified,
> +optional second argument with int type. No integral argument promotions
> +are performed on the first argument. If two arguments are specified,
> and first argument is 0, the result is the second argument. If only
> one argument is specified and it is 0, the result is undefined.
> @enddefbuiltin
>
> @defbuiltin{int __builtin_clrsbg (...)}
> Similar to @code{__builtin_clrsb}, except the argument is type-generic
> -signed integer (standard, extended or bit-precise).
> +signed integer (standard, extended or bit-precise). No integral argument
> +promotions are performed on the argument.
> @enddefbuiltin
>
> @defbuiltin{int __builtin_popcountg (...)}
> Similar to @code{__builtin_popcount}, except the argument is type-generic
> -unsigned integer (standard, extended or bit-precise).
> +unsigned integer (standard, extended or bit-precise). No integral argument
> +promotions are performed on the argument.
> @enddefbuiltin
>
> @defbuiltin{int __builtin_parityg (...)}
> Similar to @code{__builtin_parity}, except the argument is type-generic
> -unsigned integer (standard, extended or bit-precise).
> +unsigned integer (standard, extended or bit-precise). No integral argument
> +promotions are performed on the argument.
> @enddefbuiltin
>
> @defbuiltin{double __builtin_powi (double, int)}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
