[PATCH] D117356: InstructionCombining: avoid eliding mismatched alloc/free pairs

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

In D117356#3256460 <https://reviews.llvm.org/D117356#3256460>, @ychen wrote:

> Apologies if I didn't make myself clear.  The inlining of the allocator function step is already incorrect. This instcombine bailout is not a desirable direction since it implicitly assumes the earlier allocator inlining is correct but it is not. Whatever attributes would be added in the future should be used for preventing the C++ new/delete from doing any kinds of IPO/IPA (marking them to be deleted is allowed though).

Please clarify. Inlining c++ operator new seems like a desirable optimization, which (modulo bugs) should be correct -- not something that ought to be prohibited.

However, if that's going to get into a long debate, I also believe this patch is correct and desirable _regardless_ of the C++ side of things. Fixing C++ operator new handling was not the primary goal of this work -- although it seemed like a nice additional benefit.

The most important thing here is setting the stage for the future planned work to create IR attributes to allow arbitrary user-defined functions to be given the same behavior of allowing alloc+free pairs to be removed. The same problem of ensuring the alloc/free are from the same family applies just as well to such allocators if their implementation devolves to another allocator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117356