[Builtins] Fix calling long double math functions on x86_64 mingw

[mstorsjo]

Summary

On x86_64 mingw, long doubles are passed as arguments and returned as if they were a struct. For normal functions, this calling convention detail is handled within clang when generating the IR. When a math function is handled as a builtin, it gets emitted as an LLVM intrinsic call, which for x86_64 mingw in most (all?) cases gets emitted as a libcall. When the libcall is lowered in llvm, the long double specifics of the calling convention aren't taken into account (since they're normally taken care of in clang).

Skip emitting intrinsics for the functions that handle long doubles for this target.

While use of long double might be quite rare in general, it's used internally in the implementation of some mingw math functions for other precisions than long double.

[efriedma]

Can we just fix the bug in the backend, rather than trying to hack around it in clang?

[rnk]

lgtm

[mstorsjo]

In D44582#1040578, @efriedma wrote:

Can we just fix the bug in the backend, rather than trying to hack around it in clang?

That'd obviously be ideal - but I don't know how easy that'd be (not familiar enough with that aspect of LLVM); if it's easily doable there, why is the handling of this detail of the calling convention done in clang for normal functions to begin with? (The calling convention requires allocating stack space in the caller for the argument and return value, which isn't done in the IR emitted by clang originally when using intrinsics.)

[efriedma]

The backend has code to generate SRet returns, which is used when TargetLowering::CanLowerReturn returns false. Probably a tiny change to X86CallingConv.td to use that codepath on Windows.

[mstorsjo]

In D44582#1040643, @efriedma wrote:

The backend has code to generate SRet returns, which is used when TargetLowering::CanLowerReturn returns false. Probably a tiny change to X86CallingConv.td to use that codepath on Windows.

Thanks, I managed to make something that seems to work by tweaking that file, see D44592.

[mstorsjo]

Fixed the issue via D44592 instead, thanks @efriedma for the pointers!

lib/CodeGen/CGBuiltin.cpp

if (E->EvaluateAsRValue(Result, CGM.getContext()) &&
     if (Result.Val.isInt())
       return RValue::get(llvm::ConstantInt::get(getLLVMContext(),
                                                 Result.Val.getInt()));
     if (Result.Val.isFloat())
       return RValue::get(llvm::ConstantFP::get(getLLVMContext(),
                                                Result.Val.getFloat()));
   }
 
+  bool EmitMathIntrinsic = true;
+  const Triple& Triple = getTarget().getTriple();
+  if (Triple.isWindowsGNUEnvironment() &&
+      Triple.getArch() == llvm::Triple::x86_64) {
+    // On x86_64 mingw, the calling convention handles long doubles as
+    // structs. This detail is normally handled within clang, but
+    // intrinsics lowered into libcalls in LLVM don't handle this.
+    // For these cases, skip emitting an intrinsic and emit a normal
+    // function call with the right calling convention instead.
+    const char *TypeStr = getContext().BuiltinInfo.getTypeString(BuiltinID);
+    if (strstr(TypeStr, "Ld")) // long double
+      EmitMathIntrinsic = false;
+  }
+
   // There are LLVM math intrinsics/instructions corresponding to math library
   // functions except the LLVM op will never set errno while the math library
   // might. Also, math builtins have the same semantics as their math library
   // twins. Thus, we can transform math library and builtin calls to their
   // LLVM counterparts if the call is marked 'const' (known to never set errno).
-  if (FD->hasAttr<ConstAttr>()) {
+  if (FD->hasAttr<ConstAttr>() && EmitMathIntrinsic) {
     switch (BuiltinID) {
     case Builtin::BIceil:
     case Builtin::BIceilf:
     case Builtin::BIceill:
     case Builtin::BI__builtin_ceil:
     case Builtin::BI__builtin_ceilf:
     case Builtin::BI__builtin_ceill:
       return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::ceil));

test/CodeGen/mingw-x86_64-long-double-builtins.c

+// RUN: %clang_cc1 -triple x86_64-mingw32 -emit-llvm -o - %s | FileCheck %s
+
+// Check that long double math functions are emitted as direct function
+// calls instead of as intrinsics.
+
+long double floorl(long double);
+
+long double foo(long double a) {
+  return floorl(a);
+// CHECK: call void @floorl(x86_fp80* sret {{.*}}, x86_fp80* {{.*}})
+}