Skip to content

[clang] Implement constexpr support for __builtin_popcountg #84318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 8, 2024

Conversation

overmighty
Copy link
Member

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 7, 2024
@overmighty
Copy link
Member Author

cc @RKSimon

@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2024

@llvm/pr-subscribers-clang

Author: OverMighty (overmighty)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/84318.diff

4 Files Affected:

  • (modified) clang/docs/LanguageExtensions.rst (+1)
  • (modified) clang/include/clang/Basic/Builtins.td (+1-1)
  • (modified) clang/lib/AST/ExprConstant.cpp (+1)
  • (modified) clang/test/Sema/constant-builtins-2.c (+7)
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 2b54dffd058a35..06af93fd3c15ca 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -5378,6 +5378,7 @@ The following builtin intrinsics can be used in constant expressions:
 * ``__builtin_popcount``
 * ``__builtin_popcountl``
 * ``__builtin_popcountll``
+* ``__builtin_popcountg``
 * ``__builtin_rotateleft8``
 * ``__builtin_rotateleft16``
 * ``__builtin_rotateleft32``
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index a81131d82c4cb4..9c703377ca8d3e 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -706,7 +706,7 @@ def Popcount : Builtin, BitInt_Long_LongLongTemplate {
 
 def Popcountg : Builtin {
   let Spellings = ["__builtin_popcountg"];
-  let Attributes = [NoThrow, Const, CustomTypeChecking];
+  let Attributes = [NoThrow, Const, Constexpr, CustomTypeChecking];
   let Prototype = "int(...)";
 }
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index d8ca35740fbc35..4a7c7755e1d6fd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12483,6 +12483,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
   case Builtin::BI__builtin_popcountll:
+  case Builtin::BI__builtin_popcountg:
   case Builtin::BI__popcnt16: // Microsoft variants of popcount
   case Builtin::BI__popcnt:
   case Builtin::BI__popcnt64: {
diff --git a/clang/test/Sema/constant-builtins-2.c b/clang/test/Sema/constant-builtins-2.c
index 2bdd7b06daabfe..9ac0ddf94be4fe 100644
--- a/clang/test/Sema/constant-builtins-2.c
+++ b/clang/test/Sema/constant-builtins-2.c
@@ -237,6 +237,13 @@ char popcount7[__builtin_popcountl(~0L) == BITSIZE(long) ? 1 : -1];
 char popcount8[__builtin_popcountll(0LL) == 0 ? 1 : -1];
 char popcount9[__builtin_popcountll(0xF0F0LL) == 8 ? 1 : -1];
 char popcount10[__builtin_popcountll(~0LL) == BITSIZE(long long) ? 1 : -1];
+char popcount11[__builtin_popcountg(0U) == 0 ? 1 : -1];
+char popcount12[__builtin_popcountg(0xF0F0U) == 8 ? 1 : -1];
+char popcount13[__builtin_popcountg(~0U) == BITSIZE(int) ? 1 : -1];
+char popcount14[__builtin_popcountg(~0UL) == BITSIZE(long) ? 1 : -1];
+char popcount15[__builtin_popcountg(~0ULL) == BITSIZE(long long) ? 1 : -1];
+char popcount15[__builtin_popcountg(~(unsigned __int128)0) == BITSIZE(__int128) ? 1 : -1];
+char popcount16[__builtin_popcountg(~(unsigned _BitInt(128))0) == BITSIZE(_BitInt(128)) ? 1 : -1];
 
 char parity1[__builtin_parity(0) == 0 ? 1 : -1];
 char parity2[__builtin_parity(0xb821) == 0 ? 1 : -1];

@Sirraide
Copy link
Member

Sirraide commented Mar 7, 2024

This needs a release note, but LGTM otherwise.

@Sirraide
Copy link
Member

Sirraide commented Mar 7, 2024

I’ll wait for CI to finish building the docs and then merge this; it was passing before, so I don’t think we need to wait for it again as the only thing that has changed since then is the release notes.

@Sirraide
Copy link
Member

Sirraide commented Mar 7, 2024

I see the name of one of the arrays was wrong; we really should just refactor this entire test to just use static_assert; not in this pr, though.

@overmighty
Copy link
Member Author

Yes, I had actually caught the mistake but somehow reintroduced it at some point.

@Sirraide Sirraide merged commit 487cfbe into llvm:main Mar 8, 2024
@overmighty
Copy link
Member Author

Usage of __int128 in the test broke the clang-armv8-quick build: https://lab.llvm.org/buildbot/#/builders/245/builds/21611/steps/5/logs/FAIL__Clang__constant-builtins-2_c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants