[PATCH] D117356: InstructionCombining: avoid eliding mismatched alloc/free pairs
James Y Knight via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 20 06:07:27 PST 2022
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
More information about the llvm-commits
mailing list