[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2174
+  }
+
   unsigned ArgNo = 0;
----------------
rsmith wrote:
> CJ-Johnson wrote:
> > jdoerfert wrote:
> > > Even if null pointer is valid we should place dereferenceable.
> > > 
> > > We also could never place nonnull and let the middle-end make the dereferenceable -> nonnull deduction, though I don't see why we can't just add nonnull here.
> > I re-ran ninja check after making this fix and addressed the newly-affected tests. So the patch is fully up to date :)
> The LLVM LangRef says that in address space 0, `dereferenceable` implies `nonnull`, so I don't think we can emit `dereferenceable` in `NullPointerIsValid` mode, and we'd need to use `dereferenceable_or_null` instead. (Perhaps the LangRef is wrong, though, and the `null_pointer_is_valid` function attribute overrides that determination.)
> (Perhaps the LangRef is wrong, though, and the null_pointer_is_valid function attribute overrides that determination.)

This is the case. IMHO, dereferenceable has to be correct here, regardless of the mode. You could access the object in the function entry, which we would use to deduce dereferenceable, and if the `NullPointerIsValid` is not translated to the function attribute also to `nonnull`. 


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

https://reviews.llvm.org/D17993