[X86][ABI] Change the alignment of f80 in 32-bit calling convention to meet with different data layout

[RKSimon]

Should we be testing more target triples? I'm not clear how much variety there is on 32-bit targets.

llvm/test/CodeGen/X86/inline-asm-fpstack.ll

29

precommit the nounwind change so its obvious where the stack manip is coming from

[pengfei]

Rebased.

[pengfei]

Should we be testing more target triples? I'm not clear how much variety there is on 32-bit targets.

I think it's enough. Currently we only have darwin that has different f80 alignment on 32-bit:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86TargetMachine.cpp#L130

[RKSimon]

Is there a bugzilla PR covering this?

llvm/test/CodeGen/X86/inline-asm-fpstack.ll

2

So maybe add a i386-linux-unknown test?

[pengfei]

Is there a bugzilla PR covering this?

Probability not. I guess few people concerns 32-bit darwin now. I didn't mean to fix the bug. We just have the same requirment in our downstream code.

llvm/test/CodeGen/X86/inline-asm-fpstack.ll

2

I tried. Then I found the check doubled due to darwin and Linux have different inline asm comment. More importantly, the interesting info is buried by the large diff.
On the other hand, we have lots of lit tests involves f80 load/store. We have confidence given they are not changed :)

$ grep -r -E 'fldt|fstpt' llvm/test/CodeGen/X86/ | wc -l
994
$ grep -r -E 'fldt|fstpt' llvm/test/CodeGen/X86/ |cut -d ':' -f 1 |uniq | wc -l
58

[RKSimon]

LGTM - thanks for checking

[pengfei]

Thanks Simon!

[LuoYuanke]

Minor comments for the title. It seems a typo "conversion -> convention".

[pengfei]

Thanks Yuanke!

llvm/lib/Target/X86/X86CallingConv.td

def CC_X86_32_Common : CallingConv<[
 
   // Integer/Float values get stored in stack slots that are 4 bytes in
   // size and 4-byte aligned.
   CCIfType<[i32, f32], CCAssignToStack<4, 4>>,
 
   // Doubles get 8-byte slots that are 4-byte aligned.
   CCIfType<[f64], CCAssignToStack<8, 4>>,
 
-  // Long doubles get slots whose size depends on the subtarget.
-  CCIfType<[f80], CCAssignToStack<0, 4>>,
+  // Long doubles get slots whose size and alignment depends on the subtarget.
+  CCIfType<[f80], CCAssignToStack<0, 0>>,
 
   // Boolean vectors of AVX-512 are passed in SIMD registers.
   // The call from AVX to AVX-512 function should work,
   // since the boolean types in AVX/AVX2 are promoted by default.
   CCIfType<[v2i1],  CCPromoteToType<v2i64>>,
   CCIfType<[v4i1],  CCPromoteToType<v4i32>>,
   CCIfType<[v8i1],  CCPromoteToType<v8i16>>,
   CCIfType<[v16i1], CCPromoteToType<v16i8>>,

llvm/test/CodeGen/X86/2007-09-27-LDIntrinsics.ll

 ; RUN: llc < %s | FileCheck %s
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
 target triple = "i686-apple-darwin8"
 
 define x86_fp80 @foo(x86_fp80 %x) nounwind{
 entry:
   %tmp2 = call x86_fp80 @llvm.sqrt.f80( x86_fp80 %x )
   ret x86_fp80 %tmp2
         
 ; CHECK-LABEL: foo:
-; CHECK: fldt 4(%esp)
+; CHECK: fldt 16(%esp)
 ; CHECK-NEXT: fsqrt
+; CHECK-NEXT: addl $12, %esp
 ; CHECK-NEXT: ret
 }
 
 declare x86_fp80 @llvm.sqrt.f80(x86_fp80)
 
 define x86_fp80 @bar(x86_fp80 %x) nounwind {
 entry:
   %tmp2 = call x86_fp80 @llvm.powi.f80.i32( x86_fp80 %x, i32 3 )
   ret x86_fp80 %tmp2
 ; CHECK-LABEL: bar:
-; CHECK: fldt 4(%esp)
+; CHECK: fldt 16(%esp)
 ; CHECK-NEXT: fld %st(0)
 ; CHECK-NEXT: fmul  %st(1)
 ; CHECK-NEXT: fmulp
+; CHECK-NEXT: addl $12, %esp
 ; CHECK-NEXT: ret
 }
 
 declare x86_fp80 @llvm.powi.f80.i32(x86_fp80, i32)

llvm/test/CodeGen/X86/inline-asm-fpstack.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mcpu=generic -mtriple=i386-apple-darwin -verify-machineinstrs -no-integrated-as | FileCheck %s
 
 ; There should be no stack manipulations between the inline asm and ret.
 define x86_fp80 @test1() {
 ; CHECK-LABEL: test1:
 ; CHECK:       ## %bb.0:
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    fld0
 ; CHECK-NEXT:    ## InlineAsm End

 ; CHECK-NEXT:    ## InlineAsm End
 ; CHECK-NEXT:    retl
   %tmp85 = call double asm sideeffect "fld0", "={st(0)}"()
   ret double %tmp85
 }
 
 ; Setting up argument in st(0) should be a single fld.
 ; Asm consumes stack, nothing should be popped.
-define void @test3(x86_fp80 %X) {
+define void @test3(x86_fp80 %X) nounwind {
 ; CHECK-LABEL: test3:
 ; CHECK:       ## %bb.0:
+; CHECK-NEXT:    subl $12, %esp
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    frob
 ; CHECK-NEXT:    ## InlineAsm End
+; CHECK-NEXT:    addl $12, %esp
 ; CHECK-NEXT:    retl
   call void asm sideeffect "frob ", "{st(0)},~{st},~{dirflag},~{fpsr},~{flags}"( x86_fp80 %X)
   ret void
 }
 
 define void @test4(double %X) {
 ; CHECK-LABEL: test4:
 ; CHECK:       ## %bb.0:

 ; This doesn't implicitly pop the stack:
 ;
 ;   void fist1(long double x, int *p) {
 ;     asm volatile ("fistl %1" : : "t"(x), "m"(*p));
 ;   }
 define void @fist1(x86_fp80 %x, i32* %p) nounwind ssp {
 ; CHECK-LABEL: fist1:
 ; CHECK:       ## %bb.0: ## %entry
+; CHECK-NEXT:    subl $12, %esp
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    fistl (%eax)
 ; CHECK-NEXT:    ## InlineAsm End
 ; CHECK-NEXT:    fstp %st(0)
+; CHECK-NEXT:    addl $12, %esp
 ; CHECK-NEXT:    retl
 entry:
   tail call void asm sideeffect "fistl $1", "{st},*m,~{memory},~{dirflag},~{fpsr},~{flags}"(x86_fp80 %x, i32* %p) nounwind
   ret void
 }
 
 ; Here, the input operand is tied to an output which means that is is
 ; implicitly popped (and then the output is implicitly pushed).
 ;
 ;   long double fist2(long double x, int *p) {
 ;     long double y;
 ;     asm ("fistl %1" : "=&t"(y) : "0"(x), "m"(*p) : "memory");
 ;     return y;
 ;   }
 define x86_fp80 @fist2(x86_fp80 %x, i32* %p) nounwind ssp {
 ; CHECK-LABEL: fist2:
 ; CHECK:       ## %bb.0: ## %entry
+; CHECK-NEXT:    subl $12, %esp
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    fistl (%eax)
 ; CHECK-NEXT:    ## InlineAsm End
+; CHECK-NEXT:    addl $12, %esp
 ; CHECK-NEXT:    retl
 entry:
   %0 = tail call x86_fp80 asm "fistl $2", "=&{st},0,*m,~{memory},~{dirflag},~{fpsr},~{flags}"(x86_fp80 %x, i32* %p) nounwind
   ret x86_fp80 %0
 }
 
 ; An 'f' constraint is never implicitly popped:
 ;
 ;   void fucomp1(long double x, long double y) {
 ;     asm volatile ("fucomp %1" : : "t"(x), "f"(y) : "st");
 ;   }
 define void @fucomp1(x86_fp80 %x, x86_fp80 %y) nounwind ssp {
 ; CHECK-LABEL: fucomp1:
 ; CHECK:       ## %bb.0: ## %entry
+; CHECK-NEXT:    subl $12, %esp
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    fxch %st(1)
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    fucomp %st(1)
 ; CHECK-NEXT:    ## InlineAsm End
 ; CHECK-NEXT:    fstp %st(0)
+; CHECK-NEXT:    addl $12, %esp
 ; CHECK-NEXT:    retl
 entry:
   tail call void asm sideeffect "fucomp $1", "{st},f,~{st},~{dirflag},~{fpsr},~{flags}"(x86_fp80 %x, x86_fp80 %y) nounwind
   ret void
 }
 
 ; The 'u' constraint is only popped implicitly when clobbered:
 ;
 ;   void fucomp2(long double x, long double y) {
 ;     asm volatile ("fucomp %1" : : "t"(x), "u"(y) : "st");
 ;   }
 ;
 ;   void fucomp3(long double x, long double y) {
 ;     asm volatile ("fucompp %1" : : "t"(x), "u"(y) : "st", "st(1)");
 ;   }
 ;
 define void @fucomp2(x86_fp80 %x, x86_fp80 %y) nounwind ssp {
 ; CHECK-LABEL: fucomp2:
 ; CHECK:       ## %bb.0: ## %entry
+; CHECK-NEXT:    subl $12, %esp
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    fxch %st(1)
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    fucomp %st(1)
 ; CHECK-NEXT:    ## InlineAsm End
 ; CHECK-NEXT:    fstp %st(0)
+; CHECK-NEXT:    addl $12, %esp
 ; CHECK-NEXT:    retl
 entry:
   tail call void asm sideeffect "fucomp $1", "{st},{st(1)},~{st},~{dirflag},~{fpsr},~{flags}"(x86_fp80 %x, x86_fp80 %y) nounwind
   ret void
 }
 
 define void @fucomp3(x86_fp80 %x, x86_fp80 %y) nounwind ssp {
 ; CHECK-LABEL: fucomp3:
 ; CHECK:       ## %bb.0: ## %entry
+; CHECK-NEXT:    subl $12, %esp
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    fldt {{[0-9]+}}(%esp)
 ; CHECK-NEXT:    fxch %st(1)
 ; CHECK-NEXT:    ## InlineAsm Start
 ; CHECK-NEXT:    fucompp %st(1)
 ; CHECK-NEXT:    ## InlineAsm End
+; CHECK-NEXT:    addl $12, %esp
 ; CHECK-NEXT:    retl
 entry:
   tail call void asm sideeffect "fucompp $1", "{st},{st(1)},~{st},~{st(1)},~{dirflag},~{fpsr},~{flags}"(x86_fp80 %x, x86_fp80 %y) nounwind
   ret void
 }
 
 ; One input, two outputs, one dead output.
 %complex = type { float, float }