[PATCH] D79378: PR34581: Don't remove an 'if (p)' guarding a call to 'operator delete(p)' under -Oz.

[Diogo N. Sampaio via Phabricator via llvm-commits]

dnsampaio added a comment.

I believe we can avoid creating some blocks for latter removing them, no?



================
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);
----------------
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.


================
Comment at: clang/lib/CodeGen/CGExprCXX.cpp:2090
   } else {
-    EmitObjectDelete(*this, E, Ptr, DeleteTy);
+    if (!EmitObjectDelete(*this, E, Ptr, DeleteTy, DeleteEnd))
+      EmitBlock(DeleteEnd);
----------------
If avoiding the emission above it would not require changing `EmitObjectDelete` at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79378/new/

https://reviews.llvm.org/D79378