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

2023-02-05 Thread Shiva Chen via Phabricator via cfe-commits
shiva0217 added a comment. In D79378#4103366 , @rjmccall wrote: > In D79378#4101935 , @shiva0217 wrote: > >> In D79378#4101829 , @rjmccall wrote: >> >>> In D79378#4101613

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

2023-02-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D79378#4101935 , @shiva0217 wrote: > In D79378#4101829 , @rjmccall wrote: > >> In D79378#4101613 , @shiva0217 >> wrote: >> >>> Hi, >>> >>> I hav

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

2023-02-03 Thread Shiva Chen via Phabricator via cfe-commits
shiva0217 added a comment. In D79378#4101829 , @rjmccall wrote: > In D79378#4101613 , @shiva0217 wrote: > >> Hi, >> >> I have a question for the delete function call sinking in -Oz. >> >> https://www.open-std.org/jt

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

2023-02-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D79378#4101613 , @shiva0217 wrote: > Hi, > > I have a question for the delete function call sinking in -Oz. > > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3690.pdf. > According to 3.7.4.2/3 > ` The value of the f

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

2023-02-02 Thread Shiva Chen via Phabricator via cfe-commits
shiva0217 added a comment. Herald added a project: All. Hi, I have a question for the delete function call sinking in -Oz. https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3690.pdf. According to 3.7.4.2/3 ` The value of the first argument supplied to a deallocation function may be a nu

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

2020-06-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf39e12a06b60: PR34581: Don't remove an 'if (p)' guarding a call to 'operator delete(p)' under… (authored by rsmith). Changed prior to commit: https://reviews.llvm.org/D79378?vs=261963&id=268971#toc Re

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

2020-06-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D79378#2058753 , @dnsampaio wrote: > Hi @rsmith, > are you still looking into this? > cheers Sorry for the delay, I'll be getting back to this soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

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

2020-05-27 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio added a comment. Hi @rsmith, are you still looking into this? cheers 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-

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

2020-05-14 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio accepted this revision. dnsampaio added a comment. This revision is now accepted and ready to land. LGTM, as far @rjmccall 's concern about documentation is addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79378/new/ https://revie

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

2020-05-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D79378#2028705 , @rsmith wrote: > In D79378#2019336 , @rjmccall wrote: > > > Is it reasonable to figure out ahead of time that we can skip the null > > check completely? It'd be kindof

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

2020-05-11 Thread Diogo N. Sampaio via Phabricator via cfe-commits
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");

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

2020-05-10 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done. rsmith added a comment. In D79378#2019336 , @rjmccall wrote: > Is it reasonable to figure out ahead of time that we can skip the null check > completely? It'd be kindof nice to also take advantage of this at -O0

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

2020-05-05 Thread Diogo N. Sampaio via Phabricator via cfe-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

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

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Is it reasonable to figure out ahead of time that we can skip the null check completely? It'd be kindof nice to also take advantage of this at -O0 whenever we don't have real work to do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added reviewers: davide, dnsampaio, rjmccall. Herald added a subscriber: hiraditya. Herald added projects: clang, LLVM. This transformation is correct for a builtin call to 'free(p)', but not for 'operator delete(p)'. There is no guarantee that a user replaceme