[PATCH] D26151: RegCall - Handling long double arguments

[Akira Hatanaka via llvm-commits]

ahatanak added inline comments.


================
Comment at: lib/Target/X86/X86CallingConv.td:160
     // long double --> FP
-    CCIfType<[f80], CCAssignToReg<[FP0]>>,
+    CCIfType<[f80], CCAssignToReg<RC.FP>>,
 
----------------
Since RC.FP includes only FP0, this indicates that only one register is used to return a value even though two registers are used when returning complex long double. I think this currently works only because it falls backs on RetCC_X86Common to find the return register for the latter half of the complex long double return value.

Wouldn't it be better to include both FP0 and FP1 in RC.FP?


================
Comment at: lib/Target/X86/X86FloatingPoint.cpp:973
   assert(STReturns == 0 || (isMask_32(STReturns) && N <= 2));
+  StackTop = 0;
 
----------------
oren_ben_simhon wrote:
> ahatanak wrote:
> > oren_ben_simhon wrote:
> > > ahatanak wrote:
> > > > oren_ben_simhon wrote:
> > > > > ahatanak wrote:
> > > > > > I'm not sure what this comment means.
> > > > > > 
> > > > > > It seems like you are clearing StackTop because the first FP register can be used to pass an argument when the calling convention is regcall. Is that correct?
> > > > > The problem occurs when FP0 was passed as argument and we try to return a parameter in FP0 as well.
> > > > > In that case the StackTop points to 1 and we get to the following loop and we call pushreg(0) when FP0 is already assigned and StackTop is 1. 
> > > > > 
> > > > > So what actually happened before pushreg:    
> > > > >            Stack[] = { 0, ...}
> > > > >            RegMap[] = { 0, ...}
> > > > > While after pushreg[0] (No stacktop reset):      
> > > > >            Stack[] = { 0, 1..}
> > > > >            RegMap[] = { 1...}
> > > > > 
> > > > > The issue causes debug assertion in line 169 to fail because then regmap[stack[0]] != 0.
> > > > > 
> > > > > I changed the way I handle the issue.
> > > > > Instead of reseting the StackTop to zero, I do not push registers if they were already pushed into the stack.
> > > > Wouldn't this push an extra register when a function taking a long double argument and returning complex long double were called (in which case N==2 because it's returned in st0 and st1)?
> > > AFAIK, Long doubles are saved inside a single ST register (size of 80 bit). 
> > > So in the case of one FP argument and one FP retruned value, I expect N to be 1. 
> > > Meaning, The value will be returned in ST0.
> > When the following code is compiled with clang, the complex long double return value is returned in two registers. N is 2 and StackTop is 1 when the call to foo3 is visited in FPS::handleCall:
> > 
> > ```
> > typedef long double _Complex CLD;
> > long double d0;
> > 
> > CLD __regcall foo3(long double a);
> > 
> > CLD __regcall  foo2(void) {
> >   return foo3(d0);
> > }
> > ```
> > 
> > $ clang -cc1  -ffreestanding -triple=x86_64-pc-linux-gnu -o - -S  test1.c
> > 
> > Do you know why this is happening?
> You are correct. It acts according the default x64 calling convention, which returns COMPLEX_X87 in two registers.
> The updated condition below will not break this behaviour and still two registers will be assigned.
If StackTop == 1 and N ==2, this pushes FP0 and FP1 to the stack in the reverse order (FP0 is on the stack already and then pushReg(1) is called). I think you have to reset StackTop and clear Stack and RegMap before pushing all return registers to the stack.


Repository:
  rL LLVM

https://reviews.llvm.org/D26151