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] Add bitint classification for __builtin_classify_type #72036

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

tbaederr
Copy link
Contributor

See #71911

@llvmbot llvmbot added cmake Build system in general and CMake in particular clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 11, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 11, 2023

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

See #71911


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

4 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+7-1)
  • (modified) clang/test/Sema/builtin-classify-type.c (+4-1)
  • (modified) clang/test/SemaCXX/builtin-classify-type.cpp (+4-1)
  • (modified) llvm/cmake/modules/HandleLLVMOptions.cmake (+1-1)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e16fec6109e744e..2073679adf790fb 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -11486,6 +11486,7 @@ bool IntExprEvaluator::CheckReferencedDecl(const Expr* E, const Decl* D) {
 
 /// Values returned by __builtin_classify_type, chosen to match the values
 /// produced by GCC's builtin.
+/// The values can be found in gcc/typeclass.h in the GCC repository.
 enum class GCCTypeClass {
   None = -1,
   Void = 0,
@@ -11512,6 +11513,9 @@ enum class GCCTypeClass {
   // decay to pointer. (Prior to version 6 it was only used in C++ mode).
   // GCC reserves 15 for strings, but actually uses 5 (pointer) for string
   // literals.
+  // Lang = 16,
+  // OpaqueType = 17,
+  BitInt = 18
 };
 
 /// EvaluateBuiltinClassifyType - Evaluate __builtin_classify_type the same way
@@ -11644,11 +11648,13 @@ EvaluateBuiltinClassifyType(QualType T, const LangOptions &LangOpts) {
   case Type::ObjCInterface:
   case Type::ObjCObjectPointer:
   case Type::Pipe:
-  case Type::BitInt:
     // GCC classifies vectors as None. We follow its lead and classify all
     // other types that don't fit into the regular classification the same way.
     return GCCTypeClass::None;
 
+  case Type::BitInt:
+    return GCCTypeClass::BitInt;
+
   case Type::LValueReference:
   case Type::RValueReference:
     llvm_unreachable("invalid type for expression");
diff --git a/clang/test/Sema/builtin-classify-type.c b/clang/test/Sema/builtin-classify-type.c
index a222ac8af0e32fd..9a4de34e823f231 100644
--- a/clang/test/Sema/builtin-classify-type.c
+++ b/clang/test/Sema/builtin-classify-type.c
@@ -11,7 +11,8 @@ enum gcc_type_class {
   function_type_class, method_type_class,
   record_type_class, union_type_class,
   array_type_class, string_type_class,
-  lang_type_class
+  lang_type_class, opaque_type_class,
+  bitint_type_class
 };
 
 void foo(void) {
@@ -45,6 +46,7 @@ void foo(void) {
   vint32_t3 vt5;
   typedef _BitInt(64) vint64_t3 __attribute__((vector_size(16)));
   vint64_t3 vt6;
+  _BitInt(16) bitint;
 
   _Atomic int atomic_i;
   _Atomic double atomic_d;
@@ -70,6 +72,7 @@ void foo(void) {
   int a17[__builtin_classify_type(atomic_d) == real_type_class ? 1 : -1];
   int a18[__builtin_classify_type(complex_i) == complex_type_class ? 1 : -1];
   int a19[__builtin_classify_type(complex_d) == complex_type_class ? 1 : -1];
+  int a20[__builtin_classify_type(bitint) == bitint_type_class ? 1 : -1];
 }
 
 extern int (^p)(void);
diff --git a/clang/test/SemaCXX/builtin-classify-type.cpp b/clang/test/SemaCXX/builtin-classify-type.cpp
index ebc81425e401f11..ed5430960001002 100644
--- a/clang/test/SemaCXX/builtin-classify-type.cpp
+++ b/clang/test/SemaCXX/builtin-classify-type.cpp
@@ -11,7 +11,8 @@ enum gcc_type_class {
   function_type_class, method_type_class,
   record_type_class, union_type_class,
   array_type_class, string_type_class,
-  lang_type_class
+  lang_type_class, opaque_type_class,
+  bitint_type_class
 };
 
 class cl {
@@ -42,6 +43,7 @@ void foo() {
   _Atomic double atomic_d;
   _Complex int complex_i;
   _Complex double complex_d;
+  _BitInt(32) bitint;
 
   int a1[__builtin_classify_type(f()) == void_type_class ? 1 : -1];
   int a2[__builtin_classify_type(i) == integer_type_class ? 1 : -1];
@@ -65,5 +67,6 @@ void foo() {
   int a20[__builtin_classify_type(atomic_d) == real_type_class ? 1 : -1];
   int a21[__builtin_classify_type(complex_i) == complex_type_class ? 1 : -1];
   int a22[__builtin_classify_type(complex_d) == complex_type_class ? 1 : -1];
+  int a23[__builtin_classify_type(bitint) == bitint_type_class ? 1 : -1];
 }
 
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 6a3c49edc912ded..087d96550143ee7 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -1060,7 +1060,7 @@ if (UNIX AND
     (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
      (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND
       NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.9))))
-  append("-fdiagnostics-color" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+  append("-fdiagnostics-color=always" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 endif()
 
 # lld doesn't print colored diagnostics when invoked from Ninja

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

Can you add a release note? otherwise this looks good

@tbaederr tbaederr force-pushed the classify-bitint branch 2 times, most recently from f3cd338 to c721070 Compare November 12, 2023 03:48
@tbaederr tbaederr removed the cmake Build system in general and CMake in particular label Nov 12, 2023
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM

clang/lib/AST/ExprConstant.cpp Outdated Show resolved Hide resolved
Comment on lines +220 to +223
* ``__builtin_classify_type()`` now classifies ``_BitInt`` values as the return value ``18``,
to match GCC 14's behavior.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should mention that this fixes #71911

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 didn't add that because the issue also mentions some extensions to __builtin_classify_type. We should probably split that out into a separate issue where we also document the exact extensions, maybe with a link to the gcc commit and test cases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, that's reasonable.

Comment on lines +220 to +223
* ``__builtin_classify_type()`` now classifies ``_BitInt`` values as the return value ``18``,
to match GCC 14's behavior.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, that's reasonable.

@tbaederr tbaederr merged commit ea31662 into llvm:main Nov 17, 2023
4 checks passed
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
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.

None yet

4 participants