[clang] Implement __builtin_popcountg

[overmighty]

Fixes #82058.

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: OverMighty (overmighty)

Changes

Fixes #82058.

---

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

3 Files Affected:

• (modified) clang/include/clang/Basic/Builtins.td (+6)
• (modified) clang/lib/CodeGen/CGBuiltin.cpp (+6-1)
• (modified) clang/test/CodeGen/builtins.c (+43)

diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index df74026c5d2d50..3134450250edd5 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -688,6 +688,12 @@ def Popcount : Builtin, BitInt_Long_LongLongTemplate {
   let Prototype = "int(unsigned T)";
 }
 
+def Popcountg : Builtin {
+  let Spellings = ["__builtin_popcountg"];
+  let Attributes = [NoThrow, Const, Constexpr];
+  let Prototype = "int(...)";
+}
+
 def Clrsb : Builtin, BitInt_Long_LongLongTemplate {
   let Spellings = ["__builtin_clrsb"];
   let Attributes = [NoThrow, Const, Constexpr];
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index d454ccc1dd8613..d36d24e84f0afa 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3216,7 +3216,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   case Builtin::BI__popcnt64:
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
-  case Builtin::BI__builtin_popcountll: {
+  case Builtin::BI__builtin_popcountll:
+  case Builtin::BI__builtin_popcountg: {
+    if (BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_popcountg) {
+      assert(E->getNumArgs() == 1 && "__builtin_popcountg takes 1 argument");
+    }
+
     Value *ArgValue = EmitScalarExpr(E->getArg(0));
 
     llvm::Type *ArgType = ArgValue->getType();
diff --git a/clang/test/CodeGen/builtins.c b/clang/test/CodeGen/builtins.c
index 88282120283b8a..73866116e07e72 100644
--- a/clang/test/CodeGen/builtins.c
+++ b/clang/test/CodeGen/builtins.c
@@ -940,4 +940,47 @@ void test_builtin_os_log_long_double(void *buf, long double ld) {
 // CHECK: %[[V3:.*]] = load i128, ptr %[[ARG0_ADDR]], align 16
 // CHECK: store i128 %[[V3]], ptr %[[ARGDATA]], align 1
 
+// CHECK-LABEL: define{{.*}} void @test_builtin_popcountg
+void test_builtin_popcountg(unsigned char uc, unsigned short us,
+                            unsigned int ui, unsigned long ul,
+                            unsigned long long ull, unsigned __int128 ui128,
+                            unsigned _BitInt(128) ubi128) {
+  volatile int pop;
+  pop = __builtin_popcountg(uc);
+  // CHECK: %1 = load i8, ptr %uc.addr, align 1
+  // CHECK-NEXT: %conv = zext i8 %1 to i32
+  // CHECK-NEXT: %2 = call i32 @llvm.ctpop.i32(i32 %conv)
+  // CHECK-NEXT: store volatile i32 %2, ptr %pop, align 4
+  pop = __builtin_popcountg(us);
+  // CHECK-NEXT: %3 = load i16, ptr %us.addr, align 2
+  // CHECK-NEXT: %conv1 = zext i16 %3 to i32
+  // CHECK-NEXT: %4 = call i32 @llvm.ctpop.i32(i32 %conv1)
+  // CHECK-NEXT: store volatile i32 %4, ptr %pop, align 4
+  pop = __builtin_popcountg(ui);
+  // CHECK-NEXT: %5 = load i32, ptr %ui.addr, align 4
+  // CHECK-NEXT: %6 = call i32 @llvm.ctpop.i32(i32 %5)
+  // CHECK-NEXT: store volatile i32 %6, ptr %pop, align 4
+  pop = __builtin_popcountg(ul);
+  // CHECK-NEXT: %7 = load i64, ptr %ul.addr, align 8
+  // CHECK-NEXT: %8 = call i64 @llvm.ctpop.i64(i64 %7)
+  // CHECK-NEXT: %cast = trunc i64 %8 to i32
+  // CHECK-NEXT: store volatile i32 %cast, ptr %pop, align 4
+  pop = __builtin_popcountg(ull);
+  // CHECK-NEXT: %9 = load i64, ptr %ull.addr, align 8
+  // CHECK-NEXT: %10 = call i64 @llvm.ctpop.i64(i64 %9)
+  // CHECK-NEXT: %cast2 = trunc i64 %10 to i32
+  // CHECK-NEXT: store volatile i32 %cast2, ptr %pop, align 4
+  pop = __builtin_popcountg(ui128);
+  // CHECK-NEXT: %11 = load i128, ptr %ui128.addr, align 16
+  // CHECK-NEXT: %12 = call i128 @llvm.ctpop.i128(i128 %11)
+  // CHECK-NEXT: %cast3 = trunc i128 %12 to i32
+  // CHECK-NEXT: store volatile i32 %cast3, ptr %pop, align 4
+  pop = __builtin_popcountg(ubi128);
+  // CHECK-NEXT: %13 = load i128, ptr %ubi128.addr, align 8
+  // CHECK-NEXT: %14 = call i128 @llvm.ctpop.i128(i128 %13)
+  // CHECK-NEXT: %cast4 = trunc i128 %14 to i32
+  // CHECK-NEXT: store volatile i32 %cast4, ptr %pop, align 4
+  // CHECK-NEXT: ret void
+}
+
 #endif

[overmighty]

Should I add support for __builtin_popcountg in constant expressions in this PR too or in a later one?

[efriedma-quic]

Missing semantic analysis. Since the signature is unspecified in Builtin.td, you have to check that there's one argument, and that argument is an integer. That code should go in SemaChecking.cpp.

You can leave constant evaluation to a followup, sure.

[nickdesaulniers]

Awesome, thanks!

Don't forget to update https://clang.llvm.org/docs/LanguageExtensions.html#builtin-functions

[RKSimon]

Please can you add constexpr test coverage?

[overmighty]

@RKSimon The builtin currently can't be used with constexpr. Support for constant evaluation is planned for a follow-up PR unless you would like me to add it in this one. Should I remove the Constexpr attribute from the builtin in Builtins.td for now?

[RKSimon]

@RKSimon The builtin currently can't be used with constexpr. Support for constant evaluation is planned for a follow-up PR unless you would like me to add it in this one. Should I remove the Constexpr attribute from the builtin in Builtins.td for now?

Yes please!

[efriedma-quic]

LGTM

[efriedma-quic]

Please fix your GitHub settings so your email address is public. Looks like the bot didn't trigger for some reason; see #82429 (comment) for the normal message.

[overmighty]

Done.

[nickdesaulniers]

Perhaps worth rebasing then force pushing to this branch to rekick the build bots?

Thanks for implementing this @overmighty ! There's a few more builtins like this that would simplify implementations of more C23 library additions. Let me know if you're interested in those, too.

[overmighty]

Rebased.

@nickdesaulniers I am interested in implementing more builtins like this one. :)

[nickdesaulniers]

Looks like the linter is upset with pre-existing changes. ok2land. Are you able to merge this @overmighty or do you need us to?

[overmighty]

@nickdesaulniers I need someone to merge this for me, I don't have write/push access. Thanks.

[nickdesaulniers]

@nickdesaulniers I am interested in implementing more builtins like this one. :)

Cool, I've filed:

1. [clang] implement __builtin_clzg #83075
2. [clang] implement __builtin_ctzg #83076

Those would be helpful to have next, and I suspect as straight foward to implement as popcountg was.

[overmighty]

Maybe the semantic analysis and docs should have been changed to match GCC's better.

GCC's docs say their __builtin_popcountg takes a "type-generic unsigned integer" but that "No integral argument promotions are performed on the argument." Our builtin accepts signed integers too, but the argument goes through integer promotion, so the builtin can return an incorrect value if we give it a signed integer.

For example, this program will output 32 with Clang on x86_64-unknown-linux-gnu:

#include <stdio.h>

int main(void) {
    char foo = -1;
    int pop = __builtin_popcountg(foo);
    printf("%d\n", pop);
}

GCC refuses to compile it: https://godbolt.org/z/chh4WGT4v.

Should I open a new issue?

[efriedma-quic]

Oh, I should have caught that when reviewing. (I thought I checked the test was doing the right thing when I looked at it, but I guess I'm misremembering.)

Probably need to mark the intrinsic CustomTypeChecking, then make the code in SemaChecking explicitly perform lvalue-to-rvalue conversions, to match gcc.

Yes, please open an issue... or if you're planning to look at it, feel free to just post a fix.

[overmighty]

Thanks. I plan to fix it soon, so I will probably just open a PR then.

[bjope]

I've opened a pull-request to also adjust the codegen here, to make sure that we use an unsigned cast between the LLVM intrinsic result type and the builtins return type. See #90845