[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

2018-08-20 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits 
pirama abandoned this revision.
pirama added a comment.

Thanks for the clarification Richard and Eli.  I agree that leaving the status 
quo will match the intent of the macro.  I'll abandon this.


Repository:
  rC Clang

https://reviews.llvm.org/D50683



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

2018-08-15 Thread Eli Friedman via Phabricator via cfe-commits 
efriedma added a comment.

The C++ standard just says "An integer literal of type std::size_t whose value 
is the alignment guaranteed by a call to operator new(std::size_t) or operator 
new[](std::size_t)."  I would take that in the obvious sense, that any pointer 
returned by operator new must have alignment `__STDCPP_DEFAULT_NEW_ALIGNMENT__`.

Granted, I can see how that might not be the intent, given that `alignof(T) >= 
sizeof(T)` for all C++ types.


Repository:
  rC Clang

https://reviews.llvm.org/D50683



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

2018-08-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

This doesn't seem necessary. `NewAlign` specifies the alignment beyond which 
types acquire "new-extended alignment" per the C++ standard, or equivalently 
the alignment beyond which we need to pass an `align_val_t` argument to 
`operator new`.

If all types of size <= 8 are provided with sufficiently-aligned storage (which 
8 byte alignment definitely is), then they are irrelevant for the computation 
of this value, because `new T` for such a type never needs to pass an 
alignment. (A similar argument applies for the array-new case.)


Repository:
  rC Clang

https://reviews.llvm.org/D50683



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

2018-08-14 Thread Eli Friedman via Phabricator via cfe-commits 
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Basic/TargetInfo.cpp:72
+// For 64-bit Android, alignment is 8 bytes for allocations <= 8 bytes.
+NewAlign = (Triple.isArch64Bit() || Triple.isArch32Bit()) ? 64 : 0;
+  } else

Might as well just set `NewAlign = 64;` here.  But not a big deal either way.


Repository:
  rC Clang

https://reviews.llvm.org/D50683



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

2018-08-13 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits 
pirama created this revision.
pirama added reviewers: rsmith, srhines.

Android uses jemalloc allocator, which returns 8-byte-aligned pointers for
allocations smaller than 8 bytes for 64-bit architectures.  Set NewAlign
conservatively to 8 bytes.


Repository:
  rC Clang

https://reviews.llvm.org/D50683

Files:
  lib/Basic/TargetInfo.cpp
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9022,7 +9022,10 @@
 // I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U
 //
 // RUN: %clang_cc1 -x c++ -triple x86_64-linux-android -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix X86_64-ANDROID-CXX %s
-// X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8UL
+//
+// RUN: %clang_cc1 -x c++ -triple aarch64-linux-android -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix AARCH64-ANDROID-CXX %s
+// AARCH64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8UL
 //
 // RUN: %clang_cc1 -triple arm-linux-androideabi20 -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix ANDROID20 %s
 // ANDROID20:#define __ANDROID_API__ 20
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -64,10 +64,13 @@
   // From the glibc documentation, on GNU systems, malloc guarantees 16-byte
   // alignment on 64-bit systems and 8-byte alignment on 32-bit systems. See
   // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html.
-  // This alignment guarantee also applies to Windows and Android.
-  if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment() || T.isAndroid())
+  // This alignment guarantee also applies to Windows.
+  if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment())
 NewAlign = Triple.isArch64Bit() ? 128 : Triple.isArch32Bit() ? 64 : 0;
-  else
+  else if (T.isAndroid()) {
+// For 64-bit Android, alignment is 8 bytes for allocations <= 8 bytes.
+NewAlign = (Triple.isArch64Bit() || Triple.isArch32Bit()) ? 64 : 0;
+  } else
 NewAlign = 0; // Infer from basic type alignment.
   HalfWidth = 16;
   HalfAlign = 16;


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9022,7 +9022,10 @@
 // I386-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U
 //
 // RUN: %clang_cc1 -x c++ -triple x86_64-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix X86_64-ANDROID-CXX %s
-// X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// X86_64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8UL
+//
+// RUN: %clang_cc1 -x c++ -triple aarch64-linux-android -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix AARCH64-ANDROID-CXX %s
+// AARCH64-ANDROID-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8UL
 //
 // RUN: %clang_cc1 -triple arm-linux-androideabi20 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix ANDROID20 %s
 // ANDROID20:#define __ANDROID_API__ 20
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -64,10 +64,13 @@
   // From the glibc documentation, on GNU systems, malloc guarantees 16-byte
   // alignment on 64-bit systems and 8-byte alignment on 32-bit systems. See
   // https://www.gnu.org/software/libc/manual/html_node/Malloc-Examples.html.
-  // This alignment guarantee also applies to Windows and Android.
-  if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment() || T.isAndroid())
+  // This alignment guarantee also applies to Windows.
+  if (T.isGNUEnvironment() || T.isWindowsMSVCEnvironment())
 NewAlign = Triple.isArch64Bit() ? 128 : Triple.isArch32Bit() ? 64 : 0;
-  else
+  else if (T.isAndroid()) {
+// For 64-bit Android, alignment is 8 bytes for allocations <= 8 bytes.
+NewAlign = (Triple.isArch64Bit() || Triple.isArch32Bit()) ? 64 : 0;
+  } else
 NewAlign = 0; // Infer from basic type alignment.
   HalfWidth = 16;
   HalfAlign = 16;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits