[clang] [clang] Fix __builtin_popcountg not matching GCC (PR #83313)

via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 28 10:46:10 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: OverMighty (overmighty)

<details>
<summary>Changes</summary>

Our implementation previously accepted signed arguments and performed
integer promotion on the argument. GCC's implementation requires an
unsigned argument and does not perform integer promotion on it.

See https://github.com/llvm/llvm-project/pull/82359#issuecomment-1965513595.

cc @<!-- -->efriedma-quic


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


6 Files Affected:

- (modified) clang/docs/LanguageExtensions.rst (+4-4) 
- (modified) clang/include/clang/Basic/Builtins.td (+1-1) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+9-3) 
- (modified) clang/test/CodeGen/builtins.c (+14-14) 
- (modified) clang/test/Sema/builtin-popcountg.c (+13-4) 


``````````diff
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 2a177814c4df7a..bcd69198eafdbe 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3477,7 +3477,7 @@ builtin, the mangler emits their usual pattern without any special treatment.
 -----------------------
 
 ``__builtin_popcountg`` returns the number of 1 bits in the argument. The
-argument can be of any integer type.
+argument can be of any unsigned integer type.
 
 **Syntax**:
 
@@ -3489,20 +3489,20 @@ argument can be of any integer type.
 
 .. code-block:: c++
 
-  int x = 1;
+  unsigned int x = 1;
   int x_pop = __builtin_popcountg(x);
 
   unsigned long y = 3;
   int y_pop = __builtin_popcountg(y);
 
-  _BitInt(128) z = 7;
+  unsigned _BitInt(128) z = 7;
   int z_pop = __builtin_popcountg(z);
 
 **Description**:
 
 ``__builtin_popcountg`` is meant to be a type-generic alternative to the
 ``__builtin_popcount{,l,ll}`` builtins, with support for other integer types,
-such as ``__int128`` and C23 ``_BitInt(N)``.
+such as ``unsigned __int128`` and C23 ``unsigned _BitInt(N)``.
 
 Multiprecision Arithmetic Builtins
 ----------------------------------
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 3bc35c5bb38ecf..2fbc56d49a59a1 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -690,7 +690,7 @@ def Popcount : Builtin, BitInt_Long_LongLongTemplate {
 
 def Popcountg : Builtin {
   let Spellings = ["__builtin_popcountg"];
-  let Attributes = [NoThrow, Const];
+  let Attributes = [NoThrow, Const, CustomTypeChecking];
   let Prototype = "int(...)";
 }
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c8141fefb8edba..f726805dc02bd9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11984,7 +11984,7 @@ def err_builtin_invalid_arg_type: Error <
   "signed integer or floating point type|vector type|"
   "floating point type|"
   "vector of integers|"
-  "type of integer}1 (was %2)">;
+  "type of unsigned integer}1 (was %2)">;
 
 def err_builtin_matrix_disabled: Error<
   "matrix types extension is disabled. Pass -fenable-matrix to enable it">;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0de76ee119cf81..3156b3f6b892e1 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2195,12 +2195,18 @@ static bool SemaBuiltinPopcountg(Sema &S, CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 1))
     return true;
 
-  Expr *Arg = TheCall->getArg(0);
+  ExprResult ArgRes = S.DefaultLvalueConversion(TheCall->getArg(0));
+  if (ArgRes.isInvalid())
+    return true;
+
+  Expr *Arg = ArgRes.get();
+  TheCall->setArg(0, Arg);
+
   QualType ArgTy = Arg->getType();
 
-  if (!ArgTy->isIntegerType()) {
+  if (!ArgTy->isUnsignedIntegerType()) {
     S.Diag(Arg->getBeginLoc(), diag::err_builtin_invalid_arg_type)
-        << 1 << /*integer ty*/ 7 << ArgTy;
+        << 1 << /*unsigned integer ty*/ 7 << ArgTy;
     return true;
   }
   return false;
diff --git a/clang/test/CodeGen/builtins.c b/clang/test/CodeGen/builtins.c
index 73866116e07e72..4f9641d357b7ba 100644
--- a/clang/test/CodeGen/builtins.c
+++ b/clang/test/CodeGen/builtins.c
@@ -948,14 +948,14 @@ void test_builtin_popcountg(unsigned char uc, unsigned short us,
   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
+  // CHECK-NEXT: %2 = call i8 @llvm.ctpop.i8(i8 %1)
+  // CHECK-NEXT: %cast = sext i8 %2 to i32
+  // CHECK-NEXT: store volatile i32 %cast, 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
+  // CHECK-NEXT: %4 = call i16 @llvm.ctpop.i16(i16 %3)
+  // CHECK-NEXT: %cast1 = sext i16 %4 to i32
+  // CHECK-NEXT: store volatile i32 %cast1, 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)
@@ -963,23 +963,23 @@ void test_builtin_popcountg(unsigned char uc, unsigned short us,
   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
+  // CHECK-NEXT: %cast2 = trunc i64 %8 to i32
+  // CHECK-NEXT: store volatile i32 %cast2, 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
+  // CHECK-NEXT: %cast3 = trunc i64 %10 to i32
+  // CHECK-NEXT: store volatile i32 %cast3, 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
+  // CHECK-NEXT: %cast4 = trunc i128 %12 to i32
+  // CHECK-NEXT: store volatile i32 %cast4, 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: %cast5 = trunc i128 %14 to i32
+  // CHECK-NEXT: store volatile i32 %cast5, ptr %pop, align 4
   // CHECK-NEXT: ret void
 }
 
diff --git a/clang/test/Sema/builtin-popcountg.c b/clang/test/Sema/builtin-popcountg.c
index e18b910046ff0c..9d095927d24e1a 100644
--- a/clang/test/Sema/builtin-popcountg.c
+++ b/clang/test/Sema/builtin-popcountg.c
@@ -1,14 +1,23 @@
-// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -verify -Wpedantic %s
+// RUN: %clang_cc1 -std=c23 -triple=x86_64-pc-linux-gnu -fsyntax-only -verify -Wpedantic %s
 
 typedef int int2 __attribute__((ext_vector_type(2)));
 
-void test_builtin_popcountg(int i, double d, int2 i2) {
+void test_builtin_popcountg(short s, int i, __int128 i128, _BitInt(128) bi128,
+                            double d, int2 i2) {
   __builtin_popcountg();
   // expected-error at -1 {{too few arguments to function call, expected 1, have 0}}
   __builtin_popcountg(i, i);
   // expected-error at -1 {{too many arguments to function call, expected 1, have 2}}
+  __builtin_popcountg(s);
+  // expected-error at -1 {{1st argument must be a type of unsigned integer (was 'short')}}
+  __builtin_popcountg(i);
+  // expected-error at -1 {{1st argument must be a type of unsigned integer (was 'int')}}
+  __builtin_popcountg(i128);
+  // expected-error at -1 {{1st argument must be a type of unsigned integer (was '__int128')}}
+  __builtin_popcountg(bi128);
+  // expected-error at -1 {{1st argument must be a type of unsigned integer (was '_BitInt(128)')}}
   __builtin_popcountg(d);
-  // expected-error at -1 {{1st argument must be a type of integer (was 'double')}}
+  // expected-error at -1 {{1st argument must be a type of unsigned integer (was 'double')}}
   __builtin_popcountg(i2);
-  // expected-error at -1 {{1st argument must be a type of integer (was 'int2' (vector of 2 'int' values))}}
+  // expected-error at -1 {{1st argument must be a type of unsigned integer (was 'int2' (vector of 2 'int' values))}}
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/83313


More information about the cfe-commits mailing list