[PATCH] D116816: [instcombine] Delete duplicate object size logic

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2578
+  if (getObjectSize(&Call, Size, DL, TLI, Opts) && Size > 0) {
+    // TODO: should be annotating these nonnull
     if (isOpNewLikeFn(&Call, TLI))
----------------
xbolva00 wrote:
> reames wrote:
> > xbolva00 wrote:
> > > reames wrote:
> > > > xbolva00 wrote:
> > > > > reames wrote:
> > > > > > xbolva00 wrote:
> > > > > > > all allocation fns have nonnull attribute, I believe.
> > > > > > Er, definitely not.  Malloc can return nullptr on error.  Did you maybe mean that all operator new versions did?  I couldn't find that code, can you point me to it?
> > > > > Yeah right, just for new calls.
> > > > > 
> > > > > Yes, todo is correct here for others
> > > > Your wording in last comment is unclear to me, can you rephrase?  
> > > That for throwing version of 'new' we already have nonnull.
> > > 
> > > I am wondering if just “deref(N)” attribute alone is enough for other parts of LLVM to assume also “nonnull” (for addr space 0)
> > Can you point me to the code which infers nonnull for the throwing operator new variants?  I did not find it on a quick skim.  In particular, I do not see this in BuildLibCalls.cpp which I would expect.   
> > 
> > (Regardless, I'd like to leave this cleanup to a separate patch.)
> https://clang.llvm.org/doxygen/SemaDecl_8cpp_source.html#l15072
Ah, I was looking in LLVM proper.  Thanks!

Will cleanup the tests and remove this in a separate commit.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116816