efriedma added inline comments.
================ 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 ---------------- ahatanak wrote: > efriedma wrote: > > ahatanak wrote: > > > 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 > > Filed https://github.com/llvm/llvm-project/issues/60752 so we don't lose > > track of this. > I just realized we can't reduce the number of bits used for alignment here as > we need 6 bits for alignment of `1 << 32`. > > Should we allocate additional memory when `AlignLog` is either 31 or 32? If > the 5-bit alignment is equal to `0b11111`, it would mean that there is an > out-of-line storage large enough to hold the alignment and any other extra > information that is needed. I think https://reviews.llvm.org/D117262#3267899 > proposes a similar idea. How much does `sizeof(Address)` actually matter, anyway? If it's going to get that nasty to implement the packing, I'm not sure it's worth the effort to optimize. 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