uweigand marked an inline comment as done.
uweigand 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
----------------
uweigand wrote:
> jyknight wrote:
> > 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.
> @jyknight : Yes, this seems to have been simply an error. I'll check in a
> patch to remove `__float128` on SystemZ.
>
Checked in as rev. 348247.
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
