On Wed, Oct 31, 2018 at 7:39 AM <paul.robin...@sony.com> wrote: > Also, don't we usually put ABI changes under an ABI compatibility check? > This would be making Clang incompatible with itself. >
I don't think it's worth it because nobody can use i128 in a meaningful way on x64 Windows right now. If you do i128 multiplication or division, LLVM will generate calls to the *ti* (for "tetrainteger") runtime functions in compiler-rt. Without this change, clang compiles those *ti* functions with a different calling convention from the one that LLVM uses, because LLVM is trying to match libgcc's calling conventions. Clang is in a situation where all language extensions are supported by default, all the time, for all targets, whether or not anyone has thought about ABI stability for that particular feature and target combination. I don't think it's really tenable to promise ABI stability for every feature on every platform, ever. We have to give ourselves the flexibility to implement one feature (e.g. i128) generically for all targets, and then fill in the target specific blanks later. For example, I have no idea about the status of OpenMP or ObjC on Windows right now, and I certainly hope that we do not promise ABI stability with whatever our current emergent behavior is. > *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On > Behalf Of *Richard Trieu via cfe-commits > *Sent:* Tuesday, October 30, 2018 10:16 PM > *To:* Reid Kleckner > *Cc:* cfe-commits > *Subject:* Re: r345676 - [Win64] Handle passing i128 by value > > > > I have reverted this in r345691 because it caused test > CodeGen/mingw-long-double.c to start failing. > Thanks! > + case BuiltinType::LongDouble: > + // Mingw64 GCC uses the old 80 bit extended precision floating point > + // unit. It passes them indirectly through memory. > + if (IsMingw64) { > + const llvm::fltSemantics *LDF = > &getTarget().getLongDoubleFormat(); > + if (LDF == &llvm::APFloat::x87DoubleExtended()) > + return ABIArgInfo::getIndirect(Align, /*ByVal=*/false); > + break; > + } > > The bug is that I rewrote my patch at the last minute to use a switch here, and I forgot the break. -Wimplicit-fallthrough would've caught this. I can't find a bug for enabling this on our codebase, but I know there have been cleanup efforts in the past. > + case BuiltinType::Int128: > + case BuiltinType::UInt128: > + // If it's a parameter type, the normal ABI rule is that arguments > larger > + // than 8 bytes are passed indirectly. GCC follows it. We follow it > too, > + // even though it isn't particularly efficient. > + if (!IsReturnType) > + return ABIArgInfo::getIndirect(Align, /*ByVal=*/false); > + > + // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that. > + // Clang matches them for compatibility. > + return ABIArgInfo::getDirect( > + llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), > 2)); > + > + default: > + break; > + } > } > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
