[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D45015#1188141, @EricWF wrote:

> Hi, I'm in the process of moving cities. please take over.
>
> Thanks, and sorry


No worries. Thanks for spending time on this issue and making the fix, 
committing is the easy part. Good luck with the move.


Repository:
  rL LLVM

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL338934: [Preprocessor] Allow libc++ to detect when aligned 
allocation is unavailable. (authored by vsapsai, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45015?vs=150635=159121#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45015

Files:
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/lib/Frontend/InitPreprocessor.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/test/Driver/unavailable_aligned_allocation.cpp
  cfe/trunk/test/Lexer/aligned-allocation.cpp

Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td
@@ -364,7 +364,6 @@
 def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">;
 def : DiagGroup<"effc++", [NonVirtualDtor]>;
 def OveralignedType : DiagGroup<"over-aligned">;
-def AlignedAllocationUnavailable : DiagGroup<"aligned-allocation-unavailable">;
 def OldStyleCast : DiagGroup<"old-style-cast">;
 def : DiagGroup<"old-style-definition">;
 def OutOfLineDeclaration : DiagGroup<"out-of-line-declaration">;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6465,12 +6465,12 @@
   "type %0 requires %1 bytes of alignment and the default allocator only "
   "guarantees %2 bytes">,
   InGroup, DefaultIgnore;
-def warn_aligned_allocation_unavailable :Warning<
+def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
-  "available on %2 %3 or newer">, InGroup, DefaultError;
+  "available on %2 %3 or newer">;
 def note_silence_unligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
-  "-Wno-aligned-allocation-unavailable to silence this diagnostic">;
+  "-faligned-allocation to silence this diagnostic">;
 
 def err_conditional_void_nonvoid : Error<
   "%select{left|right}1 operand to ? is void, but %select{right|left}1 operand "
Index: cfe/trunk/test/Lexer/aligned-allocation.cpp
===
--- cfe/trunk/test/Lexer/aligned-allocation.cpp
+++ cfe/trunk/test/Lexer/aligned-allocation.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -DEXPECT_DEFINED
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-alloc-unavailable
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-allocation -faligned-alloc-unavailable
+
+// Test that __cpp_aligned_new is not defined when CC1 is passed
+// -faligned-alloc-unavailable by the Darwin driver, even when aligned
+// allocation is actually enabled.
+
+// expected-no-diagnostics
+#ifdef EXPECT_DEFINED
+# ifndef __cpp_aligned_new
+#   error "__cpp_aligned_new" should be defined
+# endif
+#else
+# ifdef __cpp_aligned_new
+#   error "__cpp_aligned_new" should not be defined
+# endif
+#endif
Index: cfe/trunk/test/Driver/unavailable_aligned_allocation.cpp
===
--- cfe/trunk/test/Driver/unavailable_aligned_allocation.cpp
+++ cfe/trunk/test/Driver/unavailable_aligned_allocation.cpp
@@ -51,4 +51,13 @@
 // RUN: -c -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=AVAILABLE
 //
+// Check that passing -faligned-allocation or -fno-aligned-allocation stops the
+// driver from passing -faligned-alloc-unavailable to cc1.
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -faligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -fno-aligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+
 // AVAILABLE-NOT: "-faligned-alloc-unavailable"
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2035,7 +2035,11 @@
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ,
Action::OffloadKind DeviceOffloadKind) const {
-  if (isAlignedAllocationUnavailable())
+  // Pass "-faligned-alloc-unavailable" only when the user hasn't manually
+  // enabled or disabled aligned allocations.
+  if 

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

Hi, I'm in the process of moving cities. please take over.

Thanks, and sorry


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Eric, will you have time to commit this patch? If you don't have time and don't 
have objections, I plan to land this change on your behalf.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/ToolChains/Darwin.cpp:2027
+  isAlignedAllocationUnavailable())
 CC1Args.push_back("-faligned-alloc-unavailable");
 }

Quuxplusone wrote:
> Peanut gallery asks: Why is the cc1 option spelled differently from the clang 
> driver option? Don't they do the same thing?
At the `-cc1` level, there are three different levels:
 * no aligned allocation in the language
 * aligned allocation in the language but it doesn't work (eg, C++17 on old 
Darwin)
 * aligned allocation in the language and it works

The driver aligned allocation flags force the compilation into case 1 or 3. 
Cases 1 and 2 do not advertise `__cpp_aligned_new`. Case 2 provides an error if 
an aligned allocation is attempted, whereas case 1 provides an underaligned 
allocation with a warning. This flag puts us into case 2, so it isn't the same 
as any driver flag.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6425
+  "available on %2 %3 or newer">;
 def note_silence_unligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "

I observe that this mnemonic is misspelled; but that's not this patch's fault.
The traditional spelling would probably be just 
`note_aligned_allocation_unavailable`.



Comment at: lib/Driver/ToolChains/Darwin.cpp:2027
+  isAlignedAllocationUnavailable())
 CC1Args.push_back("-faligned-alloc-unavailable");
 }

Peanut gallery asks: Why is the cc1 option spelled differently from the clang 
driver option? Don't they do the same thing?


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a reviewer: dexonsmith.
EricWF added a comment.

Ping.

Are there any more reviewers I should add to this?


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

@rsmith Ping. This needs to land before the next branch for release.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

With this change and the mentioned libc++ change the tests with old libc++ 
dylib are passing (didn't test all possible configurations though). Would like 
to get more feedback from other reviewers on this matter.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 150635.
EricWF added a comment.

Remove `-nostdinc++` check as requested.


https://reviews.llvm.org/D45015

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Driver/ToolChains/Darwin.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Driver/unavailable_aligned_allocation.cpp
  test/Lexer/aligned-allocation.cpp

Index: test/Lexer/aligned-allocation.cpp
===
--- /dev/null
+++ test/Lexer/aligned-allocation.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -DEXPECT_DEFINED
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-alloc-unavailable
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-allocation -faligned-alloc-unavailable
+
+// Test that __cpp_aligned_new is not defined when CC1 is passed
+// -faligned-alloc-unavailable by the Darwin driver, even when aligned
+// allocation is actually enabled.
+
+// expected-no-diagnostics
+#ifdef EXPECT_DEFINED
+# ifndef __cpp_aligned_new
+#   error "__cpp_aligned_new" should be defined
+# endif
+#else
+# ifdef __cpp_aligned_new
+#   error "__cpp_aligned_new" should not be defined
+# endif
+#endif
Index: test/Driver/unavailable_aligned_allocation.cpp
===
--- test/Driver/unavailable_aligned_allocation.cpp
+++ test/Driver/unavailable_aligned_allocation.cpp
@@ -51,4 +51,13 @@
 // RUN: -c -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=AVAILABLE
 //
+// Check that passing -faligned-allocation or -fno-aligned-allocation stops the
+// driver from passing -faligned-alloc-unavailable to cc1.
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -faligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -fno-aligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+
 // AVAILABLE-NOT: "-faligned-alloc-unavailable"
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1697,9 +1697,9 @@
 StringRef OSName = AvailabilityAttr::getPlatformNameSourceSpelling(
 S.getASTContext().getTargetInfo().getPlatformName());
 
-S.Diag(Loc, diag::warn_aligned_allocation_unavailable)
- << IsDelete << FD.getType().getAsString() << OSName
- << alignedAllocMinVersion(T.getOS()).getAsString();
+S.Diag(Loc, diag::err_aligned_allocation_unavailable)
+<< IsDelete << FD.getType().getAsString() << OSName
+<< alignedAllocMinVersion(T.getOS()).getAsString();
 S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
   }
 }
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -551,7 +551,7 @@
 Builder.defineMacro("__cpp_nontype_template_args", "201411");
 Builder.defineMacro("__cpp_fold_expressions", "201603");
   }
-  if (LangOpts.AlignedAllocation)
+  if (LangOpts.AlignedAllocation && !LangOpts.AlignedAllocationUnavailable)
 Builder.defineMacro("__cpp_aligned_new", "201606");
 
   // TS features.
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -2018,7 +2018,12 @@
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ,
Action::OffloadKind DeviceOffloadKind) const {
-  if (isAlignedAllocationUnavailable())
+  // Pass "-faligned-alloc-unavailable" only when the user hasn't manually
+  // enabled or disabled aligned allocations and hasn't specified a custom
+  // standard library installation.
+  if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
+options::OPT_fno_aligned_allocation) &&
+  isAlignedAllocationUnavailable())
 CC1Args.push_back("-faligned-alloc-unavailable");
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6419,12 +6419,12 @@
   "type %0 requires %1 bytes of alignment and the default allocator only "
   "guarantees %2 bytes">,
   InGroup, DefaultIgnore;
-def warn_aligned_allocation_unavailable :Warning<
+def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
-  "available on %2 %3 or newer">, InGroup, 

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45015#1127046, @vsapsai wrote:

> In https://reviews.llvm.org/D45015#1121762, @EricWF wrote:
>
> > In https://reviews.llvm.org/D45015#1121581, @ahatanak wrote:
> >
> > > Could you elaborate on what kind of changes you are planning to make in 
> > > libc++ after committing this patch?
> >
> >
> > Libc++ shouldn't actually need any changes if this current patch lands. 
> > Currently libc++ is in a "incorrect" state where
> >  it generates calls to `__builtin_operator_new(size_t, align_val_t)` when 
> > `__cpp_aligned_new` is defined but when aligned new/delete
> >  are actually unavailable.
> >
> > If we change `__cpp_aligned_new` to no longer be defined when aligned new 
> > is unavailable, then libc++ will start doing the right thing.
> >  See r328180 
> > 
> >  for the relevent commits which made these libc++ changes.
>
>
> Looks like in libc++ we need to remove `_LIBCPP_STD_VER` check for aligned 
> allocations, something like
>
>#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
>   -(!(defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER > 14 || \
>   +(!(defined(_LIBCPP_BUILDING_NEW) || \
>(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
># define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
>#endif
>
>
> Is that correct? I didn't check the rest of the code, probably 
> TEST_HAS_NO_ALIGNED_ALLOCATION needs some clean up too.


Huh, that original formulation is correct-ish, and so is your proposed changes. 
There are two problems here:

1. We want to expose the aligned allocation declarations to users in C++17, 
even if the compiler will never call them because

the feature has been disabled.

2. We don't want to generate calls to them via `__builtin_operator_new` when 
we've only enabled the declarations because of C++17.

I'll make some changes to libc++ to address this. Thanks for pointing it out.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D45015#1121762, @EricWF wrote:

> In https://reviews.llvm.org/D45015#1121581, @ahatanak wrote:
>
> > Could you elaborate on what kind of changes you are planning to make in 
> > libc++ after committing this patch?
>
>
> Libc++ shouldn't actually need any changes if this current patch lands. 
> Currently libc++ is in a "incorrect" state where
>  it generates calls to `__builtin_operator_new(size_t, align_val_t)` when 
> `__cpp_aligned_new` is defined but when aligned new/delete
>  are actually unavailable.
>
> If we change `__cpp_aligned_new` to no longer be defined when aligned new is 
> unavailable, then libc++ will start doing the right thing.
>  See r328180 
> 
>  for the relevent commits which made these libc++ changes.


Looks like in libc++ we need to remove `_LIBCPP_STD_VER` check for aligned 
allocations, something like

   #if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
  -(!(defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER > 14 || \
  +(!(defined(_LIBCPP_BUILDING_NEW) || \
   (defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
   # define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
   #endif

Is that correct? I didn't check the rest of the code, probably 
TEST_HAS_NO_ALIGNED_ALLOCATION needs some clean up too.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Sorry for the churn but can you please take out `-nostdinc++` part out of this 
change? After more thinking and discussion we think there is a chance 
developers can use `-nostdinc++` not only for building the standard library. 
`-nostdinc++` is a signal of building the standard library but it's not strong 
enough. Most likely developers will be unaware how `-nostdinc++` affects 
aligned allocations and that can lead to linker failures at runtime. So if you 
deploy on older macOS versions it is safer to assume aligned allocation isn't 
available. But if you provide your own libc++ with aligned allocations, you can 
still use `-faligned-allocation` to make that work. For now we prefer to use 
that approach and if it proves to be too onerous for developers, we can 
reintroduce `-nostdinc++` logic.

For building libc++ we rely on `_LIBCPP_BUILDING_NEW` to have support for 
aligned allocations even though the build host doesn't support them. So we 
don't need to change libc++ build.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45015#1123164, @ahatanak wrote:

> In https://reviews.llvm.org/D45015#1123097, @EricWF wrote:
>
> > In https://reviews.llvm.org/D45015#1121874, @ahatanak wrote:
> >
> > > I see, thank you.
> > >
> > > clang front-end currently fails to issue a warning or error when an 
> > > aligned allocation/deallocation functions are required but not available 
> > > in a few cases (e.g., delete called from a deleting destructor, calls to 
> > > operator or builtin operator new/delete). I suppose those bugs should be 
> > > fixed in separate patches.
> >
> >
> > I don't think we need to emit warnings from 
> > `__builtin_operator_new`/`__builtin_operator_delete`. Libc++ is the only 
> > consumer, and I think we can trust it to know what it's doing.
>
>
> Shouldn't clang warn when users explicitly call an aligned builtin operator 
> new or delete in their code and the OS is too old to support the operator?
>
> For example:
>
>   typedef __SIZE_TYPE__ size_t;
>   namespace std {
>   enum class align_val_t : size_t {};
>   }
>  
>   int main() {
> void *p = __builtin_operator_new(100, std::align_val_t(32));
> return 0;
>   }
>


Yeah, I think you're right. Initially I thought `__builtin_operator_new` was 
documented as being intended for usage only in the STL, but it doesn't quite 
say that.
I have a patch which moves the checks into `DiagnoseUseOfDecl`, which should 
catch all of the cases we haven't already handled.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D45015#1123097, @EricWF wrote:

> In https://reviews.llvm.org/D45015#1121874, @ahatanak wrote:
>
> > I see, thank you.
> >
> > clang front-end currently fails to issue a warning or error when an aligned 
> > allocation/deallocation functions are required but not available in a few 
> > cases (e.g., delete called from a deleting destructor, calls to operator or 
> > builtin operator new/delete). I suppose those bugs should be fixed in 
> > separate patches.
>
>
> I don't think we need to emit warnings from 
> `__builtin_operator_new`/`__builtin_operator_delete`. Libc++ is the only 
> consumer, and I think we can trust it to know what it's doing.


Shouldn't clang warn when users explicitly call an aligned builtin operator new 
or delete in their code and the OS is too old to support the operator?

For example:

  typedef __SIZE_TYPE__ size_t;
  namespace std {
  enum class align_val_t : size_t {};
  }
  
  int main() {
void *p = __builtin_operator_new(100, std::align_val_t(32));
return 0;
  }


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45015#1121874, @ahatanak wrote:

> I see, thank you.
>
> clang front-end currently fails to issue a warning or error when an aligned 
> allocation/deallocation functions are required but not available in a few 
> cases (e.g., delete called from a deleting destructor, calls to operator or 
> builtin operator new/delete). I suppose those bugs should be fixed in 
> separate patches.


I don't think we need to emit warnings from 
`__builtin_operator_new`/`__builtin_operator_delete`. Libc++ is the only 
consumer, and I think we can trust it to know what it's doing.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I see, thank you.

clang front-end currently fails to issue a warning or error when an aligned 
allocation/deallocation functions are required but not available in a few cases 
(e.g., delete called from a deleting destructor, calls to operator or builtin 
operator new/delete). I suppose those bugs should be fixed in separate patches.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45015#1121581, @ahatanak wrote:

> Could you elaborate on what kind of changes you are planning to make in 
> libc++ after committing this patch?


Libc++ shouldn't actually need any changes if this current patch lands. 
Currently libc++ is in a "incorrect" state where
it generates calls to `__builtin_operator_new(size_t, align_val_t)` when 
`__cpp_aligned_new` is defined but when aligned new/delete
are actually unavailable.

If we change `__cpp_aligned_new` to no longer be defined when aligned new is 
unavailable, then libc++ will start doing the right thing.
See r328180 

 for the relevent commits which made these libc++ changes.


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Could you elaborate on what kind of changes you are planning to make in libc++ 
after committing this patch?


https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 149596.
EricWF edited the summary of this revision.
EricWF added a comment.

Update the patch with the form suggested in the previous conversation. See the 
updated summary for a description of the behavior.

The Darwin driver no longer passes `-faligned-alloc-unavailable` when 
`-nostdinc++` is specified, but I'm not sure this is the behavior
we want.  Having `-nostdinc++` have side-effects which affect the 
well-formedness of new/delete expressions seems non-obvious and potentially 
problematic.


https://reviews.llvm.org/D45015

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Driver/ToolChains/Darwin.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Driver/unavailable_aligned_allocation.cpp
  test/Lexer/aligned-allocation.cpp

Index: test/Lexer/aligned-allocation.cpp
===
--- /dev/null
+++ test/Lexer/aligned-allocation.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -DEXPECT_DEFINED
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-alloc-unavailable
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-allocation -faligned-alloc-unavailable
+
+// Test that __cpp_aligned_new is not defined when CC1 is passed
+// -faligned-alloc-unavailable by the Darwin driver, even when aligned
+// allocation is actually enabled.
+
+// expected-no-diagnostics
+#ifdef EXPECT_DEFINED
+# ifndef __cpp_aligned_new
+#   error "__cpp_aligned_new" should be defined
+# endif
+#else
+# ifdef __cpp_aligned_new
+#   error "__cpp_aligned_new" should not be defined
+# endif
+#endif
Index: test/Driver/unavailable_aligned_allocation.cpp
===
--- test/Driver/unavailable_aligned_allocation.cpp
+++ test/Driver/unavailable_aligned_allocation.cpp
@@ -51,4 +51,16 @@
 // RUN: -c -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=AVAILABLE
 //
+// Check that passing -faligned-allocation, -fno-aligned-allocation, or
+// -nostdinc++ stops the driver from passing -faligned-alloc-unavailable to cc1.
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -faligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -fno-aligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -nostdinc++ -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+
 // AVAILABLE-NOT: "-faligned-alloc-unavailable"
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1697,7 +1697,7 @@
 StringRef OSName = AvailabilityAttr::getPlatformNameSourceSpelling(
 S.getASTContext().getTargetInfo().getPlatformName());
 
-S.Diag(Loc, diag::warn_aligned_allocation_unavailable)
+S.Diag(Loc, diag::err_aligned_allocation_unavailable)
 << IsDelete << FD.getType().getAsString() << OSName
 << alignedAllocMinVersion(T.getOS()).getAsString();
 S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -551,7 +551,7 @@
 Builder.defineMacro("__cpp_nontype_template_args", "201411");
 Builder.defineMacro("__cpp_fold_expressions", "201603");
   }
-  if (LangOpts.AlignedAllocation)
+  if (LangOpts.AlignedAllocation && !LangOpts.AlignedAllocationUnavailable)
 Builder.defineMacro("__cpp_aligned_new", "201606");
 
   // TS features.
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -2018,7 +2018,13 @@
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ,
Action::OffloadKind DeviceOffloadKind) const {
-  if (isAlignedAllocationUnavailable())
+  // Pass "-faligned-alloc-unavailable" only when the user hasn't manually
+  // enabled or disabled aligned allocations and hasn't specified a custom
+  // standard library installation.
+  if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
+options::OPT_fno_aligned_allocation,
+options::OPT_nostdincxx) &&
+  isAlignedAllocationUnavailable())
 CC1Args.push_back("-faligned-alloc-unavailable");
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45015#1119286, @ahatanak wrote:

> In https://reviews.llvm.org/D45015#1105388, @rsmith wrote:
>
> > In https://reviews.llvm.org/D45015#1105372, @vsapsai wrote:
> >
> > > What when compiler has `__builtin_operator_new`, 
> > > `__builtin_operator_delete`? If I build libc++ tests with recent Clang 
> > > which has these builtins and run tests with libc++.dylib from old Darwin, 
> > > there are no linkage errors. Should we define `__cpp_aligned_allocation` 
> > > in this case?
> >
> >
> > I don't see why that should make any difference -- those builtins call the 
> > same functions that the new and delete operator call. Perhaps libc++ isn't 
> > calling the forms of those builtins that take an alignment argument yet?
>
>
> It looks like clang currently doesn't issue a warning when a call to 
> __builtin_operator_new or __builtin_operator_delete calls an aligned 
> allocation function that is not support by the OS version. I suppose we 
> should fix this?
>
>   // no warning issued when triple is "thumbv7-apple-ios5.0.0" even though 
> aligned allocation is unavailable.
>   void *p = __builtin_operator_new(100, std::align_val_t(32));


If we switch to no longer defining `__cpp_aligned_new` when it's unavailable 
that means libc++ shouldn't generate calls to an aligned allocation or 
deallocation function. Do we need the compiler to protect libc++ from itself 
here?


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

The new path forward sounds good to me. I'll work on implementing it.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I think clang should error out or warn when aligned operator or builtin 
operator new/delete functions are used when they are not available (r306722 
should have handled them).

I'm also not sure not defining `__cpp_aligned_new` is sufficient. My 
understanding is that if `__cpp_aligned_new` is not defined, libc++ will define 
`_LIBCPP_HAS_NO_ALIGNED_ALLOCATION`, which will cause the aligned new/delete 
functions declared in  to be removed. That is fine if the new/delete 
function that is used in a program is one of the function implicitly declared 
by the compiler (e.g., 'void operator delete  ( void*, std::align_val_t)'), but 
if it isn't one of those functions (e.g., 'void operator delete (void *, 
std::align_val_t, const std::nothrow_t&)'), clang will produce diagnostics that 
are not very informative to the user (error: no matching function for call to 
'operator delete') instead of the one added in r306722 (aligned allocation 
function of type is only available on ...).

For example:

  #include 

  void foo1(void *p, std::align_val_t a, std::nothrow_t t) {
operator delete(p, a, t);
  }


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D45015#1105388, @rsmith wrote:

> In https://reviews.llvm.org/D45015#1105372, @vsapsai wrote:
>
> > What when compiler has `__builtin_operator_new`, 
> > `__builtin_operator_delete`? If I build libc++ tests with recent Clang 
> > which has these builtins and run tests with libc++.dylib from old Darwin, 
> > there are no linkage errors. Should we define `__cpp_aligned_allocation` in 
> > this case?
>
>
> I don't see why that should make any difference -- those builtins call the 
> same functions that the new and delete operator call. Perhaps libc++ isn't 
> calling the forms of those builtins that take an alignment argument yet?


It looks like clang currently doesn't issue a warning when a call to 
__builtin_operator_new or __builtin_operator_delete calls an aligned allocation 
function that is not support by the OS version. I suppose we should fix this?

  // no warning issued when triple is "thumbv7-apple-ios5.0.0" even though 
aligned allocation is unavailable.
  void *p = __builtin_operator_new(100, std::align_val_t(32));


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D45015#1109049, @ahatanak wrote:

> - Currently clang errors out when aligned operator new is selected but the 
> OS's version is too old to support it. What's the reason we want to change 
> this now to be a warning rather than an error?


I think it's fine to leave it as a `DefaultError` warning. I think it'd also be 
fine to change it to simply be an error (rather than an 
error-with-warning-flag) and remove the error recovery, and that actually seems 
a bit better: then our behavior under -faligned-alloc-unavailable would be 
equivalent to treating the implicit aligned forms of operator new/delete as if 
they had an availability attribute on them by default, which I think seems very 
reasonable.

> - So clang no longer needs to define macro 
> `__ALIGNED_ALLOCATION_UNAVAILABLE__` and libc++ will use `__cpp_aligned_new` 
> (I think you meant `__cpp_aligned_new`, not `__cpp_aligned_allocation`?) to 
> determine whether aligned allocation functions should be defined or made 
> available in the header?

Yes, that's the idea. `__cpp_aligned_new` exists to allow code to conditionally 
use aligned new if it's available, and we should do our best to honor that.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Somewhat tangential, in discussion with Duncan he mentioned that `-nostdinc++` 
should turn off assumptions about old Darwin. So if you build libc++ yourself, 
you don't care what does the system stdlib support. I agree with that and think 
it doesn't interfere with the latest proposal but adds more to it.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D45015#1105314, @rsmith wrote:

> Hmm, perhaps our strategy for handling aligned allocation on Darwin should be 
> revisited. We shouldn't be defining `__cpp_aligned_allocation` if we believe 
> it doesn't work -- that will break code that uses aligned allocation where 
> available and falls back to something else where it's unavailable.
>
> @ahatanak: how about this:
>
> - Change the driver to not pass `-faligned-alloc-unavailable` if an explicit 
> `-faligned-allocation` or `-fno-aligned-allocation` flag is given; update 
> Clang's note to suggest explicitly passing `-faligned-allocation` rather than 
> `-Wno-aligned-alloc-unavailable` if the user provides their own aligned 
> allocation function.
> - Change `-faligned-alloc-unavailable` so that it does not define 
> `__cpp_aligned_allocation`.
> - Change Sema's handling of the "aligned allocation unavailable" case so 
> that, after warning on selecting an aligned allocation function, it then 
> removes those functions from the candidate set and tries again.
>
>   That is: on old Darwin, we should not define `__cpp_aligned_allocation` 
> (even in C++17), produce the "no aligned allocation support" warning in C++17 
> mode, and then not try to call the aligned allocation function. But if 
> `-faligned-allocation` or `-fno-aligned-allocation` is specified explicitly, 
> then the user knows what they're doing and they get no warning.


I talked with @vsapsai today and we think this looks like a better idea.

A couple of questions:

- Currently clang errors out when aligned operator new is selected but the OS's 
version is too old to support it. What's the reason we want to change this now 
to be a warning rather than an error?

- So clang no longer needs to define macro `__ALIGNED_ALLOCATION_UNAVAILABLE__` 
and libc++ will use `__cpp_aligned_new` (I think you meant `__cpp_aligned_new`, 
not `__cpp_aligned_allocation`?) to determine whether aligned allocation 
functions should be defined or made available in the header?


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D45015#1105372, @vsapsai wrote:

> What when compiler has `__builtin_operator_new`, `__builtin_operator_delete`? 
> If I build libc++ tests with recent Clang which has these builtins and run 
> tests with libc++.dylib from old Darwin, there are no linkage errors. Should 
> we define `__cpp_aligned_allocation` in this case?


I don't see why that should make any difference -- those builtins call the same 
functions that the new and delete operator call. Perhaps libc++ isn't calling 
the forms of those builtins that take an alignment argument yet?


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D45015#1105314, @rsmith wrote:

> That is: on old Darwin, we should not define `__cpp_aligned_allocation` (even 
> in C++17), produce the "no aligned allocation support" warning in C++17 mode, 
> and then not try to call the aligned allocation function. But if 
> `-faligned-allocation` or `-fno-aligned-allocation` is specified explicitly, 
> then the user knows what they're doing and they get no warning.


What when compiler has `__builtin_operator_new`, `__builtin_operator_delete`? 
If I build libc++ tests with recent Clang which has these builtins and run 
tests with libc++.dylib from old Darwin, there are no linkage errors. Should we 
define `__cpp_aligned_allocation` in this case?


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Hmm, perhaps our strategy for handling aligned allocation on Darwin should be 
revisited. We shouldn't be defining `__cpp_aligned_allocation` if we believe it 
doesn't work -- that will break code that uses aligned allocation where 
available and falls back to something else where it's unavailable.

@ahatanak: how about this:

- Change the driver to not pass `-faligned-alloc-unavailable` if an explicit 
`-faligned-allocation` or `-fno-aligned-allocation` flag is given; update 
Clang's note to suggest explicitly passing `-faligned-allocation` rather than 
`-Wno-aligned-alloc-unavailable` if the user provides their own aligned 
allocation function.
- Change `-faligned-alloc-unavailable` so that it does not define 
`__cpp_aligned_allocation`.
- Change Sema's handling of the "aligned allocation unavailable" case so that, 
after warning on selecting an aligned allocation function, it then removes 
those functions from the candidate set and tries again.

That is: on old Darwin, we should not define `__cpp_aligned_allocation` (even 
in C++17), produce the "no aligned allocation support" warning in C++17 mode, 
and then not try to call the aligned allocation function. But if 
`-faligned-allocation` or `-fno-aligned-allocation` is specified explicitly, 
then the user knows what they're doing and they get no warning.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Eric, do you have more thoughts on this issue?


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D45015#1064930, @EricWF wrote:

> In https://reviews.llvm.org/D45015#1064922, @vsapsai wrote:
>
> > Another approach is `__has_feature` but I don't think it is applicable in 
> > this case.
> >
> > Is there a way right now to detect that aligned allocation is supported by 
> > clang, regardless of link time? Asking to make sure we are consistent.
>
>
> There is a C++ feature test macro, as specified by the standard, 
> `__cpp_aligned_new`.  But I'm not sure how to be consistent with that.


After more thinking I believe a predefined macro is the best option. And in 
spirit it is consistent with SD-6: SG10 Feature Test Recommendations 
.

As for the naming, I see it is consistent with `-faligned-alloc-unavailable` 
but for me it is hard to tell immediately if "unavailable" means "unsupported 
at all" or "supported but unavailable on the target". Maybe it is just me and 
others don't have such problem but probably including "runtime" in the macro 
name would help. Something like `__ALIGNED_ALLOCATION_UNAVAILABLE_RUNTIME__`.

For the overall aligned allocation plan overall. We also need to keep in mind 
users who provide their own aligned allocation / deallocation functions, so 
they can still do that. So far everything seems to be OK but we need to be 
careful.




Comment at: lib/Frontend/InitPreprocessor.cpp:1059-1060
 
+  if (!LangOpts.AlignedAllocation || LangOpts.AlignedAllocationUnavailable)
+Builder.defineMacro("__ALIGNED_ALLOCATION_UNAVAILABLE__");
+

Don't know what the macro will be in the end, please consider adding a comment 
clarifying runtime availability.



Comment at: test/Preprocessor/predefined-macros.c:288
+// RUN: %clang_cc1 %s -x c++ -E -dM -faligned-allocation 
-faligned-alloc-unavailable  \
+// RUN: | FileCheck %s -match-full-lines 
-check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+

Would it be useful to test `-fno-aligned-allocation` too? Extensive testing in 
different combinations doesn't add much value but `-std=c++17 
-fno-aligned-allocation` can be useful.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45015#1064922, @vsapsai wrote:

> Another approach is `__has_feature` but I don't think it is applicable in 
> this case.
>
> Is there a way right now to detect that aligned allocation is supported by 
> clang, regardless of link time? Asking to make sure we are consistent.


There is a C++ feature test macro, as specified by the standard, 
`__cpp_aligned_new`.  But I'm not sure how to be consistent with that.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Another approach is `__has_feature` but I don't think it is applicable in this 
case.

Is there a way right now to detect that aligned allocation is supported by 
clang, regardless of link time? Asking to make sure we are consistent.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

Ping. I would like to get a way for libc++ to detect this case before the next 
release.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.
EricWF added reviewers: rsmith, vsapsai, erik.pilkington, ahatanak.

Libc++ needs to know when aligned allocation is supported by clang, but is 
otherwise unavailable at link time. This patch adds a predefined macro to allow 
libc++ to do that.

IDK if using a predefined macro is the best approach. Better approaches are 
always welcome :-)

Also see llvm.org/PR22634


Repository:
  rC Clang

https://reviews.llvm.org/D45015

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Preprocessor/predefined-macros.c


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -271,3 +271,20 @@
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR
 // CHECK-SPIR: #define __IMAGE_SUPPORT__ 1
+
+
+// RUN: %clang_cc1 %s -x c++ -E -dM  -faligned-allocation \
+// RUN: | FileCheck  %s -check-prefix=CHECK-ALIGNED-ALLOC-AVAILABLE
+// RUN: %clang_cc1 %s -x c++ -E -dM  -std=c++17 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ALIGNED-ALLOC-AVAILABLE
+
+// CHECK-ALIGNED-ALLOC-AVAILABLE-NOT: __ALIGNED_ALLOCATION_UNAVAILABLE__
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -std=c++03  \
+// RUN: | FileCheck %s -match-full-lines 
-check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+// RUN: %clang_cc1 %s -x c++ -E -dM -faligned-alloc-unavailable  \
+// RUN: | FileCheck %s -match-full-lines 
-check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+// RUN: %clang_cc1 %s -x c++ -E -dM -faligned-allocation 
-faligned-alloc-unavailable  \
+// RUN: | FileCheck %s -match-full-lines 
-check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+
+// CHECK-ALIGNED-ALLOC-UNAVAILABLE: #define __ALIGNED_ALLOCATION_UNAVAILABLE__ 
1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1056,6 +1056,9 @@
 Builder.defineMacro("__GLIBCXX_BITSIZE_INT_N_0", "128");
   }
 
+  if (!LangOpts.AlignedAllocation || LangOpts.AlignedAllocationUnavailable)
+Builder.defineMacro("__ALIGNED_ALLOCATION_UNAVAILABLE__");
+
   // Get other target #defines.
   TI.getTargetDefines(LangOpts, Builder);
 }


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -271,3 +271,20 @@
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR
 // CHECK-SPIR: #define __IMAGE_SUPPORT__ 1
+
+
+// RUN: %clang_cc1 %s -x c++ -E -dM  -faligned-allocation \
+// RUN: | FileCheck  %s -check-prefix=CHECK-ALIGNED-ALLOC-AVAILABLE
+// RUN: %clang_cc1 %s -x c++ -E -dM  -std=c++17 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ALIGNED-ALLOC-AVAILABLE
+
+// CHECK-ALIGNED-ALLOC-AVAILABLE-NOT: __ALIGNED_ALLOCATION_UNAVAILABLE__
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -std=c++03  \
+// RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+// RUN: %clang_cc1 %s -x c++ -E -dM -faligned-alloc-unavailable  \
+// RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+// RUN: %clang_cc1 %s -x c++ -E -dM -faligned-allocation -faligned-alloc-unavailable  \
+// RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+
+// CHECK-ALIGNED-ALLOC-UNAVAILABLE: #define __ALIGNED_ALLOCATION_UNAVAILABLE__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1056,6 +1056,9 @@
 Builder.defineMacro("__GLIBCXX_BITSIZE_INT_N_0", "128");
   }
 
+  if (!LangOpts.AlignedAllocation || LangOpts.AlignedAllocationUnavailable)
+Builder.defineMacro("__ALIGNED_ALLOCATION_UNAVAILABLE__");
+
   // Get other target #defines.
   TI.getTargetDefines(LangOpts, Builder);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits