[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[sjames at gcc dot gnu.org via Gcc-bugs]

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

Sam James  changed:

   What|Removed |Added

 CC||sjames at gcc dot gnu.org
   See Also||https://gcc.gnu.org/bugzill
   ||a/show_bug.cgi?id=112789

--- Comment #18 from Sam James  ---
(In reply to Allan Jensen from comment #8)
> Thanks that looks good. I will test it when I have a chance. I am changing
> the Qt sources to not assume the presence of __builtin_clzs when __BMI__ is
> defined. It can use __builtin_clz() and __builtin_ctz()-16U instead, but for
> general compatibility it is nice that GCC also keeps it around. 
> 
> Note, it would be even better though if GCC could support the short forms as
> generic builtins. That changes the semantics slightly, but they are named so
> similarly to the clz, clzl and clzll it would be easy to assume they also
> are generics, with similar semantics, and can work across all targets.
> 

GCC 14 will have __builtin_clzg, if you want to start using that:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112789#c4.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[uros at gcc dot gnu.org]

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

--- Comment #17 from uros at gcc dot gnu.org ---
Author: uros
Date: Thu Jun  8 19:42:59 2017
New Revision: 249039

URL: https://gcc.gnu.org/viewcvs?rev=249039=gcc=rev
Log:
PR target/81015
Revert:
2016-12-14  Uros Bizjak  

PR target/59874
* config/i386/i386.md (*ctzhi2): New insn_and_split pattern.
(*clzhi2): Ditto.

testsuite/ChangeLog:

PR target/81015
* gcc.target/i386/pr59874-1.c (foo): Call __builtin_ctzs.
* gcc.target/i386/pr59874-2.c (foo): Call __builtin_clzs.
* gcc.target/i386/pr81015.c: New test.


Added:
branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr81015.c
Modified:
branches/gcc-7-branch/gcc/ChangeLog
branches/gcc-7-branch/gcc/config/i386/i386.md
branches/gcc-7-branch/gcc/testsuite/ChangeLog
branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr59874-1.c
branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr59874-2.c

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[uros at gcc dot gnu.org]

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

--- Comment #16 from uros at gcc dot gnu.org ---
Author: uros
Date: Thu Jun  8 17:28:19 2017
New Revision: 249028

URL: https://gcc.gnu.org/viewcvs?rev=249028=gcc=rev
Log:
PR target/81015
Revert:
2016-12-14  Uros Bizjak  

PR target/59874
* config/i386/i386.md (*ctzhi2): New insn_and_split pattern.
(*clzhi2): Ditto.

testsuite/ChangeLog:

PR target/81015
* gcc.target/i386/pr59874-1.c (foo): Call __builtin_ctzs.
* gcc.target/i386/pr59874-2.c (foo): Call __builtin_clzs.
* gcc.target/i386/pr81015.c: New test.


Added:
trunk/gcc/testsuite/gcc.target/i386/pr81015.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/i386/i386.md
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/i386/pr59874-1.c
trunk/gcc/testsuite/gcc.target/i386/pr59874-2.c

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[linux at carewolf dot com]

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

--- Comment #15 from Allan Jensen  ---
Yes, the patch works and it also evaluates at compile time.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

Uroš Bizjak  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED
   Target Milestone|--- |6.3

--- Comment #14 from Uroš Bizjak  ---
Fixed.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

--- Comment #13 from Uroš Bizjak  ---
(In reply to Allan Jensen from comment #8)
> Thanks that looks good. I will test it when I have a chance. I am changing
> the Qt sources to not assume the presence of __builtin_clzs when __BMI__ is
> defined. It can use __builtin_clz() and __builtin_ctz()-16U instead, but for
> general compatibility it is nice that GCC also keeps it around. 

There are now several possibilities:

a) Use apparently de-facto standard __builtin_clzs and __builtin_ctzs.

b) (gcc-7+) You can use documented __builtin_clz/__builtin_ctz builtins with
operand casted to (unsigned short). When instruction is available, the compiler
synthesizes 16bit insn variant from 32bit insn and zero-extension, as evident
from gcc.target/i386/pr59874-?.c test cases. This is the most portable
solution.

c) (gcc-6.3+) Figure out how to use __lzcnt16/__tzcnt_u16, as suggested in
Comment #3. This is the optimal solution, as checks for zero operand are not
needed.

> Note, it would be even better though if GCC could support the short forms as
> generic builtins. That changes the semantics slightly, but they are named so
> similarly to the clz, clzl and clzll it would be easy to assume they also
> are generics, with similar semantics, and can work across all targets.
> 
> Btw. I assume __builtin_clzs being a target specific builtin, that GCC never
> had the capability of resolving it at compile-time? If that is the case, it
> might actually be a bug that GCC allowed it at all in a constexpr function.

Hm, I'm not versed in c++, but the provided headers that define various
intrinsics are used in various c++ projects. In gcc-6.3+, the compiler folds
these two builtins with constant operand at compile time, so please retry your
tests with a newer compiler version.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

--- Comment #12 from Uroš Bizjak  ---
The above patch also reintroduces reintroduces __builtin_clzs and
__builtin_ctzs to mainline.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[uros at gcc dot gnu.org]

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

--- Comment #11 from uros at gcc dot gnu.org ---
Author: uros
Date: Wed Dec 14 20:27:06 2016
New Revision: 243668

URL: https://gcc.gnu.org/viewcvs?rev=243668=gcc=rev
Log:
PR target/59874
* config/i386/i386-builtin.def: Add __builtin_clzs and __builtin_ctzs.
(ix86_fold_builtin): Handle IX86_BUILTIN_CTZS and IX86_BUILTIN_CLZS.
* config/i386/i386.md (*ctzhi2): New insn_and_split pattern.
(*clzhi2): Ditto.

testsuite/ChangeLog

PR target/59874
* gcc.target/i386/pr59874-1.c: New test.
* gcc.target/i386/pr59874-2.c: Ditto.


Added:
trunk/gcc/testsuite/gcc.target/i386/pr59874-1.c
trunk/gcc/testsuite/gcc.target/i386/pr59874-2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/i386/i386-builtin.def
trunk/gcc/config/i386/i386.c
trunk/gcc/config/i386/i386.md
trunk/gcc/testsuite/ChangeLog

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

--- Comment #10 from Uroš Bizjak  ---
(In reply to uros from comment #9)
> Author: uros
> Date: Wed Dec 14 15:44:07 2016
> New Revision: 243651

This patch reintroduces __builtin_clzs and __builtin_ctzs to gcc-6 branch to
resolve compatibility issues on gcc-6 branch.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[uros at gcc dot gnu.org]

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

--- Comment #9 from uros at gcc dot gnu.org ---
Author: uros
Date: Wed Dec 14 15:44:07 2016
New Revision: 243651

URL: https://gcc.gnu.org/viewcvs?rev=243651=gcc=rev
Log:
PR target/59874
* config/i386/i386.c (enum ix86_builtins): Add IX86_BUILTIN_CLZS
and IX86_BUILTIN_CTZS.
(bdesc_args): Add __builtin_clzs and __builtin_ctzs.
(ix86_fold_builtin): Handle IX86_BUILTIN_CTZS and IX86_BUILTIN_CLZS.


Modified:
branches/gcc-6-branch/gcc/ChangeLog
branches/gcc-6-branch/gcc/config/i386/i386.c

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[linux at carewolf dot com]

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

--- Comment #8 from Allan Jensen  ---
Thanks that looks good. I will test it when I have a chance. I am changing the
Qt sources to not assume the presence of __builtin_clzs when __BMI__ is
defined. It can use __builtin_clz() and __builtin_ctz()-16U instead, but for
general compatibility it is nice that GCC also keeps it around. 

Note, it would be even better though if GCC could support the short forms as
generic builtins. That changes the semantics slightly, but they are named so
similarly to the clz, clzl and clzll it would be easy to assume they also are
generics, with similar semantics, and can work across all targets.

Btw. I assume __builtin_clzs being a target specific builtin, that GCC never
had the capability of resolving it at compile-time? If that is the case, it
might actually be a bug that GCC allowed it at all in a constexpr function.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

Uroš Bizjak  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2016-12-13
   Assignee|unassigned at gcc dot gnu.org  |ubizjak at gmail dot com
 Ever confirmed|0   |1

--- Comment #7 from Uroš Bizjak  ---
Created attachment 40323
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40323=edit
Patch that adds __builtin_{clzs,ctzs}

Does the attached patch (vs gcc-7) works for you?

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

--- Comment #6 from Uroš Bizjak  ---
(In reply to Allan Jensen from comment #5)
> This is more problematic to fix in Qt itself. How can we determine if we
> should/can use __builtin_clzs or __lzcnt16?
> 
> Note the former is practically standard being supported by both older gcc
> and clang. There is also the problem that we need to call a builtin, because
> the C-intrinsics don't work as constexpr.

Well, if the change makes so much pain, let's put these intrinsics back.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[linux at carewolf dot com]

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

Allan Jensen  changed:

   What|Removed |Added

 CC||linux at carewolf dot com

--- Comment #5 from Allan Jensen  ---
This is more problematic to fix in Qt itself. How can we determine if we
should/can use __builtin_clzs or __lzcnt16?

Note the former is practically standard being supported by both older gcc and
clang. There is also the problem that we need to call a builtin, because the
C-intrinsics don't work as constexpr.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[manuel.lauss at googlemail dot com]

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

--- Comment #4 from Manuel Lauss  ---
(In reply to Uroš Bizjak from comment #3)
> (In reply to Manuel Lauss from comment #2)
> 
> > I've run into this issue with current gcc-6-branch while building for
> > example qt-5.7.  gcc-5-branch can build this source just fine:
> 
> __builtin_clzs and __builtin_ctzs are internal x86 builtins and were
> recently removed. You should use __lzcnt16 and __tzcnt_u16 intrinsics.

Done,

> 
> > return v ? __builtin_ctzs(v) : 16U;
> >  return v ? __builtin_clzs(v) : 16U;
> 
> You don't need this, the instruction does exactly the above.

without it there are errors with missing constexprs.

Thanks, I've successfully built qt-5.7 with updated source.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[ubizjak at gmail dot com]

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

--- Comment #3 from Uroš Bizjak  ---
(In reply to Manuel Lauss from comment #2)

> I've run into this issue with current gcc-6-branch while building for
> example qt-5.7.  gcc-5-branch can build this source just fine:

__builtin_clzs and __builtin_ctzs are internal x86 builtins and were recently
removed. You should use __lzcnt16 and __tzcnt_u16 intrinsics.

> return v ? __builtin_ctzs(v) : 16U;
>  return v ? __builtin_clzs(v) : 16U;

You don't need this, the instruction does exactly the above.

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[manuel.lauss at googlemail dot com]

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

Manuel Lauss  changed:

   What|Removed |Added

 CC||manuel.lauss at googlemail dot 
com

--- Comment #2 from Manuel Lauss  ---
Hello,

I've run into this issue with current gcc-6-branch while building for example
qt-5.7.  gcc-5-branch can build this source just fine:

x86_64-pc-linux-gnu-g++ -c -o qmakeparser.o -O3 -march=haswell -mtune=haswell
-fno-lifetime-dse -fno-delete-null-pointer-checks -pipe -std=c++11
-ffunction-sections 
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/library
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/generators
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/generators/unix
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/generators/win32
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/generators/mac
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/generators/integrity
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/5.7.0
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/5.7.0/QtCore
-I../src/corelib/global -DHAVE_QCONFIG_CPP
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/mkspecs/linux-g++
-I/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/tools/shared
-DQT_VERSION_STR=\"5.7.0\" -DQT_VERSION_MAJOR=5 -DQT_VERSION_MINOR=7
-DQT_VERSION_PATCH=0 -DQT_BUILD_QMAKE -DQT_BOOTSTRAPPED -DPROEVALUATOR_FULL
-DQT_NO_TEXTCODEC -DQT_NO_UNICODETABLES -DQT_NO_COMPONENT -DQT_NO_COMPRESS
-DQT_NO_THREAD -DQT_NO_QOBJECT -DQT_NO_GEOM_VARIANT -DQT_NO_DATASTREAM
-DQT_NO_FOREACH -DQT_CRYPTOGRAPHICHASH_ONLY_SHA1 -DQT_JSON_READONLY
-DQT_NO_STANDARDPATHS
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/library/qmakeparser.cpp
In file included from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/qalgorithms.h:1:0,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/../../src/corelib/tools/qvector.h:43,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/qvector.h:1,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/library/proitems.h:35,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/library/qmakeparser.h:33,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/library/qmakeevaluator.h:36,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/project.h:32,
 from
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/qmake/project.cpp:29:
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/../../src/corelib/tools/qalgorithms.h:
In function 'uint qCountTrailingZeroBits(quint16)':
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/../../src/corelib/tools/qalgorithms.h:630:32:
error: '__builtin_ctzs' was not declared in this scope
 return v ? __builtin_ctzs(v) : 16U;
^
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/../../src/corelib/tools/qalgorithms.h:
In function 'uint qCountLeadingZeroBits(quint16)':
/tmp-ram/portage/dev-qt/qtcore-5.7.0/work/qtbase-opensource-src-5.7.0/include/QtCore/../../src/corelib/tools/qalgorithms.h:693:32:
error: '__builtin_clzs' was not declared in this scope
 return v ? __builtin_clzs(v) : 16U;

[Bug target/59874] Missing builtin (__builtin_clzs) when compiling with g++

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59874

Richard Biener rguenth at gcc dot gnu.org changed:

   What|Removed |Added

 Target||x86_64-*-*
  Component|c++ |target

--- Comment #1 from Richard Biener rguenth at gcc dot gnu.org ---
It seems

#pragma GCC push_options
#pragma GCC target(lzcnt)

from lzcntintrin.h doesn't work with C++?