rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2515
+ } else {
+ AI->addAttr(llvm::Attribute::NonNull);
+ }
----------------
aaron.ballman wrote:
> rjmccall wrote:
> > Isn't the old logic still correct? If the element size is static and the
> > element count is positive, the argument is dereferenceable out to their
> > product; otherwise it's nonnull if null is the zero value and we aren't
> > semantically allowing that to be a valid pointer.
> I was questioning this -- I didn't think the old logic was correct because it
> checks that the array is in address space 0, but the nonnull-ness should
> apply regardless of address space (I think). The point about valid null
> pointers still stands, though. Am I misunderstanding the intended address
> space behavior?
I believe LLVM's `nonnull` actually always means nonzero. `static` just tells
us that the address is valid, so (1) we always have to suppress the attribute
under `NullPointerIsValid` and (2) we have the suppress the attribute if the
null address is nonzero, because the zero address could still be valid in that
case. The way the existing code is implementing the latter seems excessively
conservative about non-standard address spaces, since we might know that they
still use a zero null pointer; more importantly, it seems wrong in the face of
an address space that lowers to LLVM's address space 0 but doesn't use a zero
null pointer. You can call
`getTargetInfo().getNullPointerValue(ETy.getAddressSpace()) == 0` to answer
this more correctly.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83502/new/
https://reviews.llvm.org/D83502
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
