[llvm-commits] [ARM] Extra frame for non-VA byval parameter

[manman ren]

For the patch, should we pass VA.getLocMemOffset() as ArgOffset to VarArgStyleRegisters?

-            int FI = MFI->CreateFixedObject(Bytes,
-                                            VA.getLocMemOffset(), false);
+              VarArgStyleRegisters(CCInfo, DAG,
+                                   dl, Chain, CurOrigArg,
+                                   Ins[VA.getValNo()].PartOffset, 0);

Otherwise LGTM,

Thanks,
Manman

On Oct 17, 2012, at 10:45 AM, manman ren <mren at apple.com> wrote:

> 
> Hi Stepan,
> 
> Yes, when VARegSaveSize is not equal to VARegSize, the first stack frame will start at a different offset from the second stack frame.
> In PR14099, the first stack frame starts at byte 4, while the second stack frame starts at byte 0.
> And we should use the first stack frame for correctness.
> 
> Please attach a testing case with your patch.
> 
> BTW, this should not be a problem for ABIs with 4-byte stack alignment where VARegSize is equal to VARegSaveSize.
> 
> Thanks for fixing the problem,
> Manman
> 
> On Oct 17, 2012, at 4:23 AM, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:
> 
>> Hi Manman,
>> 
>> Yesterday Saleem Abdulrasool created PR14099 that is demonstrates issue described in this thread.
>> 
>> Below I try to explain, what happens in PR14099.
>> 
>> BTW, note, VarArgStyleRegisters should be invoked the only time for each function prologue. This restriction is not satisfied if we have N byval fixed args.
>> 
>> OK. Assume we have the only fixed byval argument.
>> For fixed frame argument first stack frame (for registers) is created always in the beginning of arguments area (VarArgStyleRegisters invoked with ArgOffset == 0). Second stack frame is created with ArgOffset == 0, since it is first byval argument. Though the end of data is truncated by size of head stored in regs :-)
>> 
>> Data restoring approach suppose next: registers data should be saved in area, that is *attached* to the memory area:
>> (you need fixed width fonts here)
>> 
>>                  | [Initial SP points to mem area start]
>>                  V
>> [r0, r1, r2, ... ][memory area]
>> 
>> That works fine for VA functions. For fixed args function
>> current implementation brokes this approach.
>> 
>> PR14099 demonstrates that:
>> If 3 regs are used we still have 16 bytes stack area for registers (in EABI mode stack has 8 bytes alignment), but stack frame for regs is created with 4 bytes offset. "second" stack frame for memory data is still created with 0 bytes offset (since it knows nothing about byval regs). And we got the next stack frames:
>> 
>> [Saved regs area]|           | [Initial SP value]
>>                |           V
>>                V          [remained data stored by caller]
>> <4 dummy bytes> [r1, r2, r3]
>> [ memory stack frame ............................] <truncated>
>> 
>> Of course we got broken date if we will read it from memory stack frame.
>> 
>> We should generate at least the next one:
>> 
>> [Saved regs area]|           | [Initial SP value]
>>                |           V
>>                V          [remained data stored by caller]
>> <4 dummy bytes> [r1, r2, r3]
>>               [ memory stack frame.......... ] <truncated>
>> 
>> Note, truncation matters nothing here. Only SP offset is important. For VA functions in case where there are no regs available, only 4-bytes sized stack frame is created.
>> I still propose to combine regs and memory stack frames. It is already combined for VA functions, I propose to unify behaviour and to combine frames for fixed args functions too. Note this behaviour is unified for callers.
>> It should solve issues described in this thread and PR14099.
>> 
>> Please find updated patch in attachment.
>> 
>> -Stepan.
>> 
>> 
>> <byval-frame-fix.patch>
>