jyknight added inline comments.
================ Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double)))); +#ifdef __i386__ + __float128 __clang_max_align_nonce3 ---------------- EricWF wrote: > uweigand wrote: > > jyknight wrote: > > > uweigand wrote: > > > > jyknight wrote: > > > > > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in > > > > > InitPreprocessor alongside the rest of the SIZEOF macros? > > > > > > > > > > And then use that to determine whether to add float128 to the union? > > > > > This change, as is, will break on any target which is i386 but > > > > > doesn't define __float128, e.g. i386-unknown-unknown. > > > > > > > > > > The only additional target which will be modified with that (that is: > > > > > the only other target which has a float128 type, but for which > > > > > max_align isn't already 16) is systemz-*-linux. > > > > > > > > > > But I think that's actually indicating a pre-existing bug in the > > > > > SystemZ config -- it's configured for a 16-byte long double, with > > > > > 8-byte alignment, but the ABI doc seems to call for 16-byte > > > > > alignment. +Ulrich for comment on that. > > > > That's a bug in the ABI doc which we'll fix once we get around to > > > > releasing an updated version. > > > > > > > > long double on SystemZ must be 8-byte aligned, which is the maximum > > > > alignment of all standard types on Z, and this was chosen because > > > > historically the ABI only guarantees an 8-byte stack alignment, so > > > > 16-byte aligned standard types would be awkward. > > > Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with > > > `-target systemz-linux`? > > Huh, really __float128 should not be supported at all on SystemZ. It's not > > supported with GCC either (GCC never provides __float128 on targets where > > long double is already IEEE-128). (GCC does support the new _Float128 on > > SystemZ, but that's not yet supported by clang anywhere.) > > > > If it were supported, I agree it should be an alias for long double, and > > also have an alignof of 8. > @jyknight Ack. I'll add `__SIZEOF_FLOAT128__`. @uweigand : One of those fixes needs to land before this, so that systemz's max_align_t doesn't change to 16 in the meantime. I think your preference would be for it to be simply removed, right? Looks like the type was originally added in https://reviews.llvm.org/D19125 -- possibly in error, since the focus was x86_64. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55057/new/ https://reviews.llvm.org/D55057 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits