[llvm] r179071 - c++ new operators are not malloc-like functions because they do not return uninitialized memory.

Mon Apr 8 19:02:13 PDT 2013

On Mon, Apr 8, 2013 at 4:40 PM, Nadav Rotem <nrotem at apple.com> wrote:

> Author: nadav
> Date: Mon Apr  8 18:40:47 2013
> New Revision: 179071
>
> URL: http://llvm.org/viewvc/llvm-project?rev=179071&view=rev
> Log:
> c++ new operators are not malloc-like functions because they do not return
> uninitialized memory.
> Users may overide new-operators and implement any function that they like.
>

Nadav, I don't get it. This behavior has been in LLVM for quite some time.
It was discussed reasonably extensively at the time it went in if my memory
serves, and it was a very conscious decision to do this. Also, the
malloc-like behavior has much more to do with aliasing properties than
whether the memory is uninitialized... These seem almost entirely
orthogonal concerns.

Either way, maybe that decision was wrong, but I don't think this commit
log sums up all there is to say on the matter, and it seems really rude to
just switch behavior (which there were tests in place to exercise!) without
actually holding a discussion with the relevant parties and trying to
understand the original motivation and why things worked this way.

Can you revert this, and start such a discussion? As part of it, it would
be good to give pointers to the original discussion and some of the
motivating use cases which mean this needs to change. Also, why were other
options not considered? For example, in the original discussion an
'-fsane-operator-new' flag was discussed to allow clients that had need of
non-malloc-like behavior from their global operator new to achieve that.

>
>
> Added:
>     llvm/trunk/test/Transforms/GVN/newoperator.ll
> Modified:
>     llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>     llvm/trunk/test/Transforms/InstCombine/invoke.ll
>     llvm/trunk/test/Transforms/InstCombine/objsize-64.ll
>
> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=179071&r1=179070&r2=179071&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Mon Apr  8 18:40:47 2013
> @@ -52,14 +52,6 @@ struct AllocFnsTy {
>  static const AllocFnsTy AllocationFnData[] = {
>    {LibFunc::malloc,              MallocLike,  1, 0,  -1},
>    {LibFunc::valloc,              MallocLike,  1, 0,  -1},
> -  {LibFunc::Znwj,                MallocLike,  1, 0,  -1}, // new(unsigned
> int)
> -  {LibFunc::ZnwjRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, // new(unsigned
> int, nothrow)
> -  {LibFunc::Znwm,                MallocLike,  1, 0,  -1}, // new(unsigned
> long)
> -  {LibFunc::ZnwmRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, // new(unsigned
> long, nothrow)
> -  {LibFunc::Znaj,                MallocLike,  1, 0,  -1}, //
> new[](unsigned int)
> -  {LibFunc::ZnajRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, //
> new[](unsigned int, nothrow)
> -  {LibFunc::Znam,                MallocLike,  1, 0,  -1}, //
> new[](unsigned long)
> -  {LibFunc::ZnamRKSt9nothrow_t,  MallocLike,  2, 0,  -1}, //
> new[](unsigned long, nothrow)
>    {LibFunc::posix_memalign,      MallocLike,  3, 2,  -1},
>    {LibFunc::calloc,              CallocLike,  2, 0,   1},
>    {LibFunc::realloc,             ReallocLike, 2, 1,  -1},
>
> Added: llvm/trunk/test/Transforms/GVN/newoperator.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/newoperator.ll?rev=179071&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/GVN/newoperator.ll (added)
> +++ llvm/trunk/test/Transforms/GVN/newoperator.ll Mon Apr  8 18:40:47 2013
> @@ -0,0 +1,20 @@
> +; RUN: opt < %s -basicaa -gvn -S | FileCheck %s
> +
> +; We can't remove the load because new operators are overideable and can
> return non-undefined memory.
> +;CHECK: main
> +;CHECK: load
> +;CHECK: ret
> +define i32 @main(i32 %argc, i8** nocapture %argv) ssp uwtable {
> +  %1 = tail call noalias i8* @_Znam(i64 800)
> +  %2 = bitcast i8* %1 to i32**
> +  %3 = load i32** %2, align 8, !tbaa !0
> +  %4 = icmp eq i32* %3, null
> +  %5 = zext i1 %4 to i32
> +  ret i32 %5
> +}
> +
> +declare noalias i8* @_Znam(i64)
> +
> +!0 = metadata !{metadata !"any pointer", metadata !1}
> +!1 = metadata !{metadata !"omnipotent char", metadata !2}
> +!2 = metadata !{metadata !"Simple C/C++ TBAA"}
>
> Modified: llvm/trunk/test/Transforms/InstCombine/invoke.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/invoke.ll?rev=179071&r1=179070&r2=179071&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/invoke.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/invoke.ll Mon Apr  8 18:40:47
> 2013
> @@ -47,19 +47,3 @@ lpad:
>    unreachable
>  }
>
> -; CHECK: @f3
> -define void @f3() nounwind uwtable ssp {
> -; CHECK: invoke void @llvm.donothing()
> -  %call = invoke noalias i8* @_Znwm(i64 13)
> -          to label %invoke.cont unwind label %lpad
> -
> -invoke.cont:
> -  ret void
> -
> -lpad:
> -  %1 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)*
> @__gxx_personality_v0 to i8*)
> -          filter [0 x i8*] zeroinitializer
> -  %2 = extractvalue { i8*, i32 } %1, 0
> -  tail call void @__cxa_call_unexpected(i8* %2) noreturn nounwind
> -  unreachable
> -}
>
> Modified: llvm/trunk/test/Transforms/InstCombine/objsize-64.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/objsize-64.ll?rev=179071&r1=179070&r2=179071&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/objsize-64.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/objsize-64.ll Mon Apr  8
> 18:40:47 2013
> @@ -25,7 +25,7 @@ entry:
>            to label %invoke.cont unwind label %lpad
>
>  invoke.cont:
> -; CHECK: ret i64 13
> +; CHECK: ret i64 %0
>    store i8* %call, i8** %esc
>    %0 = tail call i64 @llvm.objectsize.i64(i8* %call, i1 false)
>    ret i64 %0
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/7f39995c/attachment.html>