r229241 - Revise the implementation logic of sized deallocation: Do not automatically generate weak definitions of the sized operator delete (in terms of unsized operator delete). Instead, provide the funcitonality via a new compiler flag, -fdef-sized-delete.
Richard Smith
richard at metafoo.co.uk
Thu Feb 19 15:25:35 PST 2015
On Thu, Feb 19, 2015 at 12:29 PM, Larisse Voufo <lvoufo at google.com> wrote:
>
> On Feb 17, 2015 5:07 PM, "Larisse Voufo" <lvoufo at google.com> wrote:
> >
> >
> >
> > On Sun, Feb 15, 2015 at 6:10 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> >>
> >> On Fri, Feb 13, 2015 at 9:42 PM, Larisse Voufo <lvoufo at google.com>
> wrote:
> >>>
> >>> Author: lvoufo
> >>> Date: Fri Feb 13 23:42:57 2015
> >>> New Revision: 229241
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=229241&view=rev
> >>> Log:
> >>> Revise the implementation logic of sized deallocation: Do not
> automatically generate weak definitions of the sized operator delete (in
> terms of unsized operator delete). Instead, provide the funcitonality via a
> new compiler flag, -fdef-sized-delete.
> >>> The current implementation causes link-time ODR violations when the
> delete symbols are exported into the dynamic table.
> >>>
> >>> Modified:
> >>> cfe/trunk/include/clang/Basic/LangOptions.def
> >>> cfe/trunk/include/clang/Driver/Options.td
> >>> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> >>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> >>> cfe/trunk/lib/Driver/Tools.cpp
> >>> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >>> cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
> >>> cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp
> >>>
> >>> Modified: cfe/trunk/include/clang/Basic/LangOptions.def
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/include/clang/Basic/LangOptions.def (original)
> >>> +++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Feb 13 23:42:57
> 2015
> >>> @@ -162,6 +162,7 @@ LANGOPT(CUDAIsDevice , 1, 0, "Compi
> >>>
> >>> LANGOPT(AssumeSaneOperatorNew , 1, 1, "implicit
> __attribute__((malloc)) for C++'s new operators")
> >>> LANGOPT(SizedDeallocation , 1, 0, "enable sized deallocation
> functions")
> >>> +LANGOPT(DefaultSizedDelete , 1, 0, "Generate weak definitions of
> sized delete")
> >>
> >>
> >> Please start this string with a lowercase letter. How about
> DefineSizedDeallocation, to match the existing SizedDeallocation LangOpt?
> ("Define" seems like a better word than "Default" here in any case.)
> >>
> >>>
> >>> BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor
> elision")
> >>> BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of
> IRgen'd records")
> >>> BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of
> IRgen'd records in a simple form")
> >>>
> >>> Modified: cfe/trunk/include/clang/Driver/Options.td
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/include/clang/Driver/Options.td (original)
> >>> +++ cfe/trunk/include/clang/Driver/Options.td Fri Feb 13 23:42:57 2015
> >>> @@ -394,6 +394,8 @@ def fasm_blocks : Flag<["-"], "fasm-bloc
> >>> def fno_asm_blocks : Flag<["-"], "fno-asm-blocks">, Group<f_Group>;
> >>>
> >>> def fassume_sane_operator_new : Flag<["-"],
> "fassume-sane-operator-new">, Group<f_Group>;
> >>> +def fdef_sized_delete: Flag<["-"], "fdef-sized-delete">,
> Group<f_Group>,
> >>> + HelpText<"Allow compiler-generated definition of sized deallocation
> function">, Flags<[CC1Option]>;
> >>
> >>
> >> Likewise here, how about -fdefine-sized-deallocation, to match the
> existing -fsized-deallocation flag?
> >
> >
> > Thanks, Richard. Please see r229597 for the above comments.
> >
> >>
> >>
> >>> def fastcp : Flag<["-"], "fastcp">, Group<f_Group>;
> >>> def fastf : Flag<["-"], "fastf">, Group<f_Group>;
> >>> def fast : Flag<["-"], "fast">, Group<f_Group>;
> >>>
> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> >>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Feb 13 23:42:57 2015
> >>> @@ -891,8 +891,11 @@ void CodeGenFunction::GenerateCode(Globa
> >>> } else if (FunctionDecl *UnsizedDealloc =
> >>>
> FD->getCorrespondingUnsizedGlobalDeallocationFunction()) {
> >>> // Global sized deallocation functions get an implicit weak
> definition if
> >>> - // they don't have an explicit definition.
> >>> + // they don't have an explicit definition, if allowed.
> >>> + assert(getLangOpts().DefaultSizedDelete &&
> >>> + "Can't emit unallowed definition.");
> >>> EmitSizedDeallocationFunction(*this, UnsizedDealloc);
> >>> +
> >>> } else
> >>> llvm_unreachable("no definition for emitted function");
> >>>
> >>>
> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> >>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Feb 13 23:42:57 2015
> >>> @@ -1615,13 +1615,13 @@ CodeGenModule::GetOrCreateLLVMFunction(S
> >>> // don't need it anymore).
> >>> addDeferredDeclToEmit(F, DDI->second);
> >>> DeferredDecls.erase(DDI);
> >>> -
> >>> +
> >>> // Otherwise, if this is a sized deallocation function, emit a
> weak
> >>> - // definition
> >>> - // for it at the end of the translation unit.
> >>> + // definition for it at the end of the translation unit.
> >>> } else if (D && cast<FunctionDecl>(D)
> >>>
> ->getCorrespondingUnsizedGlobalDeallocationFunction()) {
> >>> - addDeferredDeclToEmit(F, GD);
> >>> + if (getLangOpts().DefaultSizedDelete)
> >>> + addDeferredDeclToEmit(F, GD);
> >>>
> >>> // Otherwise, there are cases we have to worry about where we're
> >>> // using a declaration for which we must emit a definition but
> where
> >>>
> >>> Modified: cfe/trunk/lib/Driver/Tools.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> >>> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Feb 13 23:42:57 2015
> >>> @@ -4243,6 +4243,11 @@ void Clang::ConstructJob(Compilation &C,
> >>> if (!Args.hasFlag(options::OPT_fassume_sane_operator_new,
> >>> options::OPT_fno_assume_sane_operator_new))
> >>> CmdArgs.push_back("-fno-assume-sane-operator-new");
> >>> +
> >>> + // -def-sized-delete: default implementation of sized delete as a
> >>> + // weak definition.
> >>> + if (Args.hasArg(options::OPT_fdef_sized_delete))
> >>> + CmdArgs.push_back("-fdef-sized-delete");
> >>>
> >>> // -fconstant-cfstrings is default, and may be subject to argument
> translation
> >>> // on Darwin.
> >>>
> >>> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> >>> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Feb 13 23:42:57
> 2015
> >>> @@ -1525,6 +1525,8 @@ static void ParseLangArgs(LangOptions &O
> >>> Opts.NoMathBuiltin = Args.hasArg(OPT_fno_math_builtin);
> >>> Opts.AssumeSaneOperatorNew =
> !Args.hasArg(OPT_fno_assume_sane_operator_new);
> >>> Opts.SizedDeallocation |= Args.hasArg(OPT_fsized_deallocation);
> >>> + Opts.DefaultSizedDelete = Opts.SizedDeallocation &&
> >>> + Args.hasArg(OPT_fdef_sized_delete);
> >>> Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
> >>> Opts.AccessControl = !Args.hasArg(OPT_fno_access_control);
> >>> Opts.ElideConstructors = !Args.hasArg(OPT_fno_elide_constructors);
> >>>
> >>> Modified: cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp (original)
> >>> +++ cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp Fri Feb 13
> 23:42:57 2015
> >>> @@ -1,5 +1,7 @@
> >>> -// RUN: %clang_cc1 -std=c++1y %s -emit-llvm -triple x86_64-linux-gnu
> -o - | FileCheck %s
> >>> -// RUN: %clang_cc1 -std=c++11 -fsized-deallocation %s -emit-llvm
> -triple x86_64-linux-gnu -o - | FileCheck %s
> >>> +// RUN: %clang_cc1 -std=c++1y %s -emit-llvm -triple x86_64-linux-gnu
> -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKUND
> >>> +// RUN: %clang_cc1 -std=c++1y %s -emit-llvm -triple x86_64-linux-gnu
> -fdef-sized-delete -o - | FileCheck %s --check-prefix=CHECK
> --check-prefix=CHECKDEF
> >>> +// RUN: %clang_cc1 -std=c++11 -fsized-deallocation %s -emit-llvm
> -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK
> --check-prefix=CHECKUND
> >>> +// RUN: %clang_cc1 -std=c++11 -fsized-deallocation -fdef-sized-delete
> %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s
> --check-prefix=CHECK --check-prefix=CHECKDEF
> >>> // RUN: %clang_cc1 -std=c++11 %s -emit-llvm -triple x86_64-linux-gnu
> -o - | FileCheck %s --check-prefix=CHECK-UNSIZED
> >>>
> >>> // CHECK-UNSIZED-NOT: _ZdlPvm
> >>> @@ -50,8 +52,9 @@ D::D() {}
> >>> // CHECK: call void @_ZdlPvm(i8* %{{[^ ]*}}, i64 4)
> >>> // CHECK: call void @_ZdaPv(i8* %{{[^ ]*}})
> >>>
> >>> -// CHECK-LABEL: define linkonce void @_ZdlPvm(i8*
> >>> -// CHECK: call void @_ZdlPv(i8* %0)
> >>> +// CHECKDEF-LABEL: define linkonce void @_ZdlPvm(i8*
> >>> +// CHECKDEF: call void @_ZdlPv(i8* %0)
> >>> +// CHECKUND-LABEL: declare void @_ZdlPvm(i8*
> >>>
> >>> // CHECK-LABEL: define weak_odr void @_Z3delI1BEvv()
> >>> // CHECK: call void @_ZdlPvm(i8* %{{[^ ]*}}, i64 4)
> >>> @@ -71,8 +74,9 @@ D::D() {}
> >>> // CHECK: add i64 %{{[^ ]*}}, 8
> >>> // CHECK: call void @_ZdaPvm(i8* %{{[^ ]*}}, i64 %{{[^ ]*}})
> >>>
> >>> -// CHECK-LABEL: define linkonce void @_ZdaPvm(i8*
> >>> -// CHECK: call void @_ZdaPv(i8* %0)
> >>> +// CHECKDEF-LABEL: define linkonce void @_ZdaPvm(i8*
> >>> +// CHECKDEF: call void @_ZdaPv(i8* %0)
> >>> +// CHECKUND-LABEL: declare void @_ZdaPvm(i8*
> >>>
> >>> // CHECK-LABEL: define weak_odr void @_Z3delI1DEvv()
> >>> // CHECK: call void @_ZdlPvm(i8* %{{[^ ]*}}, i64 8)
> >>>
> >>> Modified: cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp?rev=229241&r1=229240&r2=229241&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp
> (original)
> >>> +++ cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp Fri
> Feb 13 23:42:57 2015
> >>> @@ -1,7 +1,9 @@
> >>> // RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++11 %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF
> -check-prefix=CHECK11
> >>> // RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++11 -fvisibility hidden %s 2>&1 | FileCheck %s
> -check-prefix=CHECKHID -check-prefix=CHECK11
> >>> -// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++14 %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF
> -check-prefix=CHECK14
> >>> -// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++14 -fvisibility hidden %s 2>&1 | FileCheck %s
> -check-prefix=CHECKHID -check-prefix=CHECK14
> >>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++14 %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF
> -check-prefix=CHECK14 -check-prefix=CHECK14UND
> >>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++14 -fvisibility hidden %s 2>&1 | FileCheck %s
> -check-prefix=CHECKHID -check-prefix=CHECK14 -check-prefix=CHECK14UND
> >>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++14 -fdef-sized-delete %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF
> -check-prefix=CHECK14 -check-prefix=CHECK14DEF
> >>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o -
> -std=c++14 -fdef-sized-delete -fvisibility hidden %s 2>&1 | FileCheck %s
> -check-prefix=CHECKHID -check-prefix=CHECK14 -check-prefix=CHECK14DEF
> >>>
> >>> // PR22419: Implicit sized deallocation functions always have default
> visibility.
> >>> // Generalized to all implicit allocation functions.
> >>> @@ -27,8 +29,9 @@ void foo(A* is) {
> >>> // CHECK11-DAG: declare void @_ZdlPv(i8*)
> >>>
> >>> // CHECK14-DAG: declare noalias i8* @_Znwm(i64)
> >>> -// CHECK14-DAG: define linkonce void @_ZdlPvm(i8*, i64)
> >>> -// CHECK14-DAG: declare void @_ZdlPv(i8*)
> >>> +// CHECK14UND-DAG: declare void @_ZdlPvm(i8*, i64)
> >>> +// CHECK14DEF-DAG: define linkonce void @_ZdlPvm(i8*, i64)
> >>> +// CHECK14DEF-DAG: declare void @_ZdlPv(i8*)
> >>>
> >>> // CHECK14-DAG: %struct.B = type { i8 }
> >>> struct B { ~B() { }};
> >>> @@ -50,5 +53,6 @@ void f(B *p) {
> >>> // CHECK11-DAG: declare void @_ZdaPv(i8*)
> >>>
> >>> // CHECK14-DAG: declare noalias i8* @_Znam(i64)
> >>> -// CHECK14-DAG: define linkonce void @_ZdaPvm(i8*, i64)
> >>> -// CHECK14-DAG: declare void @_ZdaPv(i8*)
> >>> +// CHECK14UND-DAG: declare void @_ZdaPvm(i8*, i64)
> >>> +// CHECK14DEF-DAG: define linkonce void @_ZdaPvm(i8*, i64)
> >>> +// CHECK14DEF-DAG: declare void @_ZdaPv(i8*)
> >>
> >>
> >> Please can you update cxx_status.html and the Clang 3.7 release notes
> to note that we now require one of (a) libc++ 3.7 or later, (b) libstdc++ 5
> or later, (c) -fdefine-sized-deallocation, or (d) -fno-sized-deallocation
> to build code in C++14 mode?
>
> Hi Richard -- did you mean -fno-sized-deallocation here?
>
I did, but it looks like we don't expose these flags in the driver yet.
We've had some more discussion of this, and have a new proposal for a way
forward, as follows. Clang should provide three modes for sized
deallocation:
-fsized-deallocation=never -- never call a sized deallocation function
-fsized-deallocation=check -- use a sized deallocation function if it's
available
-fsized-deallocation=always -- call a sized deallocation, assuming that
someone defines it
(names here may need more work).
If none of these is explicitly specified, C++14 onwards uses "check" and
all other modes use "never".
The "check" mode would generate code for a delete operator that checks
whether a sized deallocation function is available, and calls it if it is,
and calls the unsized deallocation function otherwise. We can implement
this by emitting an extern_weak declaration of the sized deallocation, with
each call site checking whether the declaration is null, and calling either
it or the unsized deallocation, or by using the existing code to generate a
weak definition if extern_weak linkage isn't available on our target.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150219/681aef0d/attachment.html>
More information about the cfe-commits
mailing list