Skip to content
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

[clang] Implement constexpr support for __builtin_{clzg,ctzg} #86577

Merged

Conversation

overmighty
Copy link
Contributor

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

llvmbot commented Mar 25, 2024

@llvm/pr-subscribers-clang

Author: OverMighty (overmighty)

Changes

Fixes #86549.

cc @tbaederr @nickdesaulniers


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

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+5)
  • (modified) clang/include/clang/Basic/Builtins.td (+2-2)
  • (modified) clang/lib/AST/ExprConstant.cpp (+24-4)
  • (modified) clang/test/Sema/constant-builtins-2.c (+116)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7fbe2fec6ca065..4bf40136e16a22 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,11 @@ Non-comprehensive list of changes in this release
 
 - Lambda expressions are now accepted in C++03 mode as an extension.
 
+- Added ``__builtin_clzg`` and ``__builtin_ctzg`` as type-generic alternatives
+  to ``__builtin_clz{,s,l,ll}`` and ``__builtin_ctz{,s,l,ll}`` respectively,
+  with support for any unsigned integer type. Like the previous builtins, these
+  new builtins are constexpr and may be used in constant expressions.
+
 New Compiler Flags
 ------------------
 
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 21ab9bb86d1b8c..52c0dd52c28b11 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -678,7 +678,7 @@ def Clz : Builtin, BitShort_Int_Long_LongLongTemplate {
 
 def Clzg : Builtin {
   let Spellings = ["__builtin_clzg"];
-  let Attributes = [NoThrow, Const, CustomTypeChecking];
+  let Attributes = [NoThrow, Const, Constexpr, CustomTypeChecking];
   let Prototype = "int(...)";
 }
 
@@ -690,7 +690,7 @@ def Ctz : Builtin, BitShort_Int_Long_LongLongTemplate {
 
 def Ctzg : Builtin {
   let Spellings = ["__builtin_ctzg"];
-  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 592d43597dc1b4..44a09c4f91bb45 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12354,6 +12354,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
   case Builtin::BI__builtin_clzl:
   case Builtin::BI__builtin_clzll:
   case Builtin::BI__builtin_clzs:
+  case Builtin::BI__builtin_clzg:
   case Builtin::BI__lzcnt16: // Microsoft variants of count leading-zeroes
   case Builtin::BI__lzcnt:
   case Builtin::BI__lzcnt64: {
@@ -12367,8 +12368,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
                            BuiltinOp != Builtin::BI__lzcnt &&
                            BuiltinOp != Builtin::BI__lzcnt64;
 
-    if (ZeroIsUndefined && !Val)
-      return Error(E);
+    if (!Val) {
+      if (BuiltinOp == Builtin::BI__builtin_clzg && E->getNumArgs() > 1) {
+        APSInt Fallback;
+        if (!EvaluateInteger(E->getArg(1), Fallback, Info))
+          return false;
+        return Success(Fallback, E);
+      }
+
+      if (ZeroIsUndefined)
+        return Error(E);
+    }
 
     return Success(Val.countl_zero(), E);
   }
@@ -12410,12 +12420,22 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
   case Builtin::BI__builtin_ctzll:
-  case Builtin::BI__builtin_ctzs: {
+  case Builtin::BI__builtin_ctzs:
+  case Builtin::BI__builtin_ctzg: {
     APSInt Val;
     if (!EvaluateInteger(E->getArg(0), Val, Info))
       return false;
-    if (!Val)
+
+    if (!Val) {
+      if (BuiltinOp == Builtin::BI__builtin_ctzg && E->getNumArgs() > 1) {
+        APSInt Fallback;
+        if (!EvaluateInteger(E->getArg(1), Fallback, Info))
+          return false;
+        return Success(Fallback, E);
+      }
+
       return Error(E);
+    }
 
     return Success(Val.countr_zero(), E);
   }
diff --git a/clang/test/Sema/constant-builtins-2.c b/clang/test/Sema/constant-builtins-2.c
index 6dd1d88759c751..a60a1f16a45874 100644
--- a/clang/test/Sema/constant-builtins-2.c
+++ b/clang/test/Sema/constant-builtins-2.c
@@ -218,6 +218,64 @@ char clz6[__builtin_clzll(0xFFLL) == BITSIZE(long long) - 8 ? 1 : -1];
 char clz7[__builtin_clzs(0x1) == BITSIZE(short) - 1 ? 1 : -1];
 char clz8[__builtin_clzs(0xf) == BITSIZE(short) - 4 ? 1 : -1];
 char clz9[__builtin_clzs(0xfff) == BITSIZE(short) - 12 ? 1 : -1];
+int clz10 = __builtin_clzg((unsigned char)0); // expected-error {{not a compile-time constant}}
+char clz11[__builtin_clzg((unsigned char)0, 42) == 42 ? 1 : -1];
+char clz12[__builtin_clzg((unsigned char)0x1) == BITSIZE(char) - 1 ? 1 : -1];
+char clz13[__builtin_clzg((unsigned char)0x1, 42) == BITSIZE(char) - 1 ? 1 : -1];
+char clz14[__builtin_clzg((unsigned char)0xf) == BITSIZE(char) - 4 ? 1 : -1];
+char clz15[__builtin_clzg((unsigned char)0xf, 42) == BITSIZE(char) - 4 ? 1 : -1];
+char clz16[__builtin_clzg((unsigned char)(1 << (BITSIZE(char) - 1))) == 0 ? 1 : -1];
+char clz17[__builtin_clzg((unsigned char)(1 << (BITSIZE(char) - 1)), 42) == 0 ? 1 : -1];
+int clz18 = __builtin_clzg((unsigned short)0); // expected-error {{not a compile-time constant}}
+char clz19[__builtin_clzg((unsigned short)0, 42) == 42 ? 1 : -1];
+char clz20[__builtin_clzg((unsigned short)0x1) == BITSIZE(short) - 1 ? 1 : -1];
+char clz21[__builtin_clzg((unsigned short)0x1, 42) == BITSIZE(short) - 1 ? 1 : -1];
+char clz22[__builtin_clzg((unsigned short)0xf) == BITSIZE(short) - 4 ? 1 : -1];
+char clz23[__builtin_clzg((unsigned short)0xf, 42) == BITSIZE(short) - 4 ? 1 : -1];
+char clz24[__builtin_clzg((unsigned short)(1 << (BITSIZE(short) - 1))) == 0 ? 1 : -1];
+char clz25[__builtin_clzg((unsigned short)(1 << (BITSIZE(short) - 1)), 42) == 0 ? 1 : -1];
+int clz26 = __builtin_clzg(0U); // expected-error {{not a compile-time constant}}
+char clz27[__builtin_clzg(0U, 42) == 42 ? 1 : -1];
+char clz28[__builtin_clzg(0x1U) == BITSIZE(int) - 1 ? 1 : -1];
+char clz29[__builtin_clzg(0x1U, 42) == BITSIZE(int) - 1 ? 1 : -1];
+char clz30[__builtin_clzg(0xfU) == BITSIZE(int) - 4 ? 1 : -1];
+char clz31[__builtin_clzg(0xfU, 42) == BITSIZE(int) - 4 ? 1 : -1];
+char clz32[__builtin_clzg(1U << (BITSIZE(int) - 1)) == 0 ? 1 : -1];
+char clz33[__builtin_clzg(1U << (BITSIZE(int) - 1), 42) == 0 ? 1 : -1];
+int clz34 = __builtin_clzg(0UL); // expected-error {{not a compile-time constant}}
+char clz35[__builtin_clzg(0UL, 42) == 42 ? 1 : -1];
+char clz36[__builtin_clzg(0x1UL) == BITSIZE(long) - 1 ? 1 : -1];
+char clz37[__builtin_clzg(0x1UL, 42) == BITSIZE(long) - 1 ? 1 : -1];
+char clz38[__builtin_clzg(0xfUL) == BITSIZE(long) - 4 ? 1 : -1];
+char clz39[__builtin_clzg(0xfUL, 42) == BITSIZE(long) - 4 ? 1 : -1];
+char clz40[__builtin_clzg(1UL << (BITSIZE(long) - 1)) == 0 ? 1 : -1];
+char clz41[__builtin_clzg(1UL << (BITSIZE(long) - 1), 42) == 0 ? 1 : -1];
+int clz42 = __builtin_clzg(0ULL); // expected-error {{not a compile-time constant}}
+char clz43[__builtin_clzg(0ULL, 42) == 42 ? 1 : -1];
+char clz44[__builtin_clzg(0x1ULL) == BITSIZE(long long) - 1 ? 1 : -1];
+char clz45[__builtin_clzg(0x1ULL, 42) == BITSIZE(long long) - 1 ? 1 : -1];
+char clz46[__builtin_clzg(0xfULL) == BITSIZE(long long) - 4 ? 1 : -1];
+char clz47[__builtin_clzg(0xfULL, 42) == BITSIZE(long long) - 4 ? 1 : -1];
+char clz48[__builtin_clzg(1ULL << (BITSIZE(long long) - 1)) == 0 ? 1 : -1];
+char clz49[__builtin_clzg(1ULL << (BITSIZE(long long) - 1), 42) == 0 ? 1 : -1];
+#ifdef __SIZEOF_INT128__
+int clz50 = __builtin_clzg((unsigned __int128)0); // expected-error {{not a compile-time constant}}
+char clz51[__builtin_clzg((unsigned __int128)0, 42) == 42 ? 1 : -1];
+char clz52[__builtin_clzg((unsigned __int128)0x1) == BITSIZE(__int128) - 1 ? 1 : -1];
+char clz53[__builtin_clzg((unsigned __int128)0x1, 42) == BITSIZE(__int128) - 1 ? 1 : -1];
+char clz54[__builtin_clzg((unsigned __int128)0xf) == BITSIZE(__int128) - 4 ? 1 : -1];
+char clz55[__builtin_clzg((unsigned __int128)0xf, 42) == BITSIZE(__int128) - 4 ? 1 : -1];
+char clz56[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1))) == 0 ? 1 : -1];
+char clz57[__builtin_clzg((unsigned __int128)(1 << (BITSIZE(__int128) - 1)), 42) == 0 ? 1 : -1];
+#endif
+int clz58 = __builtin_clzg((unsigned _BitInt(128))0); // expected-error {{not a compile-time constant}}
+char clz59[__builtin_clzg((unsigned _BitInt(128))0, 42) == 42 ? 1 : -1];
+char clz60[__builtin_clzg((unsigned _BitInt(128))0x1) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
+char clz61[__builtin_clzg((unsigned _BitInt(128))0x1, 42) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
+char clz62[__builtin_clzg((unsigned _BitInt(128))0xf) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
+char clz63[__builtin_clzg((unsigned _BitInt(128))0xf, 42) == BITSIZE(_BitInt(128)) - 4 ? 1 : -1];
+char clz64[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1))) == 0 ? 1 : -1];
+char clz65[__builtin_clzg((unsigned _BitInt(128))(1 << (BITSIZE(_BitInt(128)) - 1)), 42) == 0 ? 1 : -1];
 
 char ctz1[__builtin_ctz(1) == 0 ? 1 : -1];
 char ctz2[__builtin_ctz(8) == 3 ? 1 : -1];
@@ -226,6 +284,64 @@ int ctz4 = __builtin_ctz(0); // expected-error {{not a compile-time constant}}
 char ctz5[__builtin_ctzl(0x10L) == 4 ? 1 : -1];
 char ctz6[__builtin_ctzll(0x100LL) == 8 ? 1 : -1];
 char ctz7[__builtin_ctzs(1 << (BITSIZE(short) - 1)) == BITSIZE(short) - 1 ? 1 : -1];
+int ctz8 = __builtin_ctzg((unsigned char)0); // expected-error {{not a compile-time constant}}
+char ctz9[__builtin_ctzg((unsigned char)0, 42) == 42 ? 1 : -1];
+char ctz10[__builtin_ctzg((unsigned char)0x1) == 0 ? 1 : -1];
+char ctz11[__builtin_ctzg((unsigned char)0x1, 42) == 0 ? 1 : -1];
+char ctz12[__builtin_ctzg((unsigned char)0x10) == 4 ? 1 : -1];
+char ctz13[__builtin_ctzg((unsigned char)0x10, 42) == 4 ? 1 : -1];
+char ctz14[__builtin_ctzg((unsigned char)(1 << (BITSIZE(char) - 1))) == BITSIZE(char) - 1 ? 1 : -1];
+char ctz15[__builtin_ctzg((unsigned char)(1 << (BITSIZE(char) - 1)), 42) == BITSIZE(char) - 1 ? 1 : -1];
+int ctz16 = __builtin_ctzg((unsigned short)0); // expected-error {{not a compile-time constant}}
+char ctz17[__builtin_ctzg((unsigned short)0, 42) == 42 ? 1 : -1];
+char ctz18[__builtin_ctzg((unsigned short)0x1) == 0 ? 1 : -1];
+char ctz19[__builtin_ctzg((unsigned short)0x1, 42) == 0 ? 1 : -1];
+char ctz20[__builtin_ctzg((unsigned short)0x10) == 4 ? 1 : -1];
+char ctz21[__builtin_ctzg((unsigned short)0x10, 42) == 4 ? 1 : -1];
+char ctz22[__builtin_ctzg((unsigned short)(1 << (BITSIZE(short) - 1))) == BITSIZE(short) - 1 ? 1 : -1];
+char ctz23[__builtin_ctzg((unsigned short)(1 << (BITSIZE(short) - 1)), 42) == BITSIZE(short) - 1 ? 1 : -1];
+int ctz24 = __builtin_ctzg(0U); // expected-error {{not a compile-time constant}}
+char ctz25[__builtin_ctzg(0U, 42) == 42 ? 1 : -1];
+char ctz26[__builtin_ctzg(0x1U) == 0 ? 1 : -1];
+char ctz27[__builtin_ctzg(0x1U, 42) == 0 ? 1 : -1];
+char ctz28[__builtin_ctzg(0x10U) == 4 ? 1 : -1];
+char ctz29[__builtin_ctzg(0x10U, 42) == 4 ? 1 : -1];
+char ctz30[__builtin_ctzg(1U << (BITSIZE(int) - 1)) == BITSIZE(int) - 1 ? 1 : -1];
+char ctz31[__builtin_ctzg(1U << (BITSIZE(int) - 1), 42) == BITSIZE(int) - 1 ? 1 : -1];
+int ctz32 = __builtin_ctzg(0UL); // expected-error {{not a compile-time constant}}
+char ctz33[__builtin_ctzg(0UL, 42) == 42 ? 1 : -1];
+char ctz34[__builtin_ctzg(0x1UL) == 0 ? 1 : -1];
+char ctz35[__builtin_ctzg(0x1UL, 42) == 0 ? 1 : -1];
+char ctz36[__builtin_ctzg(0x10UL) == 4 ? 1 : -1];
+char ctz37[__builtin_ctzg(0x10UL, 42) == 4 ? 1 : -1];
+char ctz38[__builtin_ctzg(1UL << (BITSIZE(long) - 1)) == BITSIZE(long) - 1 ? 1 : -1];
+char ctz39[__builtin_ctzg(1UL << (BITSIZE(long) - 1), 42) == BITSIZE(long) - 1 ? 1 : -1];
+int ctz40 = __builtin_ctzg(0ULL); // expected-error {{not a compile-time constant}}
+char ctz41[__builtin_ctzg(0ULL, 42) == 42 ? 1 : -1];
+char ctz42[__builtin_ctzg(0x1ULL) == 0 ? 1 : -1];
+char ctz43[__builtin_ctzg(0x1ULL, 42) == 0 ? 1 : -1];
+char ctz44[__builtin_ctzg(0x10ULL) == 4 ? 1 : -1];
+char ctz45[__builtin_ctzg(0x10ULL, 42) == 4 ? 1 : -1];
+char ctz46[__builtin_ctzg(1ULL << (BITSIZE(long long) - 1)) == BITSIZE(long long) - 1 ? 1 : -1];
+char ctz47[__builtin_ctzg(1ULL << (BITSIZE(long long) - 1), 42) == BITSIZE(long long) - 1 ? 1 : -1];
+#ifdef __SIZEOF_INT128__
+int ctz48 = __builtin_ctzg((unsigned __int128)0); // expected-error {{not a compile-time constant}}
+char ctz49[__builtin_ctzg((unsigned __int128)0, 42) == 42 ? 1 : -1];
+char ctz50[__builtin_ctzg((unsigned __int128)0x1) == 0 ? 1 : -1];
+char ctz51[__builtin_ctzg((unsigned __int128)0x1, 42) == 0 ? 1 : -1];
+char ctz52[__builtin_ctzg((unsigned __int128)0x10) == 4 ? 1 : -1];
+char ctz53[__builtin_ctzg((unsigned __int128)0x10, 42) == 4 ? 1 : -1];
+char ctz54[__builtin_ctzg((unsigned __int128)1 << (BITSIZE(__int128) - 1)) == BITSIZE(__int128) - 1 ? 1 : -1];
+char ctz55[__builtin_ctzg((unsigned __int128)1 << (BITSIZE(__int128) - 1), 42) == BITSIZE(__int128) - 1 ? 1 : -1];
+#endif
+int ctz56 = __builtin_ctzg((unsigned _BitInt(128))0); // expected-error {{not a compile-time constant}}
+char ctz57[__builtin_ctzg((unsigned _BitInt(128))0, 42) == 42 ? 1 : -1];
+char ctz58[__builtin_ctzg((unsigned _BitInt(128))0x1) == 0 ? 1 : -1];
+char ctz59[__builtin_ctzg((unsigned _BitInt(128))0x1, 42) == 0 ? 1 : -1];
+char ctz60[__builtin_ctzg((unsigned _BitInt(128))0x10) == 4 ? 1 : -1];
+char ctz61[__builtin_ctzg((unsigned _BitInt(128))0x10, 42) == 4 ? 1 : -1];
+char ctz62[__builtin_ctzg((unsigned _BitInt(128))1 << (BITSIZE(_BitInt(128)) - 1)) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
+char ctz63[__builtin_ctzg((unsigned _BitInt(128))1 << (BITSIZE(_BitInt(128)) - 1), 42) == BITSIZE(_BitInt(128)) - 1 ? 1 : -1];
 
 char popcount1[__builtin_popcount(0) == 0 ? 1 : -1];
 char popcount2[__builtin_popcount(0xF0F0) == 8 ? 1 : -1];

Comment on lines +191 to +193
- Added ``__builtin_clzg`` and ``__builtin_ctzg`` as type-generic alternatives
to ``__builtin_clz{,s,l,ll}`` and ``__builtin_ctz{,s,l,ll}`` respectively,
with support for any unsigned integer type. Like the previous builtins, these
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had forgotten to add a release note for the addition of these builtins in #83431.

Copy link

✅ With the latest revision this PR passed the Python code formatter.

Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the patch!

clang/lib/AST/ExprConstant.cpp Show resolved Hide resolved
clang/lib/AST/ExprConstant.cpp Show resolved Hide resolved
clang/lib/AST/ExprConstant.cpp Show resolved Hide resolved
Comment on lines 12373 to 12374
APSInt Fallback;
if (!EvaluateInteger(E->getArg(1), Fallback, Info))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just reuse Val here; we don't need extra storage for Fallback.

Comment on lines 12431 to 12432
APSInt Fallback;
if (!EvaluateInteger(E->getArg(1), Fallback, Info))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reuse Val?

return Success(Fallback, E);
}

if (ZeroIsUndefined)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sink the definition of ZeroIsUndefined closer to the use to reduce its scope. We might not need to evaluate that expression.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! are you able to merge or do you need me to?

@overmighty
Copy link
Contributor Author

overmighty commented Mar 26, 2024

I need you to merge this for me, please. Thanks.

@nickdesaulniers nickdesaulniers merged commit ac1af75 into llvm:main Mar 26, 2024
5 checks passed
BuiltinOp != Builtin::BI__lzcnt64;
if (!Val) {
if (BuiltinOp == Builtin::BI__builtin_clzg && E->getNumArgs() > 1) {
if (!EvaluateInteger(E->getArg(1), Val, Info))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This EvaluateInteger call can't be guarded by the if (!Val) check: even if we don't actually use the value of the second argument, we have to evaluate it for side-effects. We don't want short-circuit the evaluation like for &&/||.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I will fix this now.

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.

[clang] __builtin_clzg / __builtin_ctzg aren't usable in constexpr contexts
4 participants