[Bug c++/112789] New: Missing clang __builtin_ctzs for short

[Bug c++/112789] New: Missing clang __builtin_ctzs for short

[gonzalo.gadeschi at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

            Bug ID: 112789
           Summary: Missing clang __builtin_ctzs for short
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gonzalo.gadeschi at gmail dot com
  Target Milestone: ---

GCC is missing clang's __builtin_ctzs .

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
ok, and?

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Anyways GCC 14 adds __builtin_ctzg, __builtin_stdc_first_trailing_one, and
__builtin_stdc_trailing_zeros :
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fctzg

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Also clang does not even document __builtin_ctzs anywhere ...

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So just for future reference __builtin_c[lt]zs is for short.

But with GCC 14 (trunk) added __builtin_ct[lt]g, there is no need to add
__builtin_c[lt]zs to GCC.

You could just do:

#if __has_builtin(__builtin_ctzg) && !__has_builtin(__builtin_ctzs)
#define ctzs(a) __builtin_ctzg((short)a)
#elif __has_builtin(__builtin_ctzs)
#define ctzs(a) __builtin_ctzs((short)a)
#else
#error implement fall back for not both cases
#endif

[Bug c++/112789] Missing clang __builtin_ctzs for short

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> So just for future reference __builtin_c[lt]zs is for short.
> 
> But with GCC 14 (trunk) added __builtin_ct[lt]g, there is no need to add
> __builtin_c[lt]zs to GCC.
> 
> You could just do:
> 
> #if __has_builtin(__builtin_ctzg) && !__has_builtin(__builtin_ctzs)
> #define ctzs(a) __builtin_ctzg((short)a)
> #elif __has_builtin(__builtin_ctzs)
> #define ctzs(a) __builtin_ctzs((short)a)
> #else
> #error implement fall back for not both cases
> #endif

s/short/unsigned short/ above (and not really needed for the clang
__builtin_ctzs).
So
#if __has_builtin(__builtin_ctzs)
#define ctzs(a) __builtin_ctzs(a)
#elif __has_builtin(__builtin_ctzg)
#define ctzs(a) __builtin_ctzg((unsigned short)(a))
#else
...
#endif

[Bug c++/112789] Missing clang __builtin_ctzs for short

[gonzalo.gadeschi at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #6 from gnzlbg <gonzalo.gadeschi at gmail dot com> ---
Thanks for all the quick feedback!

> Also clang does not even document __builtin_ctzs anywhere ...

This builtin is documented in the list of clang bultins. You can find it by
using CTRL+F for the builtin, in that list,
https://clang.llvm.org/docs/LanguageExtensions.html, which is the first link
that Google reveals when searching for this exact builtin. Hope that helps.

> ok, and?

There is an HPC QCD app that was developed mostly using clang, and for which we
can't apply source code modifications, that we'd like to compile which GCC as
is, but currently doesn't due to this issue. 

Does that make sense?


> You could just do:

Thanks for the workaround! We were already using something similar as a
temporary stopgap, but can't use that forever. Hope that makes sense!

Thanks!

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to gnzlbg from comment #6)
> Thanks for all the quick feedback!
> 
> > Also clang does not even document __builtin_ctzs anywhere ...
> 
> This builtin is documented in the list of clang bultins. You can find it by
> using CTRL+F for the builtin, in that list,
> https://clang.llvm.org/docs/LanguageExtensions.html, which is the first link
> that Google reveals when searching for this exact builtin. Hope that helps.

Look at that part where __builtin_ctzs  is mentioned there.
It just says it is valid inside a constexpr and nothing else about the builtin.

>There is an HPC QCD app that was developed mostly using clang, and for which we can't apply source code modifications, that we'd like to compile which GCC as is, but currently doesn't due to this issue. 

Sounds like that HPC QCD app is broken. You should report it to the developers.
Also since __builtin_ctzg is added, adding __builtin_ctzs really does not make
any sense since it would just done as `#define __builtin_ctzs
__builtin_ctzg((unsigned short)a)` in the source.

[Bug c++/112789] Missing clang __builtin_ctzs for short

[gonzalo.gadeschi at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #8 from gnzlbg <gonzalo.gadeschi at gmail dot com> ---
Oh! Sorry! I see this was resolved as WONTFIX.

Does that mean that GCC, in contrast with the LLVM community - which is always
super helpful and friendly when it comes to trying to enable their toolchains
to compile SW developed with GNU - intentionally does not want to enable the
GNU toolchains to compile SW developed with clang? 

If that's the case, and this is community policy, and the person who closed
this as WONTFIX speaks for the entirety of the GNU community, such that no
patches to fix this will be accepted, then thank you for your time considering
this issue and for your prompt response!

[Bug c++/112789] Missing clang __builtin_ctzs for short

[gonzalo.gadeschi at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #9 from gnzlbg <gonzalo.gadeschi at gmail dot com> ---
> > Look at that part where __builtin_ctzs  is mentioned there.
> It just says it is valid inside a constexpr and nothing else about the builtin.

Yes, seems like a bug / pr should be opened in the llvm project to better
document what the intrinsic does. I'll do that later. 

> Sounds like that HPC QCD app is broken. 

To you maybe. To me it seems like GCC is broken, hence why I opened the bug.

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to gnzlbg from comment #8)
> Oh! Sorry! I see this was resolved as WONTFIX.
> 
> Does that mean that GCC, in contrast with the LLVM community - which is
> always super helpful and friendly when it comes to trying to enable their
> toolchains to compile SW developed with GNU - intentionally does not want to
> enable the GNU toolchains to compile SW developed with clang? 

Wait, there is a builtin that does exactly the same as what you need/wanted to
do. Is there a need for another builtin?

> 
> If that's the case, and this is community policy, and the person who closed
> this as WONTFIX speaks for the entirety of the GNU community, such that no
> patches to fix this will be accepted, then thank you for your time
> considering this issue and for your prompt response!

Since there is a new builtin included already to do the same thing as
__builtin_ctzs but in a generic fashion (added to support _BitInt and the C23
bit functions), why add another builtin that is just there.

Also reading the history on __builtin_ctzs for clang and even for GCC (it
exists as an x86_64 target builtin with a specific option but undocumented
because it was only to be used by the intrinics), you will see clang added it
as a generic builtin and didn't document the behavior or anything.
The history here seems clang added it to be compatiable with GCC's internal
intrinsics implementation and exposed it for all targets but didn't document
it.

So the situtation we are at right now, add the builtin as a non-target specific
builtin for GCC just to be compatiable with clang but we have no idea what the
semantics of the builtin is because it is not documented.

[Bug c++/112789] Missing clang __builtin_ctzs for short

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #11 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> Sounds like that HPC QCD app is broken. You should report it to the
> developers. Also since __builtin_ctzg is added, adding __builtin_ctzs really
> does not make any sense since it would just done as `#define __builtin_ctzs
> __builtin_ctzg((unsigned short)a)` in the source.

And you can achieve this in a bunch of ways without even directly modifying the
source via the command line.

[Bug c++/112789] Missing clang __builtin_ctzs for short

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Sam James from comment #11)
> (In reply to Andrew Pinski from comment #7)
> > Sounds like that HPC QCD app is broken. You should report it to the
> > developers. Also since __builtin_ctzg is added, adding __builtin_ctzs really
> > does not make any sense since it would just done as `#define __builtin_ctzs
> > __builtin_ctzg((unsigned short)a)` in the source.
> 
> And you can achieve this in a bunch of ways without even directly modifying
> the source via the command line.

One way is to do the add `-include ${PATH_TO_SRC}/clang_builtins_to_gcc.h` to
the command line and have something like:
```
#ifndef __CLANG_BUILTINS_TO_GCC_H__
#define __CLANG_BUILTINS_TO_GCC_H__

#if !__has_builtin(__builtin_ctzs)
static inline unsigned short __builtin_ctzs(unsigned short a)
{
#if __has_builtin(__builtin_ctzg)
  return __builtin_ctzg(a);
#else
  #error implement fallback
#endif
}
#endif

#endif
```
in clang_builtins_to_gcc.h .

[Bug c++/112789] Missing clang __builtin_ctzs for short

[xry111 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xry111 at gcc dot gnu.org

--- Comment #13 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
(In reply to gnzlbg from comment #8)
> Oh! Sorry! I see this was resolved as WONTFIX.
> 
> Does that mean that GCC, in contrast with the LLVM community - which is
> always super helpful and friendly when it comes to trying to enable their
> toolchains to compile SW developed with GNU - intentionally does not want to
> enable the GNU toolchains to compile SW developed with clang? 

Because Clang developers actively claim to support GNU extensions, while GCC
never has such a policy to support other extensions.  When an extension is
considered useful we port it, but in this case we already have __builtin_ctzg
as all others have mentioned.

And Clang developers also reject GNU extensions if they consider the extension
"wrong", despite they make such a "compatibility" claim.

In the future (when C23 is widely adopted) people should just use
stdc_trailing_zerosus instead of directly invoking __builtin_ctz{g,s} of
whatever compiler.

[Bug c++/112789] Missing clang __builtin_ctzs for short

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
One thing against adding these builtins (especially when we have a type-generic
builtin which can handle all types, not just some of them) is that clang has
added them completely unorthogonally.
GCC has __builtin_{ctz,clz,clrsb,ffs,popcount,parity}{,l,ll,imax,g} (the last
one in GCC 14 only), while clang has
__builtin_{ctz,clz,clrsb,ffs,popcount,parity}{,l,ll} and then just
__builtin_c{l,t}zs for unsigned short extra, but none of the other 4 similar
builtins, nor any builtin for unsigned char, etc.  Why is unsigned short that
much more important than unsigned char or unsigned __int128 or all the unsigned
_BitInt types?