dnsampaio added a comment.
From my point it does LGTM.
================
Comment at: clang/lib/CodeGen/CGExprCXX.cpp:2042-2049
// Null check the pointer.
llvm::BasicBlock *DeleteNotNull = createBasicBlock("delete.notnull");
llvm::BasicBlock *DeleteEnd = createBasicBlock("delete.end");
llvm::Value *IsNull = Builder.CreateIsNull(Ptr.getPointer(), "isnull");
Builder.CreateCondBr(IsNull, DeleteEnd, DeleteNotNull);
----------------
rsmith wrote:
> dnsampaio wrote:
> > Unless I missed something, isn't it better to just avoid emitting this
> > check and basic blocks all together if we are optimizing for size and when
> > we know that Ptr is never null?
> > I would consider in doing something alike:
> > ```
> > const bool emitNullCheck = CGM.getCodeGenOpts().OptimizeSize <= 1;
> > llvm::BasicBlock *DeleteNotNull;
> > llvm::BasicBlock *DeleteEnd;
> > if (emitNullCheck){
> > // Null check the pointer.
> > DeleteNotNull = createBasicBlock("delete.notnull");
> > DeleteEnd = createBasicBlock("delete.end");
> >
> > llvm::Value *IsNull = Builder.CreateIsNull(Ptr.getPointer(), "isnull");
> >
> > Builder.CreateCondBr(IsNull, DeleteEnd, DeleteNotNull);
> > EmitBlock(DeleteNotNull);
> > }
> > ```
> >
> > and we use the same emitNullCheck to avoid EmitBlocks below.
> I don't think we can reasonably do this. There are a lot of different ways
> that `delete` emission can be performed, and many of them (for example,
> calling a virtual deleting destructor, destroying operator delete, or array
> delete with cookie) require the null check to be performed in advance for
> correctness. It would be brittle to duplicate all of those checks here.
>
> We *could* sink the null checks into the various paths through
> `EmitArrayDelete` and `EmitObjectDelete`, but I think that makes the code
> significantly more poorly factored.
Fair enough.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79378/new/
https://reviews.llvm.org/D79378
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
