[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e19ba36fca9: [X86][MS] Add 80bit long double support for 
Windows (authored by pengfei).

Changed prior to commit:
  https://reviews.llvm.org/D115441?vs=395129=408328#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/X86/long-double-config-size.c


Index: clang/test/CodeGen/X86/long-double-config-size.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/long-double-config-size.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-64 
-o - | FileCheck %s --check-prefix=SIZE64
+// RUN: %clang_cc1 -triple i386-windows-msvc %s -emit-llvm -mlong-double-80 -o 
- | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-80 
-o - | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-128 
-o - | FileCheck %s --check-prefix=SIZE128
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -o - | FileCheck 
%s --check-prefix=SIZE64
+
+long double global;
+// SIZE64: @global = dso_local global double 0
+// SIZE80: @global = dso_local global x86_fp80 0xK{{0+}}, align 16
+// SIZE128: @global = dso_local global fp128 0
+
+long double func(long double param) {
+  // SIZE64: define dso_local double @func(double noundef %param)
+  // SIZE80: define dso_local x86_fp80 @func(x86_fp80 noundef %param)
+  // SIZE128: define dso_local fp128  @func(fp128 noundef %param)
+  long double local = param;
+  // SIZE64: alloca double
+  // SIZE80: alloca x86_fp80, align 16
+  // SIZE128: alloca fp128
+  local = param;
+  return local + param;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3456,6 +3456,8 @@
 GenerateArg(Args, OPT_mlong_double_128, SA);
   else if (Opts.LongDoubleSize == 64)
 GenerateArg(Args, OPT_mlong_double_64, SA);
+  else if (Opts.LongDoubleSize == 80)
+GenerateArg(Args, OPT_mlong_double_80, SA);
 
   // Not generating '-mrtd', it's just an alias for '-fdefault-calling-conv='.
 
@@ -3838,9 +3840,16 @@
   Opts.NoBuiltin = Args.hasArg(OPT_fno_builtin) || Opts.Freestanding;
   if (!Opts.NoBuiltin)
 getAllNoBuiltinFuncValues(Args, Opts.NoBuiltinFuncs);
-  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
-? 128
-: Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
+  if (Arg *A = Args.getLastArg(options::OPT_LongDouble_Group)) {
+if (A->getOption().matches(options::OPT_mlong_double_64))
+  Opts.LongDoubleSize = 64;
+else if (A->getOption().matches(options::OPT_mlong_double_80))
+  Opts.LongDoubleSize = 80;
+else if (A->getOption().matches(options::OPT_mlong_double_128))
+  Opts.LongDoubleSize = 128;
+else
+  Opts.LongDoubleSize = 0;
+  }
   if (Opts.FastRelaxedMath)
 Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
   llvm::sort(Opts.ModuleFeatures);
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -449,6 +449,20 @@
 } else if (Opts.LongDoubleSize == 128) {
   LongDoubleWidth = LongDoubleAlign = 128;
   LongDoubleFormat = ::APFloat::IEEEquad();
+} else if (Opts.LongDoubleSize == 80) {
+  LongDoubleFormat = ::APFloat::x87DoubleExtended();
+  if (getTriple().isWindowsMSVCEnvironment()) {
+LongDoubleWidth = 128;
+LongDoubleAlign = 128;
+  } else { // Linux
+if (getTriple().getArch() == llvm::Triple::x86) {
+  LongDoubleWidth = 96;
+  LongDoubleAlign = 32;
+} else {
+  LongDoubleWidth = 128;
+  LongDoubleAlign = 128;
+}
+  }
 }
   }
 


Index: clang/test/CodeGen/X86/long-double-config-size.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/long-double-config-size.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-64 -o - | FileCheck %s --check-prefix=SIZE64
+// RUN: %clang_cc1 -triple i386-windows-msvc %s -emit-llvm -mlong-double-80 -o - | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-80 -o - | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-128 -o - | FileCheck %s --check-prefix=SIZE128
+// RUN: %clang_cc1 -triple 

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

lgtm

I believe you can go ahead and land this, it doesn't depend on the data layout 
changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 395129.
pengfei added a comment.

Split the LLVM datalayout to a different patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/X86/long-double-config-size.c


Index: clang/test/CodeGen/X86/long-double-config-size.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/long-double-config-size.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-64 
-o - | FileCheck %s --check-prefix=SIZE64
+// RUN: %clang_cc1 -triple i386-windows-msvc %s -emit-llvm -mlong-double-80 -o 
- | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-80 
-o - | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-128 
-o - | FileCheck %s --check-prefix=SIZE128
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -o - | FileCheck 
%s --check-prefix=SIZE64
+
+long double global;
+// SIZE64: @global = dso_local global double 0
+// SIZE80: @global = dso_local global x86_fp80 0xK{{0+}}, align 16
+// SIZE128: @global = dso_local global fp128 0
+
+long double func(long double param) {
+  // SIZE64: define dso_local double @func(double %param)
+  // SIZE80: define dso_local x86_fp80 @func(x86_fp80 %param)
+  // SIZE128: define dso_local fp128  @func(fp128 %param)
+  long double local = param;
+  // SIZE64: alloca double
+  // SIZE80: alloca x86_fp80, align 16
+  // SIZE128: alloca fp128
+  local = param;
+  return local + param;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3454,6 +3454,8 @@
 GenerateArg(Args, OPT_mlong_double_128, SA);
   else if (Opts.LongDoubleSize == 64)
 GenerateArg(Args, OPT_mlong_double_64, SA);
+  else if (Opts.LongDoubleSize == 80)
+GenerateArg(Args, OPT_mlong_double_80, SA);
 
   // Not generating '-mrtd', it's just an alias for '-fdefault-calling-conv='.
 
@@ -3837,9 +3839,16 @@
   Opts.NoBuiltin = Args.hasArg(OPT_fno_builtin) || Opts.Freestanding;
   if (!Opts.NoBuiltin)
 getAllNoBuiltinFuncValues(Args, Opts.NoBuiltinFuncs);
-  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
-? 128
-: Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
+  if (Arg *A = Args.getLastArg(options::OPT_LongDouble_Group)) {
+if (A->getOption().matches(options::OPT_mlong_double_64))
+  Opts.LongDoubleSize = 64;
+else if (A->getOption().matches(options::OPT_mlong_double_80))
+  Opts.LongDoubleSize = 80;
+else if (A->getOption().matches(options::OPT_mlong_double_128))
+  Opts.LongDoubleSize = 128;
+else
+  Opts.LongDoubleSize = 0;
+  }
   if (Opts.FastRelaxedMath)
 Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
   llvm::sort(Opts.ModuleFeatures);
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -446,6 +446,20 @@
 } else if (Opts.LongDoubleSize == 128) {
   LongDoubleWidth = LongDoubleAlign = 128;
   LongDoubleFormat = ::APFloat::IEEEquad();
+} else if (Opts.LongDoubleSize == 80) {
+  LongDoubleFormat = ::APFloat::x87DoubleExtended();
+  if (getTriple().isWindowsMSVCEnvironment()) {
+LongDoubleWidth = 128;
+LongDoubleAlign = 128;
+  } else { // Linux
+if (getTriple().getArch() == llvm::Triple::x86) {
+  LongDoubleWidth = 96;
+  LongDoubleAlign = 32;
+} else {
+  LongDoubleWidth = 128;
+  LongDoubleAlign = 128;
+}
+  }
 }
   }
 


Index: clang/test/CodeGen/X86/long-double-config-size.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/long-double-config-size.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-64 -o - | FileCheck %s --check-prefix=SIZE64
+// RUN: %clang_cc1 -triple i386-windows-msvc %s -emit-llvm -mlong-double-80 -o - | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-80 -o - | FileCheck %s --check-prefix=SIZE80
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -mlong-double-128 -o - | FileCheck %s --check-prefix=SIZE128
+// RUN: %clang_cc1 -triple x86_64-windows-msvc %s -emit-llvm -o - | FileCheck %s --check-prefix=SIZE64
+
+long double global;
+// SIZE64: @global = dso_local global double 0
+// SIZE80: @global = dso_local global x86_fp80 0xK{{0+}}, align 16
+// SIZE128: @global = 

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 395128.
pengfei added a comment.

Split the LLVM datalayout to a different patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/X86/long-double-config-size.c
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/long-double-abi-align.ll
  llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
  llvm/test/CodeGen/X86/scalar-fp-to-i64.ll

Index: llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
===
--- llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
+++ llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
@@ -909,8 +909,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:flds __real@5f00
 ; X86-AVX512-WIN-NEXT:xorl %edx, %edx
@@ -985,8 +985,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:flds __real@5f00
 ; X86-SSE3-WIN-NEXT:xorl %edx, %edx
@@ -1061,8 +1061,8 @@
 ; X86-SSE2-WIN:   # %bb.0:
 ; X86-SSE2-WIN-NEXT:pushl %ebp
 ; X86-SSE2-WIN-NEXT:movl %esp, %ebp
-; X86-SSE2-WIN-NEXT:andl $-8, %esp
-; X86-SSE2-WIN-NEXT:subl $16, %esp
+; X86-SSE2-WIN-NEXT:andl $-16, %esp
+; X86-SSE2-WIN-NEXT:subl $32, %esp
 ; X86-SSE2-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE2-WIN-NEXT:flds __real@5f00
 ; X86-SSE2-WIN-NEXT:xorl %edx, %edx
@@ -1161,8 +1161,8 @@
 ; X87-WIN:   # %bb.0:
 ; X87-WIN-NEXT:pushl %ebp
 ; X87-WIN-NEXT:movl %esp, %ebp
-; X87-WIN-NEXT:andl $-8, %esp
-; X87-WIN-NEXT:subl $16, %esp
+; X87-WIN-NEXT:andl $-16, %esp
+; X87-WIN-NEXT:subl $32, %esp
 ; X87-WIN-NEXT:fldt 8(%ebp)
 ; X87-WIN-NEXT:flds __real@5f00
 ; X87-WIN-NEXT:fucom %st(1)
@@ -1235,8 +1235,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:fisttpll (%esp)
 ; X86-AVX512-WIN-NEXT:movl (%esp), %eax
@@ -1275,8 +1275,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:fisttpll (%esp)
 ; X86-SSE3-WIN-NEXT:movl (%esp), %eax
@@ -1315,8 +1315,8 @@
 ; X86-SSE2-WIN:   # %bb.0:
 ; X86-SSE2-WIN-NEXT:pushl %ebp
 ; X86-SSE2-WIN-NEXT:movl %esp, %ebp
-; X86-SSE2-WIN-NEXT:andl $-8, %esp
-; X86-SSE2-WIN-NEXT:subl $16, %esp
+; X86-SSE2-WIN-NEXT:andl $-16, %esp
+; X86-SSE2-WIN-NEXT:subl $32, %esp
 ; X86-SSE2-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE2-WIN-NEXT:fnstcw {{[0-9]+}}(%esp)
 ; X86-SSE2-WIN-NEXT:movzwl {{[0-9]+}}(%esp), %eax
@@ -1379,8 +1379,8 @@
 ; X87-WIN:   # %bb.0:
 ; X87-WIN-NEXT:pushl %ebp
 ; X87-WIN-NEXT:movl %esp, %ebp
-; X87-WIN-NEXT:andl $-8, %esp
-; X87-WIN-NEXT:subl $16, %esp
+; X87-WIN-NEXT:andl $-16, %esp
+; X87-WIN-NEXT:subl $32, %esp
 ; X87-WIN-NEXT:fldt 8(%ebp)
 ; X87-WIN-NEXT:fnstcw {{[0-9]+}}(%esp)
 ; X87-WIN-NEXT:movzwl {{[0-9]+}}(%esp), %eax
Index: llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
===
--- llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
+++ llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
@@ -344,8 +344,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:fisttpll (%esp)
 ; X86-AVX512-WIN-NEXT:movl (%esp), %eax
@@ -382,8 +382,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; 

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I see, thanks for the info.  Can you please add a targeted LLVM test for long 
double arguments? From what I can tell, the auto-generated 
update_llc_test_checks.py style tests are not a good fit for testing parameter 
passing because they pattern-match away the stack offsets which are relevant to 
the test.

I think it's also worth breaking this into LLVM and clang-side patches:

1. make clang emit x86_fp80 with -mlong-double-80
2. update LLVM datalayout (must affect clang via clang/lib/Basic/Targets/X86.h) 
to align fp80 to 16 bytes in the MSVC environment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Hi @rnk , mine is Phoebe#3036. I haven't really used it before. No sure if I 
invited you correctly, so I try to explain here.

> My comment here refers to the alignment of argument values, not user-declared 
> variables. The frontend controls the alignment of user-declared variables by 
> setting the alloca alignment.

Sure. We have the same goal :)

> GCC and ICC appear to align long double arguments to 4 bytes: 
> https://gcc.godbolt.org/z/PbobWdrPf

Unfortunately, compiler explorer doesn't provide the Windows version ICC. And 
the alignment on Windows is different from Linux for ICC. Here are my local 
result:

  > cat ex5.cpp
  void escape(void*);
  void foo(int x, long double d, int y) {
escape();
escape();
escape();
  }
  
  > icl -c ex5.cpp /Qlong-double /Qpc80
  > dumpbin.exe /disasm ex5.obj
  Microsoft (R) COFF/PE Dumper Version 14.29.30133.0
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  
  Dump of file ex5.obj
  
  File Type: COFF OBJECT
  
  ?foo@@YAXH_TH@Z (void __cdecl foo(int,decltype(auto),int)):
: 8D 44 24 04lea eax,[esp+4]
0004: 50 pusheax
0005: E8 00 00 00 00 call?escape@@YAXPAX@Z
000A: 8D 44 24 18lea eax,[esp+18h]
000E: 50 pusheax
000F: E8 00 00 00 00 call?escape@@YAXPAX@Z
0014: 8D 44 24 2Clea eax,[esp+2Ch]
0018: 50 pusheax
0019: E8 00 00 00 00 call?escape@@YAXPAX@Z
001E: 83 C4 0C   add esp,0Ch
0021: C3 ret
0022: 0F 1F 80 00 00 00  nop dword ptr [eax]
  00
0029: 0F 1F 80 00 00 00  nop dword ptr [eax]
  00
  
Summary
  
B7 .drectve
30 .text

As you can see, the `x` and `d` has 16 bytes distance.
I realized I should have put the result earlier to avoid the ambiguity. Sorry 
for the inconvenience!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Let me know if it would be more helpful to set up a call, that might help us 
reach agreement sooner. I've used discord for this previously if that works for 
you, my username there is the same (`@rnk#8591`).

In D115441#3194066 , @pengfei wrote:

>> GCC doesn't align fp80 long double to 16 bytes on i686, so I see no reason 
>> for LLVM to do it. Is there some other compiler that you need ABI 
>> compatibility with?
>
> Yes. ICC aligns long double to 16 bytes on 32bit Windows. (I mentioned it in 
> the summary :). In contrast with GCC, ICC is more compatible with MSVC. So I 
> think it's reasonable to align with ICC ranther than GCC.

My comment here refers to the alignment of argument values, not user-declared 
variables. The frontend controls the alignment of user-declared variables by 
setting the alloca alignment. GCC and ICC appear to align long double arguments 
to 4 bytes: https://gcc.godbolt.org/z/PbobWdrPf

>> Also consider that in LLVM, the alignment of arguments passed in memory is 
>> not observable (unless byval or inalloca is used). If the user takes the 
>> address of an argument, they actually take the address of a local alloca, 
>> which is a copy of the argument. The frontend (clang) decides the alignment 
>> of the alloca.
>
> We cannot force user to always pass arguments by address. Once they are 
> passed by value, (actually it's common in the code, we usually write like 
> foo(double a) rather than foo(double *a) ), it turns to the scope of calling 
> conversion. It's true all basic types except f80 is aligned to 4 when passed 
> by value on 32 bits. But Windows 32 bits is not alone. Darwin 32 bits uses 
> the same calling conversion for f80 too.

I don't intend to ask users to pass arguments by address. What I mean is, LLVM 
will copy an under-aligned long double argument passed in memory to properly 
aligned memory. Consider this example using double:
https://gcc.godbolt.org/z/v6oTfrTr5

  void escape(void*);
  void foo(int x, double d, int y) {
escape();
escape();
escape();
  }

-->

  "?foo@@YAXHNH@Z": # @"?foo@@YAXHNH@Z"
push ebp
mov ebp, esp
# Align stack to 8 bytes
and esp, -8
sub esp, 8
# Copy bytes of d to aligned memory
movsd xmm0, qword ptr [ebp + 12] # xmm0 = mem[0],zero
movsd qword ptr [esp], xmm0
lea eax, [ebp + 8]
  ...
# Take address of aligned alloca at ESP+0
mov eax, esp
push eax
call "?escape@@YAXPAX@Z"



---

So, I'm still not convinced we have to change the LLVM data layout. Is there 
some other aspect of calling convention lowering that needs to know the 
alignment of long double?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> GCC doesn't align fp80 long double to 16 bytes on i686, so I see no reason 
> for LLVM to do it. Is there some other compiler that you need ABI 
> compatibility with?

Yes. ICC aligns long double to 16 bytes on 32bit Windows. (I mentioned it in 
the summary :). In contrast with GCC, ICC is more compatible with MSVC. So I 
think it's reasonable to align with ICC ranther than GCC.

> Also consider that in LLVM, the alignment of arguments passed in memory is 
> not observable (unless byval or inalloca is used). If the user takes the 
> address of an argument, they actually take the address of a local alloca, 
> which is a copy of the argument. The frontend (clang) decides the alignment 
> of the alloca.

We cannot force user to always pass arguments by address. Once they are passed 
by value, (actually it's common in the code, we usually write like foo(double 
a) rather than foo(double *a) ), it turns to the scope of calling conversion. 
It's true all basic types except f80 is aligned to 4 when passed by value on 32 
bits. But Windows 32 bits is not alone. Darwin 32 bits uses the same calling 
conversion for f80 too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D115441#3191482 , @pengfei wrote:

> We have to change LLVM data layout because it's required by the calling 
> conversion.

Is that necessary? It would be simpler to leave the fp80 value 4 byte aligned, 
which I believe is consistent with the way doubles are passed unaligned. GCC 
doesn't align fp80 long double to 16 bytes on i686, so I see no reason for LLVM 
to do it. Is there some other compiler that you need ABI compatibility with?

Also consider that in LLVM, the alignment of arguments passed in memory is not 
observable (unless byval or inalloca is used). If the user takes the address of 
an argument, they actually take the address of a local alloca, which is a copy 
of the argument. The frontend (clang) decides the alignment of the alloca.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D115441#3189551 , @JohnReagan 
wrote:

> Does any of this impact the -f128 support?  We use f128 x-float on OpenVMS.  
> We've historically only aligned on 8-byte boundaries for legacy reasons (I'm 
> not opposed to having my own mods to control the record layout and/or data 
> layout)

I don't think so. `f128` has its distinct layout `-f128:xxx`, though I found 
only X86-32 MCU defines it in trunk code: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/X86.h#L628


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> I have a thought. Why do you need to change the LLVM data layout at all? 
> Clang's record layout is distinct from LLVM's data layout. This is similar to 
> how -malign-double works, which does not affect LLVM's data layout, it is 
> entirely a frontend change.

We have to change LLVM data layout because it's required by the calling 
conversion.

1. We specified the alignment of `f80` on 32 bits to 0: 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86CallingConv.td#L869
2. Which means the alignment is actually determined by the data layout: 
https://github.com/llvm/llvm-project/blob/main/llvm/utils/TableGen/CallingConvEmitter.cpp#L200

As far as I understand it, `-malign-double` only affects the alignment of local 
and global variables. It has nothing to do with calling conversion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[John Reagan via Phabricator via cfe-commits]

JohnReagan added a comment.

Does any of this impact the -f128 support?  We use f128 x-float on OpenVMS.  
We've historically only aligned on 8-byte boundaries for legacy reasons (I'm 
not opposed to having my own mods to control the record layout and/or data 
layout)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D115441#3188526 , @mstorsjo wrote:

> In D115441#3188172 , @pengfei wrote:
>
>>> In GCC on Windows (and clang in mingw mode), long double is always 80 bit 
>>> on x86. (On i386, sizeof(long double) == 12, while on x86_64 it's 16.)
>>
>> How about the alignment? I can see on the i386 Linux case, the alignment is 
>> 4, I assume it is also 4 for GCC on Windows, right?
>
> Yes, it's 4 for i386 in GCC on Windows too (and Clang in mingw mode). For 
> x86_64, both sizeof and alignof are 16.

Yeah, the alignment is the key thing which is generating a lot of the 
MSVC-specific complexity.

I have a thought. Why do you need to change the LLVM data layout at all? 
Clang's record layout is distinct from LLVM's data layout. This is similar to 
how `-malign-double` works, which does not affect LLVM's data layout, it is 
entirely a frontend change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D115441#3188172 , @pengfei wrote:

>> In GCC on Windows (and clang in mingw mode), long double is always 80 bit on 
>> x86. (On i386, sizeof(long double) == 12, while on x86_64 it's 16.)
>
> How about the alignment? I can see on the i386 Linux case, the alignment is 
> 4, I assume it is also 4 for GCC on Windows, right?

Yes, it's 4 for i386 in GCC on Windows too (and Clang in mingw mode). For 
x86_64, both sizeof and alignof are 16.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks @mstorsjo for the inputs.

> However outside of the core OS, any function in the CRT, that uses long 
> doubles, is going to be wrong

Good point! I didn't think much on the CRT library. But I think this is not a 
blocking issue, given

1. The option is off by default. So it's not destructive for the code that 
using default CRT.
2. For users who use this option, they should have knowledge of the difference 
on long double type. There're 2 use scenarios I can think out:
  - Users who have their own CRT libraries. This is the case for our downstream 
compiler.
  - Users who are using freestanding environment or using CRT with their own 
implementation of long double functions.

> In GCC on Windows (and clang in mingw mode), long double is always 80 bit on 
> x86. (On i386, sizeof(long double) == 12, while on x86_64 it's 16.)

How about the alignment? I can see on the i386 Linux case, the alignment is 4, 
I assume it is also 4 for GCC on Windows, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D115441#3187009 , @rnk wrote:

> I seem to recall assuming that Windows `long double` was 64-bits in many, 
> many places. Unfortunately, I have no idea where that could've happened.

Nothing comes to mind for me about that - in _most_ cases, Windows is kinda 
oblivious to `long double`, as nothing in Windows public API uses that type.

However outside of the core OS, any function in the CRT, that uses long 
doubles, is going to be wrong; in the C99 runtime, there's plenty of `long 
double` functions - a separate `-l` suffixed version of most math functions, 
but also more important conversion functions like `strtold`.

> Something that comes to mind immediately is the MSVC name mangler. I don't 
> think that's a blocking issue, but it's something you should be aware of if 
> you want to promote this flag's usage.

Oh, right, I have no familiarity with those aspects and what might break there.

> @mstorsjo, can you advise what GCC does here? I've forgotten how this is 
> supposed to work.

In GCC on Windows (and clang in mingw mode), `long double` is always 80 bit on 
x86. (On i386, `sizeof(long double) == 12`, while on x86_64 it's 16.)

Regarding the initial FPU state, I think the statically linked CRT startup bits 
differ from MSVC in this aspect, so the x87 state is initialized in 80 bit mode.

Then for runtime functions, mingw handles this by providing their own 
(statically linked) reimplementation of essentially all functions that touch 
long doubles. For math and similar, it's pretty straightforward, but for 
printf, it's a bit of a mess since we'd otherwise want to use UCRT's (otherwise 
standards compliant) printfs, but whenever long doubles are involved (very 
rarely in practice, but libc++'s testsuite do exercise them) the mingw provided 
version has to be used.

For mingw on arm (32 and 64) I haven't wanted to introduce any further deviance 
from MSVC, so there it's all identical to plain double.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> I seem to recall assuming that Windows long double was 64-bits in many, many 
> places. Unfortunately, I have no idea where that could've happened.

Is it because MSDN explicitly declares it? 
https://docs.microsoft.com/en-us/previous-versions/9cx8xs15(v=vs.140)?redirectedfrom=MSDN

> Something that comes to mind immediately is the MSVC name mangler. I don't 
> think that's a blocking issue, but it's something you should be aware of if 
> you want to promote this flag's usage.

Not sure if I got your point. I checked both Clang and MSVC can mangle/demangle 
it, e.g., "long double foo(long double a, long double b)" <==> "?foo@@YAOOO@Z". 
So this is not a problem?

  >undname "?foo@@YAOOO@Z"
  Microsoft (R) C++ Name Undecorator
  Copyright (C) Microsoft Corporation. All rights reserved.
  
  Undecoration of :- "?foo@@YAOOO@Z"
  is :- "long double __cdecl foo(long double,long double)"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: mstorsjo.
rnk added a comment.

I seem to recall assuming that Windows `long double` was 64-bits in many, many 
places. Unfortunately, I have no idea where that could've happened. Something 
that comes to mind immediately is the MSVC name mangler. I don't think that's a 
blocking issue, but it's something you should be aware of if you want to 
promote this flag's usage.

@mstorsjo, can you advise what GCC does here? I've forgotten how this is 
supposed to work.




Comment at: clang/lib/Basic/Targets/X86.h:537
 resetDataLayout(IsWinCOFF ? "e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:"
 "64-i64:64-f80:32-n8:16:32-a:0:32-S32"
   : "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:"

If GCC aligns f80 to 16 bytes, we might as well make the change here and share 
it with the mingw target.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D115441#3182938 , @craig.topper 
wrote:

> Doesn’t icc also emit code into main to change the FPCW precision control 
> field? Is making long double 80 bits useful if you don’t increase the 
> precision in hardware?

Yes, but it is controlled by an independent option `/Qpc80`. We can do a 
follow-up if it is required. Currently, I think it's still useful for library 
build and functional compatibility with ICC. Even considering the missing 
`/Qpc80`, we can still get the less precise result rather than the wrong one 
when linking each other. Besides, users are always able to set it manually.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Doesn’t icc also emit code into main to change the FPCW precision control 
field? Is making long double 80 bits useful if you don’t increase the precision 
in hardware?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

This looks good to me, and mirrors something I implemented in our downstream a 
few years ago.  Please don't submit for another day or two to give others a 
chance to review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115441/new/

https://reviews.llvm.org/D115441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Phoebe Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: rnk, andrew.w.kaylor, erichkeane, craig.topper.
Herald added a subscriber: hiraditya.
pengfei requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

MSVC currently doesn't support 80 bits long double. But ICC does support
it on Windows. Besides, there're also some users asked for this feature.
We can find the discussions from stackoverflow, msdn etc.

Given Clang has already support `-mlong-double-80`, extending it to
support for Windows seems worthwhile.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115441

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/X86/long-double-config-size.c
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
  llvm/test/CodeGen/X86/scalar-fp-to-i64.ll

Index: llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
===
--- llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
+++ llvm/test/CodeGen/X86/scalar-fp-to-i64.ll
@@ -909,8 +909,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:flds __real@5f00
 ; X86-AVX512-WIN-NEXT:xorl %edx, %edx
@@ -985,8 +985,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:flds __real@5f00
 ; X86-SSE3-WIN-NEXT:xorl %edx, %edx
@@ -1061,8 +1061,8 @@
 ; X86-SSE2-WIN:   # %bb.0:
 ; X86-SSE2-WIN-NEXT:pushl %ebp
 ; X86-SSE2-WIN-NEXT:movl %esp, %ebp
-; X86-SSE2-WIN-NEXT:andl $-8, %esp
-; X86-SSE2-WIN-NEXT:subl $16, %esp
+; X86-SSE2-WIN-NEXT:andl $-16, %esp
+; X86-SSE2-WIN-NEXT:subl $32, %esp
 ; X86-SSE2-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE2-WIN-NEXT:flds __real@5f00
 ; X86-SSE2-WIN-NEXT:xorl %edx, %edx
@@ -1161,8 +1161,8 @@
 ; X87-WIN:   # %bb.0:
 ; X87-WIN-NEXT:pushl %ebp
 ; X87-WIN-NEXT:movl %esp, %ebp
-; X87-WIN-NEXT:andl $-8, %esp
-; X87-WIN-NEXT:subl $16, %esp
+; X87-WIN-NEXT:andl $-16, %esp
+; X87-WIN-NEXT:subl $32, %esp
 ; X87-WIN-NEXT:fldt 8(%ebp)
 ; X87-WIN-NEXT:flds __real@5f00
 ; X87-WIN-NEXT:fucom %st(1)
@@ -1235,8 +1235,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl $16, %esp
 ; X86-AVX512-WIN-NEXT:fldt 8(%ebp)
 ; X86-AVX512-WIN-NEXT:fisttpll (%esp)
 ; X86-AVX512-WIN-NEXT:movl (%esp), %eax
@@ -1275,8 +1275,8 @@
 ; X86-SSE3-WIN:   # %bb.0:
 ; X86-SSE3-WIN-NEXT:pushl %ebp
 ; X86-SSE3-WIN-NEXT:movl %esp, %ebp
-; X86-SSE3-WIN-NEXT:andl $-8, %esp
-; X86-SSE3-WIN-NEXT:subl $8, %esp
+; X86-SSE3-WIN-NEXT:andl $-16, %esp
+; X86-SSE3-WIN-NEXT:subl $16, %esp
 ; X86-SSE3-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE3-WIN-NEXT:fisttpll (%esp)
 ; X86-SSE3-WIN-NEXT:movl (%esp), %eax
@@ -1315,8 +1315,8 @@
 ; X86-SSE2-WIN:   # %bb.0:
 ; X86-SSE2-WIN-NEXT:pushl %ebp
 ; X86-SSE2-WIN-NEXT:movl %esp, %ebp
-; X86-SSE2-WIN-NEXT:andl $-8, %esp
-; X86-SSE2-WIN-NEXT:subl $16, %esp
+; X86-SSE2-WIN-NEXT:andl $-16, %esp
+; X86-SSE2-WIN-NEXT:subl $32, %esp
 ; X86-SSE2-WIN-NEXT:fldt 8(%ebp)
 ; X86-SSE2-WIN-NEXT:fnstcw {{[0-9]+}}(%esp)
 ; X86-SSE2-WIN-NEXT:movzwl {{[0-9]+}}(%esp), %eax
@@ -1379,8 +1379,8 @@
 ; X87-WIN:   # %bb.0:
 ; X87-WIN-NEXT:pushl %ebp
 ; X87-WIN-NEXT:movl %esp, %ebp
-; X87-WIN-NEXT:andl $-8, %esp
-; X87-WIN-NEXT:subl $16, %esp
+; X87-WIN-NEXT:andl $-16, %esp
+; X87-WIN-NEXT:subl $32, %esp
 ; X87-WIN-NEXT:fldt 8(%ebp)
 ; X87-WIN-NEXT:fnstcw {{[0-9]+}}(%esp)
 ; X87-WIN-NEXT:movzwl {{[0-9]+}}(%esp), %eax
Index: llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
===
--- llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
+++ llvm/test/CodeGen/X86/scalar-fp-to-i32.ll
@@ -344,8 +344,8 @@
 ; X86-AVX512-WIN:   # %bb.0:
 ; X86-AVX512-WIN-NEXT:pushl %ebp
 ; X86-AVX512-WIN-NEXT:movl %esp, %ebp
-; X86-AVX512-WIN-NEXT:andl $-8, %esp
-; X86-AVX512-WIN-NEXT:subl $8, %esp
+; X86-AVX512-WIN-NEXT:andl $-16, %esp
+; X86-AVX512-WIN-NEXT:subl