rsmith added inline comments.

================
Comment at: clang/lib/CodeGen/CGCall.cpp:2169
+    if (!CodeGenOpts.NullPointerIsValid &&
+        getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) {
+      Attrs.addAttribute(llvm::Attribute::NonNull);
----------------
jdoerfert wrote:
> arichardson wrote:
> > Isn't the `this` pointer also nonnull in other address spaces?
> > 
> > In our CHERI fork we use AS200 for the this pointer and would quite like to 
> > have the nonnull attribute.
> > I can obviously change this line locally when I next merge from upstream, 
> > but I would like to avoid diffs and it seems to me like this restriction is 
> > unnecessary.
> I also think `NullPointerIsValid` is sufficient. 
It's my understanding that:
* The LLVM `null` value in any address space is the all-zero-bits value.
* In address space zero, the `null` value does not correspond to addressable 
memory, but this is not assumed to hold in other address spaces.
* An address-space-zero `null` value that is addressspacecast to a different 
address space might not be the `null` in the target address space.
* The `nonnull` attribute implies that the pointer value is not the `null` 
value.
* A null pointer in the frontend in a non-zero address space corresponds to the 
value produced by an addressspacecast of an address-space-zero `null` value to 
the target address space.

That being the case, there is simply no connection between the C and C++ notion 
of a null pointer and a `null` LLVM pointer value in a non-zero address space 
in general, so it is not correct to use the `nonnull` attribute in a non-zero 
address space in general. Only if we know that a C++ null pointer is actually 
represented by the LLVM `null` value in the corresponding address space can we 
use the `nonnull` attribute to expose that fact to LLVM. And we do not know 
that in general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17993

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to