r345676 - [Win64] Handle passing i128 by value

[Reid Kleckner via cfe-commits]

On Wed, Oct 31, 2018 at 7:39 AM <paul.robinson at 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-bounces at 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;
> +    }
>    }
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181031/f68e66b9/attachment.html>