EricWF created this revision.
EricWF added reviewers: rsmith, majnemer, aaron.ballman, erik.pilkington, 
bogner, ahatanak.

Libc++'s default allocator uses `__builtin_operator_new` and 
`__builtin_operator_delete` in order to allow the calls to new/delete to be 
ellided. However, libc++ now needs to support over-aligned types in the default 
allocator. In order to support this without disabling the existing optimization 
Clang needs to support calling the aligned new overloads from the builtins.

See llvm.org/PR22634 for more information about the libc++ bug.

This patch changes `__builtin_operator_new`/`__builtin_operator_delete` to 
allow passing an optional alignment parameter. The two parameter overloads will 
forward to the aligned versions of `operator new` and `operator delete`.

Additionally, libc++ needs a way to detect these new overloads. There seems to 
be no great way to do this. This patch chooses to add the feature check 
`has_aligned_builtin_operator_new`. Suggestions on how to better handle this 
case are very welcome!


https://reviews.llvm.org/D43047

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGenCXX/new.cpp
  test/Lexer/has_feature_aligned_builtin_new.cpp
  test/SemaCXX/builtin-operator-new-delete-alignment-disabled.cpp
  test/SemaCXX/builtin-operator-new-delete.cpp

Index: test/SemaCXX/builtin-operator-new-delete.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/builtin-operator-new-delete.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c++1z -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++03 -faligned-allocation -fsyntax-only -verify %s
+
+void *NP = 0;
+
+void test_signature() {
+  __builtin_operator_new();           // expected-error {{too few arguments to function call, expected 1, have 0}}
+  __builtin_operator_new(0, 0, 0);    // expected-error {{too many arguments to function call, expected 2, have 3}}
+  __builtin_operator_delete();        // expected-error {{too few arguments to function call, expected 1, have 0}}
+  __builtin_operator_delete(0, 0, 0); // expected-error {{too many arguments to function call, expected 2, have 3}}
+}
+
+void test_typo_in_args() {
+  __builtin_operator_new(DNE);          // expected-error {{undeclared identifier 'DNE'}}
+  __builtin_operator_new(DNE, DNE2);    // expected-error {{undeclared identifier 'DNE'}} expected-error {{'DNE2'}}
+  __builtin_operator_delete(DNE);       // expected-error {{'DNE'}}
+  __builtin_operator_delete(DNE, DNE2); // expected-error {{'DNE'}} expected-error {{'DNE2'}}
+}
+
+void test_arg_types() {
+  __builtin_operator_new(NP);    // expected-error {{cannot initialize a parameter of type 'unsigned long' with an lvalue of type 'void *'}}
+  __builtin_operator_new(NP, 0); // expected-error {{cannot initialize a parameter of type 'unsigned long' with an lvalue of type 'void *'}}
+}
+
+void test_return_type() {
+  int w = __builtin_operator_new(42);        // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void *'}}
+  int x = __builtin_operator_new(42, 42);    // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void *'}}
+  int y = __builtin_operator_delete(NP);     // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void'}}
+  int z = __builtin_operator_delete(NP, 42); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void'}}
+}
Index: test/SemaCXX/builtin-operator-new-delete-alignment-disabled.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/builtin-operator-new-delete-alignment-disabled.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++03 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++03 -faligned-allocation -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++1z -fno-aligned-allocation -fsyntax-only -verify %s
+
+void test_disabled() {
+#ifdef __cpp_aligned_new
+  // expected-no-diagnostics
+#else
+  // expected-error@+3 {{__builtin_operator_new with alignment requires aligned allocation support; use -faligned-allocation to enable}}
+  // expected-error@+3 {{__builtin_operator_delete with alignment requires aligned allocation support; use -faligned-allocation to enable}}
+#endif
+  (void)__builtin_operator_new(1, 2);
+  __builtin_operator_delete((void *)0, 2);
+}
Index: test/Lexer/has_feature_aligned_builtin_new.cpp
===================================================================
--- /dev/null
+++ test/Lexer/has_feature_aligned_builtin_new.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -E -triple x86_64-linux-gnu -std=c++11 %s -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-NO-BUILTIN %s
+// RUN: %clang_cc1 -E -triple x86_64-linux-gnu -std=c++11 -faligned-allocation %s -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-HAS-BUILTIN %s
+// RUN: %clang_cc1 -E -triple x86_64-linux-gnu -std=c++1z %s -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-HAS-BUILTIN %s
+// RUN: %clang_cc1 -E -triple x86_64-linux-gnu -std=c++1z -fno-aligned-allocation %s -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-NO-BUILTIN %s
+
+#if __has_feature(has_aligned_builtin_operator_new)
+void has_builtin();
+#else
+void no_builtin();
+#endif
+
+// CHECK-HAS-BUILTIN: has_builtin
+// CHECK-NO-BUILTIN: no_builtin
Index: test/CodeGenCXX/new.cpp
===================================================================
--- test/CodeGenCXX/new.cpp
+++ test/CodeGenCXX/new.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -verify -faligned-allocation \
+// RUN:    -emit-llvm -o - | FileCheck %s
 
 typedef __typeof__(sizeof(0)) size_t;
 
@@ -363,6 +364,12 @@
     // CHECK: call void @_ZdlPv({{.*}}) [[ATTR_BUILTIN_DELETE]]
     __builtin_operator_delete(__builtin_operator_new(4));
   }
+  // CHECK-LABEL: define void @_ZN8builtins1gEv
+  void g() {
+    // CHECK: call i8* @_ZnwmSt11align_val_t(i64 4, i64 4) [[ATTR_BUILTIN_NEW]]
+    // CHECK: call void @_ZdlPvSt11align_val_t({{.*}}, i64 4) [[ATTR_BUILTIN_DELETE]]
+    __builtin_operator_delete(__builtin_operator_new(4, 4), 4);
+  }
 }
 
 // CHECK-DAG: attributes [[ATTR_NOBUILTIN]] = {{[{].*}} nobuiltin {{.*[}]}}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -36,6 +36,7 @@
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Analysis/Analyses/FormatString.h"
 #include "clang/Basic/AddressSpaces.h"
+#include "clang/Basic/AlignedAllocation.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -1097,20 +1098,14 @@
       return ExprError();
     break;
   case Builtin::BI__builtin_operator_new:
-  case Builtin::BI__builtin_operator_delete:
-    if (!getLangOpts().CPlusPlus) {
-      Diag(TheCall->getExprLoc(), diag::err_builtin_requires_language)
-        << (BuiltinID == Builtin::BI__builtin_operator_new
-                ? "__builtin_operator_new"
-                : "__builtin_operator_delete")
-        << "C++";
-      return ExprError();
-    }
-    // CodeGen assumes it can find the global new and delete to call,
-    // so ensure that they are declared.
-    DeclareGlobalNewDelete();
-    break;
-
+  case Builtin::BI__builtin_operator_delete: {
+    bool IsDelete = BuiltinID == Builtin::BI__builtin_operator_delete;
+    ExprResult Res =
+        SemaBuiltinOperatorNewDeleteOverloaded(TheCallResult, IsDelete);
+    if (Res.isInvalid())
+      CorrectDelayedTyposInExpr(TheCallResult.get());
+    return Res;
+  }
   // check secure string manipulation functions where overflows
   // are detectable at compile time
   case Builtin::BI__builtin___memcpy_chk:
@@ -2904,6 +2899,71 @@
   }
 }
 
+ExprResult
+Sema::SemaBuiltinOperatorNewDeleteOverloaded(ExprResult TheCallResult,
+                                             bool IsDelete) {
+  CallExpr *TheCall = cast<CallExpr>(TheCallResult.get());
+  if (!getLangOpts().CPlusPlus) {
+    Diag(TheCall->getExprLoc(), diag::err_builtin_requires_language)
+        << (IsDelete ? "__builtin_operator_delete" : "__builtin_operator_new")
+        << "C++";
+    return ExprError();
+  }
+  // CodeGen assumes it can find the global new and delete to call,
+  // so ensure that they are declared.
+  DeclareGlobalNewDelete();
+
+  TheCall->setType(IsDelete ? Context.VoidTy : Context.VoidPtrTy);
+
+  if (TheCall->getNumArgs() < 1) {
+    Diag(TheCall->getLocEnd(), diag::err_typecheck_call_too_few_args)
+        << 0 << 1 << TheCall->getNumArgs()
+        << TheCall->getCallee()->getSourceRange();
+    return ExprError();
+  } else if (TheCall->getNumArgs() > 2) {
+    Diag(TheCall->getArg(2)->getLocStart(),
+         diag::err_typecheck_call_too_many_args)
+        << 0 << 2 << TheCall->getNumArgs()
+        << TheCall->getCallee()->getSourceRange();
+    return ExprError();
+  }
+
+  llvm::SmallVector<QualType, 2> ArgTypes;
+  ArgTypes.push_back(IsDelete ? Context.VoidPtrTy : Context.getSizeType());
+
+  if (TheCall->getNumArgs() == 2) {
+    if (!getLangOpts().AlignedAllocation) {
+      Diag(TheCall->getArg(1)->getLocStart(),
+           diag::err_builtin_operator_new_requires_aligned_allocation)
+          << IsDelete << TheCall->getSourceRange();
+      return ExprError();
+    }
+    if (getLangOpts().AlignedAllocationUnavailable) {
+      const llvm::Triple &T = Context.getTargetInfo().getTriple();
+      StringRef OSName = AvailabilityAttr::getPlatformNameSourceSpelling(
+          Context.getTargetInfo().getPlatformName());
+      auto Loc = TheCall->getExprLoc();
+      Diag(Loc, diag::warn_aligned_builtin_operator_new_unavailable)
+          << IsDelete << OSName
+          << alignedAllocMinVersion(T.getOS()).getAsString();
+      Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
+    }
+    ArgTypes.push_back(getStdAlignValT()->getIntegerType());
+  }
+
+  for (unsigned i = 0; i != TheCall->getNumArgs(); ++i) {
+    InitializedEntity Entity =
+        InitializedEntity::InitializeParameter(Context, ArgTypes[i], false);
+    ExprResult Arg = TheCall->getArg(i);
+    Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
+    if (Arg.isInvalid())
+      return ExprError();
+    TheCall->setArg(i, Arg.get());
+  }
+
+  return TheCallResult;
+}
+
 ExprResult Sema::SemaAtomicOpsOverloaded(ExprResult TheCallResult,
                                          AtomicExpr::AtomicOp Op) {
   CallExpr *TheCall = cast<CallExpr>(TheCallResult.get());
Index: lib/Lex/PPMacroExpansion.cpp
===================================================================
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -1238,6 +1238,10 @@
       // NOTE: For features covered by SD-6, it is preferable to provide *only*
       // the SD-6 macro and not a __has_feature check.
 
+      // Clang specific changes which libc++ needs to be able to detect
+      .Case("has_aligned_builtin_operator_new",
+            LangOpts.CPlusPlus && LangOpts.AlignedAllocation)
+
       // C++ TSes
       //.Case("cxx_runtime_arrays", LangOpts.CPlusPlusTSArrays)
       //.Case("cxx_concepts", LangOpts.CPlusPlusTSConcepts)
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2346,7 +2346,7 @@
                       CharUnits CookieSize = CharUnits());
 
   RValue EmitBuiltinNewDeleteCall(const FunctionProtoType *Type,
-                                  const Expr *Arg, bool IsDelete);
+                                  const Expr *TheCallExpr, bool IsDelete);
 
   llvm::Value *EmitCXXTypeidExpr(const CXXTypeidExpr *E);
   llvm::Value *EmitDynamicCast(Address V, const CXXDynamicCastExpr *DCE);
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -1307,19 +1307,39 @@
 }
 
 RValue CodeGenFunction::EmitBuiltinNewDeleteCall(const FunctionProtoType *Type,
-                                                 const Expr *Arg,
-                                                 bool IsDelete) {
+                                                 const Expr *E, bool IsDelete) {
+  const auto *TheCall = dyn_cast<CallExpr>(E);
+  llvm::SmallVector<QualType, 2> ArgTypes;
+  for (auto *Arg : TheCall->arguments())
+    ArgTypes.push_back(Arg->getType());
   CallArgList Args;
-  const Stmt *ArgS = Arg;
-  EmitCallArgs(Args, *Type->param_type_begin(), llvm::makeArrayRef(ArgS));
+  EmitCallArgs(Args, ArgTypes, TheCall->arguments());
   // Find the allocation or deallocation function that we're calling.
   ASTContext &Ctx = getContext();
   DeclarationName Name = Ctx.DeclarationNames
       .getCXXOperatorName(IsDelete ? OO_Delete : OO_New);
+
+  auto IsMatchingDecl = [&](FunctionDecl *FD) {
+    if (TheCall->getNumArgs() != FD->getNumParams())
+      return false;
+    if (!Ctx.hasSameType(TheCall->getArg(0)->getType(),
+                         FD->getParamDecl(0)->getType()))
+      return false;
+    if (TheCall->getNumArgs() == 2 &&
+        !FD->getParamDecl(1)->getType()->isAlignValT())
+      return false;
+    // FIXME: Should we be comparing things like the exception type of the
+    // functions to ensure it perfectly matches the standard?
+    if (Type->getExtInfo() !=
+        FD->getType()->getAs<FunctionProtoType>()->getExtInfo())
+      return false;
+    return true;
+  };
+
   for (auto *Decl : Ctx.getTranslationUnitDecl()->lookup(Name))
     if (auto *FD = dyn_cast<FunctionDecl>(Decl))
-      if (Ctx.hasSameType(FD->getType(), QualType(Type, 0)))
-        return EmitNewDeleteCall(*this, cast<FunctionDecl>(Decl), Type, Args);
+      if (IsMatchingDecl(FD))
+        return EmitNewDeleteCall(*this, FD, Type, Args);
   llvm_unreachable("predeclared global operator new/delete is missing");
 }
 
Index: lib/CodeGen/CGBuiltin.cpp
===================================================================
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2612,10 +2612,11 @@
     return RValue::get(EmitLValue(E->getArg(0)).getPointer());
   case Builtin::BI__builtin_operator_new:
     return EmitBuiltinNewDeleteCall(FD->getType()->castAs<FunctionProtoType>(),
-                                    E->getArg(0), false);
+                                    E, false);
   case Builtin::BI__builtin_operator_delete:
     return EmitBuiltinNewDeleteCall(FD->getType()->castAs<FunctionProtoType>(),
-                                    E->getArg(0), true);
+                                    E, true);
+
   case Builtin::BI__noop:
     // __noop always evaluates to an integer literal zero.
     return RValue::get(ConstantInt::get(IntTy, 0));
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10363,6 +10363,8 @@
   ExprResult SemaBuiltinNontemporalOverloaded(ExprResult TheCallResult);
   ExprResult SemaAtomicOpsOverloaded(ExprResult TheCallResult,
                                      AtomicExpr::AtomicOp Op);
+  ExprResult SemaBuiltinOperatorNewDeleteOverloaded(ExprResult TheCallResult,
+                                                    bool IsDelete);
   bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
                               llvm::APSInt &Result);
   bool SemaBuiltinConstantArgRange(CallExpr *TheCall, int ArgNum,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6510,10 +6510,15 @@
 def warn_aligned_allocation_unavailable :Warning<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
   "available on %2 %3 or newer">, InGroup<AlignedAllocationUnavailable>, DefaultError;
+def warn_aligned_builtin_operator_new_unavailable :Warning<
+  "aligned %select{__builtin_operator_new|__builtin_operator_delete}0 function is only "
+  "available on %2 %3 or newer">, InGroup<AlignedAllocationUnavailable>, DefaultError;
 def note_silence_unligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
   "-Wno-aligned-allocation-unavailable to silence this diagnostic">;
-
+def err_builtin_operator_new_requires_aligned_allocation : Error<
+  "%select{__builtin_operator_new|__builtin_operator_delete}0 with alignment "
+  "requires aligned allocation support; use -faligned-allocation to enable">;
 def err_conditional_void_nonvoid : Error<
   "%select{left|right}1 operand to ? is void, but %select{right|left}1 operand "
   "is of type %0">;
Index: include/clang/Basic/Builtins.def
===================================================================
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1371,8 +1371,9 @@
 
 // Clang builtins (not available in GCC).
 BUILTIN(__builtin_addressof, "v*v&", "nct")
-BUILTIN(__builtin_operator_new, "v*z", "c")
-BUILTIN(__builtin_operator_delete, "vv*", "n")
+BUILTIN(__builtin_operator_new, "v*z", "tc")
+BUILTIN(__builtin_operator_delete, "vv*", "tn")
+
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
 
 // Safestack builtins
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D43047: [... Eric Fiselier via Phabricator via cfe-commits

Reply via email to