[builtin] Fixed definitions of builtins that rely on the int/long long type is 32/64 bits

[Ka-Ka]

Summary

The definition of the builtins builtin_bswap32, builtin_bitreverse32, builtin_rotateleft32 and builtin_rotateright32 rely on that the int type is 32 bits wide on the target.
The defintions of the builtins builtin_bswap64, builtin_bitreverse64, builtin_rotateleft64, and builtin_rotateright64 rely on that the long long type is 64 bits wide.

On targets where this is not the case (e.g. AVR) clang will generate faulty code (wrong llvm assembler intrinsics).

This patch add support for using 'Z' (the int32_t type) in Bultins.def. The builtins above are changed to be based on the int32_t type instead of the int type, and the int64_t type instead of the long long type.

The AVR backend (experimental) have a native int type that is only 16 bits wide. The supplied testcase will therefore fail if running the testcase on trunk as clang will convert e.g. __builtin_bitreverse32 into llvm.bitreverse.i16 on AVR.

[efriedma]

Adding "Z" makes sense.

If you're going to mess with the 64-bit builtins, it's probably worth adding a testcase that can be built with gcc to show that int64_t is actually correct. You should be able to write a C++ testcase using decltype (declare a variable of type "int64_t", then redeclare it as type "decltype(__builtin_bswap64(0))").

test/CodeGen/avr-builtins.c

1 ↗

(On Diff #199207)

I don't think this needs avr-registered-target; it doesn't actually invoke the backend or any optimization passes that depend on it.

[Ka-Ka]

Added a new C++ testcase.
Removed the REQUIRES: avr-registered-target in the avr-builtins.c testcase.

[efriedma]

test/CodeGen/builtins.cpp

5 ↗

(On Diff #199385)

You don't need quite so many targets on this list. There are essentially three interesting kinds of targets: targets where int is 16 bits (like avr), targets where int is 32 bits and long is 64 bits (like x86_64 Linux), and targets where int and long are both 32 bits (like i686 Linux).

You might need to pass -ffreestanding to avoid including /usr/include/stdint.h on non-Linux systems.

21 ↗

(On Diff #199385)

If you write "extern uint16_t bswap16;", there won't be any error message when the types match.

[Ka-Ka]

Updated according to review comments

[efriedma]

LGTM

cfe/trunk/include/clang/Basic/Builtins.def

 //  K -> ucontext_t
 //  p -> pid_t
 //  . -> "...".  This may only occur at the end of the function list.
 //
 // Types may be prefixed with the following modifiers:
 //  L   -> long (e.g. Li for 'long int', Ld for 'long double')
 //  LL  -> long long (e.g. LLi for 'long long int', LLd for __float128)
 //  LLL -> __int128_t (e.g. LLLi)
-//  W   -> int64_t
+//  Z   -> int32_t (require a native 32-bit integer type on the target)
+//  W   -> int64_t (require a native 64-bit integer type on the target)
 //  N   -> 'int' size if target is LP64, 'L' otherwise.
 //  S   -> signed
 //  U   -> unsigned
 //  I   -> Required to constant fold to an integer constant expression.
 //
 // Types may be postfixed with the following modifiers:
 // * -> pointer (optionally followed by an address space number, if no address
 //               space is specified than any address space will be accepted)

 BUILTIN(__builtin_parityll, "iULLi", "nc")
 BUILTIN(__builtin_popcount  , "iUi"  , "nc")
 BUILTIN(__builtin_popcountl , "iULi" , "nc")
 BUILTIN(__builtin_popcountll, "iULLi", "nc")
 BUILTIN(__builtin_clrsb  , "ii"  , "nc")
 BUILTIN(__builtin_clrsbl , "iLi" , "nc")
 BUILTIN(__builtin_clrsbll, "iLLi", "nc")
 
-// FIXME: These type signatures are not correct for targets with int != 32-bits
-// or with ULL != 64-bits.
+// The following builtins rely on that char == 8 bits, short == 16 bits and that
+// there exists native types on the target that are 32- and 64-bits wide, unless
+// these conditions are fulfilled these builtins will operate on a not intended
+// bitwidth.
 BUILTIN(__builtin_bswap16, "UsUs", "nc")
-BUILTIN(__builtin_bswap32, "UiUi", "nc")
-BUILTIN(__builtin_bswap64, "ULLiULLi", "nc")
+BUILTIN(__builtin_bswap32, "UZiUZi", "nc")
+BUILTIN(__builtin_bswap64, "UWiUWi", "nc")
 
 BUILTIN(__builtin_bitreverse8, "UcUc", "nc")
 BUILTIN(__builtin_bitreverse16, "UsUs", "nc")
-BUILTIN(__builtin_bitreverse32, "UiUi", "nc")
-BUILTIN(__builtin_bitreverse64, "ULLiULLi", "nc")
+BUILTIN(__builtin_bitreverse32, "UZiUZi", "nc")
+BUILTIN(__builtin_bitreverse64, "UWiUWi", "nc")
 
 BUILTIN(__builtin_rotateleft8, "UcUcUc", "nc")
 BUILTIN(__builtin_rotateleft16, "UsUsUs", "nc")
-BUILTIN(__builtin_rotateleft32, "UiUiUi", "nc")
-BUILTIN(__builtin_rotateleft64, "ULLiULLiULLi", "nc")
+BUILTIN(__builtin_rotateleft32, "UZiUZiUZi", "nc")
+BUILTIN(__builtin_rotateleft64, "UWiUWiUWi", "nc")
 BUILTIN(__builtin_rotateright8, "UcUcUc", "nc")
 BUILTIN(__builtin_rotateright16, "UsUsUs", "nc")
-BUILTIN(__builtin_rotateright32, "UiUiUi", "nc")
-BUILTIN(__builtin_rotateright64, "ULLiULLiULLi", "nc")
+BUILTIN(__builtin_rotateright32, "UZiUZiUZi", "nc")
+BUILTIN(__builtin_rotateright64, "UWiUWiWi", "nc")
 
 // Random GCC builtins
 BUILTIN(__builtin_constant_p, "i.", "nctu")
 BUILTIN(__builtin_classify_type, "i.", "nctu")
 BUILTIN(__builtin___CFStringMakeConstantString, "FC*cC*", "nc")
 BUILTIN(__builtin___NSStringMakeConstantString, "FC*cC*", "nc")
 BUILTIN(__builtin_va_start, "vA.", "nt")
 BUILTIN(__builtin_va_end, "vA", "n")

cfe/trunk/lib/AST/ASTContext.cpp

static QualType DecodeTypeFromStr(const char *&Str, const ASTContext &Context,
   // Modifiers.
   int HowLong = 0;
   bool Signed = false, Unsigned = false;
   RequiresICE = false;
 
   // Read the prefixed modifiers first.
   bool Done = false;
   #ifndef NDEBUG
-  bool IsSpecialLong = false;
+  bool IsSpecial = false;
   #endif
   while (!Done) {
     switch (*Str++) {
     default: Done = true; --Str; break;
     case 'I':
       RequiresICE = true;
       break;
     case 'S':
       assert(!Unsigned && "Can't use both 'S' and 'U' modifiers!");
       assert(!Signed && "Can't use 'S' modifier multiple times!");
       Signed = true;
       break;
     case 'U':
       assert(!Signed && "Can't use both 'S' and 'U' modifiers!");
       assert(!Unsigned && "Can't use 'U' modifier multiple times!");
       Unsigned = true;
       break;
     case 'L':
-      assert(!IsSpecialLong && "Can't use 'L' with 'W' or 'N' modifiers");
+      assert(!IsSpecial && "Can't use 'L' with 'W', 'N' or 'Z' modifiers");
       assert(HowLong <= 2 && "Can't have LLLL modifier");
       ++HowLong;
       break;
     case 'N':
       // 'N' behaves like 'L' for all non LP64 targets and 'int' otherwise.
-      assert(!IsSpecialLong && "Can't use two 'N' or 'W' modifiers!");
+      assert(!IsSpecial && "Can't use two 'N', 'W' or 'Z' modifiers!");
       assert(HowLong == 0 && "Can't use both 'L' and 'N' modifiers!");
       #ifndef NDEBUG
-      IsSpecialLong = true;
+      IsSpecial = true;
       #endif
       if (Context.getTargetInfo().getLongWidth() == 32)
         ++HowLong;
       break;
     case 'W':
       // This modifier represents int64 type.
-      assert(!IsSpecialLong && "Can't use two 'N' or 'W' modifiers!");
+      assert(!IsSpecial && "Can't use two 'N', 'W' or 'Z'  modifiers!");
       assert(HowLong == 0 && "Can't use both 'L' and 'W' modifiers!");
       #ifndef NDEBUG
-      IsSpecialLong = true;
+      IsSpecial = true;
       #endif
       switch (Context.getTargetInfo().getInt64Type()) {
       default:
         llvm_unreachable("Unexpected integer type");
       case TargetInfo::SignedLong:
         HowLong = 1;
         break;
       case TargetInfo::SignedLongLong:
         HowLong = 2;
         break;
       }
       break;
+    case 'Z':
+      // This modifier represents int32 type.
+      assert(!IsSpecial && "Can't use two 'N', 'W' or 'Z' modifiers!");
+      assert(HowLong == 0 && "Can't use both 'L' and 'Z' modifiers!");
+      #ifndef NDEBUG
+      IsSpecial = true;
+      #endif
+      switch (Context.getTargetInfo().getIntTypeByWidth(32, true)) {
+      default:
+        llvm_unreachable("Unexpected integer type");
+      case TargetInfo::SignedInt:
+        HowLong = 0;
+        break;
+      case TargetInfo::SignedLong:
+        HowLong = 1;
+        break;
+      case TargetInfo::SignedLongLong:
+        HowLong = 2;
+        break;
+      }
+      break;
     }
   }
 
   QualType Type;
 
   // Read the base type.
   switch (*Str++) {
   default: llvm_unreachable("Unknown builtin type letter!");

cfe/trunk/test/CodeGen/avr-builtins.c

+// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+unsigned char bitrev8(unsigned char data) {
+    return __builtin_bitreverse8(data);
+}
+
+// CHECK: define zeroext i8 @bitrev8
+// CHECK: i8 @llvm.bitreverse.i8(i8
+
+unsigned int bitrev16(unsigned int data) {
+    return __builtin_bitreverse16(data);
+}
+
+// CHECK: define i16 @bitrev16
+// CHECK: i16 @llvm.bitreverse.i16(i16
+
+unsigned long bitrev32(unsigned long data) {
+    return __builtin_bitreverse32(data);
+}
+// CHECK: define i32 @bitrev32
+// CHECK: i32 @llvm.bitreverse.i32(i32
+
+unsigned long long bitrev64(unsigned long long data) {
+    return __builtin_bitreverse64(data);
+}
+
+// CHECK: define i64 @bitrev64
+// CHECK: i64 @llvm.bitreverse.i64(i64
+
+unsigned char rotleft8(unsigned char x, unsigned char y) {
+    return __builtin_rotateleft8(x, y);
+}
+
+// CHECK: define zeroext i8 @rotleft8
+// CHECK: i8 @llvm.fshl.i8(i8
+
+unsigned int rotleft16(unsigned int x, unsigned int y) {
+    return __builtin_rotateleft16(x, y);
+}
+
+// CHECK: define i16 @rotleft16
+// CHECK: i16 @llvm.fshl.i16(i16
+
+unsigned long rotleft32(unsigned long x, unsigned long y) {
+    return __builtin_rotateleft32(x, y);
+}
+// CHECK: define i32 @rotleft32
+// CHECK: i32 @llvm.fshl.i32(i32
+
+unsigned long long rotleft64(unsigned long long x, unsigned long long y) {
+    return __builtin_rotateleft64(x, y);
+}
+
+// CHECK: define i64 @rotleft64
+// CHECK: i64 @llvm.fshl.i64(i64
+
+unsigned char rotright8(unsigned char x, unsigned char y) {
+    return __builtin_rotateright8(x, y);
+}
+
+// CHECK: define zeroext i8 @rotright8
+// CHECK: i8 @llvm.fshr.i8(i8
+
+unsigned int rotright16(unsigned int x, unsigned int y) {
+    return __builtin_rotateright16(x, y);
+}
+
+// CHECK: define i16 @rotright16
+// CHECK: i16 @llvm.fshr.i16(i16
+
+unsigned long rotright32(unsigned long x, unsigned long y) {
+    return __builtin_rotateright32(x, y);
+}
+// CHECK: define i32 @rotright32
+// CHECK: i32 @llvm.fshr.i32(i32
+
+unsigned long long rotright64(unsigned long long x, unsigned long long y) {
+    return __builtin_rotateright64(x, y);
+}
+
+// CHECK: define i64 @rotright64
+// CHECK: i64 @llvm.fshr.i64(i64
+
+unsigned int byteswap16(unsigned int x) {
+    return __builtin_bswap16(x);
+}
+
+// CHECK: define i16 @byteswap16
+// CHECK: i16 @llvm.bswap.i16(i16
+
+unsigned long byteswap32(unsigned long x) {
+    return __builtin_bswap32(x);
+}
+// CHECK: define i32 @byteswap32
+// CHECK: i32 @llvm.bswap.i32(i32
+
+unsigned long long byteswap64(unsigned long long x) {
+    return __builtin_bswap64(x);
+}
+
+// CHECK: define i64 @byteswap64
+// CHECK: i64 @llvm.bswap.i64(i64

cfe/trunk/test/CodeGen/builtins.cpp

+// RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -ffreestanding -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple i686-pc-linux-gnu -ffreestanding -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple avr-unknown-unknown -ffreestanding -verify %s
+
+// expected-no-diagnostics
+
+// Test that checks that the builtins return the same type as the stdint.h type
+// withe the same witdh. This is done by first declaring a variable of a stdint
+// type of the correct width and the redeclaring the variable with the type that
+// the builting return. If the types are different you should the an error from
+// clang of the form:
+// "redefinition of '<varname>' with a different type: '<type1>' vs '<type2>'
+// (with gcc you get an error message
+// "conflicting declaration '<type> <varname>'").
+
+#include <stdint.h>
+
+extern uint16_t bswap16;
+decltype(__builtin_bswap16(0)) bswap16 = 42;
+extern uint32_t bswap32;
+decltype(__builtin_bswap32(0)) bswap32 = 42;
+extern uint64_t bswap64;
+decltype(__builtin_bswap64(0)) bswap64 = 42;
+
+#ifdef __clang__
+extern uint8_t bitrev8;
+decltype(__builtin_bitreverse8(0)) bitrev8 = 42;
+extern uint16_t bitrev16;
+decltype(__builtin_bitreverse16(0)) bitrev16 = 42;
+extern uint32_t bitrev32;
+decltype(__builtin_bitreverse32(0)) bitrev32 = 42;
+extern uint64_t bitrev64;
+decltype(__builtin_bitreverse64(0)) bitrev64 = 42;
+
+extern uint8_t rotl8;
+decltype(__builtin_rotateleft8(0,0)) rotl8 = 42;
+extern uint16_t rotl16;
+decltype(__builtin_rotateleft16(0,0)) rotl16 = 42;
+extern uint32_t rotl32;
+decltype(__builtin_rotateleft32(0,0)) rotl32 = 42;
+extern uint64_t rotl64;
+decltype(__builtin_rotateleft64(0,0)) rotl64 = 42;
+
+extern uint8_t rotr8;
+decltype(__builtin_rotateright8(0,0)) rotr8 = 42;
+extern uint16_t rotr16;
+decltype(__builtin_rotateright16(0,0)) rotr16 = 42;
+extern uint32_t rotr32;
+decltype(__builtin_rotateright32(0,0)) rotr32 = 42;
+extern uint64_t rotr64;
+decltype(__builtin_rotateright64(0,0)) rotr64 = 42;
+#endif