[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:
> > > > > 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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116816