ahatanak added inline comments.

================
Comment at: clang/lib/CodeGen/Address.h:65
       : Pointer(Pointer), ElementType(ElementType) {
     if (Alignment.isZero())
       return;
----------------
efriedma wrote:
> Do you need to do something with IsKnownNonNull in the `Alignment.isZero()` 
> case?
We can probably get away with not doing anything here because currently the 
alignment is zero only when `Address::invalid` is called.


================
Comment at: clang/lib/CodeGen/Address.h:67
       return;
-    // Currently the max supported alignment is much less than 1 << 63 and is
+    // Currently the max supported alignment is much less than 1 << 32 and is
     // guaranteed to be a power of 2, so we can store the log of the alignment
----------------
efriedma wrote:
> This comment isn't right.  The max alignment is, as far as I can tell, 1<<32 
> exactly.  (But there's something weird going on with very large values... 
> somehow `int a[1LL<<32] __attribute((aligned(1ULL<<32))) = {};` ignores the 
> alignment.)
The following function generated by tablegen (and a few others directly or 
indirectly calling the function) returns a 32-bit int, but it should be 
returning a 64-bit int.

https://github.com/llvm/llvm-project/blob/main/clang/utils/TableGen/ClangAttrEmitter.cpp#L532


================
Comment at: clang/lib/CodeGen/CGBuilder.h:164
+    return Addr.withPointer(CreateAddrSpaceCast(Addr.getPointer(), Ty, Name),
+                            Addr.isKnownNonNull());
   }
----------------
efriedma wrote:
> Do address-space casts preserve non-null?
I think we can preserve non-null here. We just can't assume that `KnownNonNull` 
indicates the pointer is non-zero when the address space isn't the default 
address space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142584

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

Reply via email to