aaron.ballman added inline comments.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2515
+ } else {
+ AI->addAttr(llvm::Attribute::NonNull);
+ }
----------------
rjmccall wrote:
> 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.
Ah, I see! Thank you for the explanation -- I wasn't aware that null could be a
valid address in other address spaces, but that makes sense.
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
