Fix PR15293: ARM codegen ice - expected larger existing stack allocation
Stepan Dyatkovskiy
stpworld at narod.ru
Tue Apr 30 12:10:30 PDT 2013
Commited in r180774. Now I'm working on milestone patch, that fixes
byval support for llvm level. I published it before, now I need to
update it. Hope it come soon.
-Stepan.
Manman Ren wrote:
>
> LGTM,
>
> Thanks,
> Manman
>
> On Apr 28, 2013, at 10:50 AM, Stepan Dyatkovskiy wrote:
>
>> Hi Manman,
>> Just for keeping things actual: I reattached patch with new names.
>> -Stepan
>>
>> Stepan Dyatkovskiy wrote:
>>> Hi Manman,
>>>
>>>> Since it represents the stack area needed to save parameters that are
>>> actually passed in registers, how about RegsSaveSizeForParams or just
>>> RegsSaveSize?
>>>
>>> In AAPCS R0-R3 are called like:
>>> "Argument / result / scratch register".
>>> So maybe we'll use "set/getArgRegsSaveSize" names?
>>>
>>>> Also it is weird to have "unsigned VARegSaveSize =
>>> AFI->getByValRegsSaveSize();"
>>>
>>> Yea :-) Here replace "VAReg" with "ArgRegs": ArgRegsSaveSize, ArgRegsSize.
>>>
>>> -Stepan
>>>
>>> Manman Ren wrote:
>>>>
>>>> Hi Stepan,
>>>>
>>>> Thanks for working on this.
>>>>
>>>> I put in a comment about the refactor patch sometime ago:
>>>>>> I am not sure about the renaming patch though. Functions
>>>>>> get|setVarArgsRegSaveSize are used for both vararg and byval.
>>>>>> It does not sound right to replace VarARgs with ByVal.
>>>>>
>>>>> Actually it is used not for vararg function, as I found it main role
>>>>> of this param in emitPrologue, it is used for proper stack allocation.
>>>>> Stack size should enough for storing all registers, used for byval
>>>>> parameters. And yes, if it is VA function, we must also extend this
>>>>> stack area for all unallocated R0-R3 regs storage.
>>>>
>>>> Agree. That is why I think ByVal is not a great name either.
>>>> Since it represents the stack area needed to save parameters that are
>>>> actually passed in registers, how about RegsSaveSizeForParams or just
>>>> RegsSaveSize?
>>>> Also it is weird to have "unsigned VARegSaveSize =
>>>> AFI->getByValRegsSaveSize();" where we assign return value of
>>>> getByValRegsSaveSize to a variable "VARegSaveSize".
>>>>
>>>> Manman
>>>>
>>>> On Apr 26, 2013, at 11:12 AM, Stepan Dyatkovskiy wrote:
>>>>
>>>>> Hi Manman and Renato,
>>>>> Patch was tested with test-suite all test have bee passed.
>>>>> -Stepan
>>>>>
>>>>> Stepan Dyatkovskiy wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> AAPCS, 5.5 Parameters Passing, Stage C, C4 and C5 have been fixed,
>>>>>> r180011.
>>>>>>
>>>>>> So, next patch is in attachment: VarArgsStyleRegisters refactoring.
>>>>>>
>>>>>> -Stepan.
>>>>>>
>>>>>> Stepan Dyatkovskiy wrote:
>>>>>>> Hello, Manman.
>>>>>>> That's kind of snowball!
>>>>>>> Today test-suite finished with two new failures on my beagle.
>>>>>>> It helped me find out one more hidden logic dependency in ARM backend.
>>>>>>>
>>>>>>> May be its not a mistake, but look:
>>>>>>>
>>>>>>> In ARMCallingConv.h, "f64AssignAAPCS" allocates registers for f64.
>>>>>>> This
>>>>>>> method is called for VA functions, since there is no CPRCs, and *any*
>>>>>>> parameters could be allocated in R0-R3. Either R0,R1 or R2,R3 could be
>>>>>>> allocated here. If all the registers except R3 were allocated
>>>>>>> somewhere
>>>>>>> before, f64AssignAAPCS just moves whole f64 to stack, it doesn't waste
>>>>>>> R3, so R3 could be allocated later. But logic seems to be same as in
>>>>>>> ARMTargetLowering::HandleByVal.
>>>>>>>
>>>>>>> My main fix in calling convention was:
>>>>>>>
>>>>>>> def CC_ARM_AAPCS_Common : CallingConv<[
>>>>>>> ...
>>>>>>> - CCIfType<[i32], CCIf<"State.getNextStackOffset() == 0 &&"
>>>>>>> - "ArgFlags.getOrigAlign() != 8",
>>>>>>> + CCIfType<[i32], CCIf<"ArgFlags.getOrigAlign() != 8",
>>>>>>> CCAssignToReg<[R0, R1, R2, R3]>>>,
>>>>>>>
>>>>>>> Now I got some picture why "State.getNextStackOffset() == 0" was
>>>>>>> added:
>>>>>>> due to f64AssignAAPCS implementation. May be the are more reasons:
>>>>>>> next
>>>>>>> test-suite results will show it then :-)
>>>>>>>
>>>>>>> Anyway, original CCIfType brakes C.4 and C.5, while my fix brakes some
>>>>>>> implicit statement defined in f64AssignAAPCS:
>>>>>>>
>>>>>>> "stack-offset != 0 doesn't mean all registers are allocated"
>>>>>>>
>>>>>>> So I've fixed f64AssignAAPCS too. Once we waste registers in
>>>>>>> HandleByVal
>>>>>>> (when we send parameter to stack), I think it would be good to keep
>>>>>>> same
>>>>>>> behaviour for f64AssignAAPCS since its HandleByVal's analogue for f64
>>>>>>> params.
>>>>>>>
>>>>>>> I attached new patch, now with 3 tests :-)
>>>>>>> (3rd test is 2013-04-21-AAPCS-VA-C.1.cp.ll)
>>>>>>>
>>>>>>> -Stepan.
>>>>>>>
>>>>>>> Manman Ren wrote:
>>>>>>>>
>>>>>>>> LGTM,
>>>>>>>>
>>>>>>>> Thanks a lot,
>>>>>>>> Manman
>>>>>>>>
>>>>>>>> On Apr 18, 2013, at 8:41 PM, Stepan Dyatkovskiy wrote:
>>>>>>>>
>>>>>>>>> Fixed :-) New patch (+2 tests) is attached.
>>>>>>>>> -Stepan
>>>>>>>>>
>>>>>>>>> Manman Ren wrote:
>>>>>>>>>>
>>>>>>>>>> "AAPCS-C.4-vs-VFP-2013-04-17.patch" looks good to me.
>>>>>>>>>>
>>>>>>>>>> As a side note, we may have issues with 9 floats, 3 ints and a
>>>>>>>>>> small
>>>>>>>>>> byval.
>>>>>>>>>> The standard says we should split the byval only when NSAA is equal
>>>>>>>>>> to SP (C.5), in the case of 9 floats, 3 ints and a small byval,
>>>>>>>>>> we should not split the byval since we advanced NSAA with the 9th
>>>>>>>>>> float.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Manman
>>>>>>>>>>
>>>>>>>>>> On Apr 16, 2013, at 8:51 PM, Stepan Dyatkovskiy wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Manman,
>>>>>>>>>>>
>>>>>>>>>>>> AAPCS-C.4-vs-VFP-2013-04-16.patch looks good except that the
>>>>>>>>>>>> testing case.
>>>>>>>>>>>> Should it have 9 doubles and 1 integer instead of a single double
>>>>>>>>>>>> and a single integer?
>>>>>>>>>>>> The testing case passes even without the fix.
>>>>>>>>>>> That was my midnight mistake, sorry.
>>>>>>>>>>> I reattached patch with proper test. In the end of test file, I've
>>>>>>>>>>> added comments for @doFoo: small description of how parameters
>>>>>>>>>>> must
>>>>>>>>>>> be passed:
>>>>>>>>>>> first eight doubles --> D0-D7
>>>>>>>>>>> 9th double --> Stack
>>>>>>>>>>> 10th param (first i32) --> R0.
>>>>>>>>>>>
>>>>>>>>>>> It is important to have 9 double params here, since we must
>>>>>>>>>>> fill-up
>>>>>>>>>>> all possible VFPs, then get compiler to use stack offset, and only
>>>>>>>>>>> after start use GPRs.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I am not sure about the renaming patch though. Functions
>>>>>>>>>>>> get|setVarArgsRegSaveSize are used for both vararg and byval.
>>>>>>>>>>>> It does not sound right to replace VarARgs with ByVal.
>>>>>>>>>>>
>>>>>>>>>>> Actually it is used not for vararg function, as I found it main
>>>>>>>>>>> role of this param in emitPrologue, it is used for proper stack
>>>>>>>>>>> allocation.
>>>>>>>>>>> Stack size should enough for storing all registers, used for byval
>>>>>>>>>>> parameters. And yes, if it is VA function, we must also extend
>>>>>>>>>>> this
>>>>>>>>>>> stack area for all unallocated R0-R3 regs storage.
>>>>>>>>>> Agree. That is why I think ByVal is not a great name either.
>>>>>>>>>> Since it represents the stack area needed to save parameters that
>>>>>>>>>> are actually passed in registers, how about
>>>>>>>>>> RegsSaveSizeForParams or
>>>>>>>>>> just RegsSaveSize?
>>>>>>>>>> Also it is weird to have "unsigned VARegSaveSize =
>>>>>>>>>> AFI->getByValRegsSaveSize();" where we assign return value of
>>>>>>>>>> getByValRegsSaveSize to a variable "VARegSaveSize".
>>>>>>>>>>
>>>>>>>>>> Thanks for working on this.
>>>>>>>>>>>
>>>>>>>>>>> -Stepan.
>>>>>>>>>>>
>>>>>>>>>>>> Thanks for working on this.
>>>>>>>>>>>> Manman
>>>>>>>>>>>>
>>>>>>>>>>>> On Apr 15, 2013, at 9:23 AM, Stepan Dyatkovskiy wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Manman,
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> *4.* One new change I've made is
>>>>>>>>>>>>>>> CC_ARM_AAPCS_Common rule.
>>>>>>>>>>>>>> Can we fix the above issue in a separate patch with additional
>>>>>>>>>>>>>> testing case?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Well.. We can use next case: Co-Processor register candidates
>>>>>>>>>>>>> may be
>>>>>>>>>>>>> either in VFP or in stack, so when all VFP are allocated
>>>>>>>>>>>>> stack is
>>>>>>>>>>>>> used. We can use stack without GPR allocation in that case,
>>>>>>>>>>>>> passing 9 f64 params, for example: first eight goes to d0-d7,
>>>>>>>>>>>>> night goes to stack. Now as 10th parameter we pass i32,
>>>>>>>>>>>>> according
>>>>>>>>>>>>> to AAPCS, 5.5 Parameters Passing, Stage C, C.4:
>>>>>>>>>>>>>
>>>>>>>>>>>>> [quote]
>>>>>>>>>>>>> If the size in words of the argument is not more than r4 minus
>>>>>>>>>>>>> NCRN, the argument is copied into
>>>>>>>>>>>>> core registers, starting at the NCRN. The NCRN is incremented by
>>>>>>>>>>>>> the number of registers used.
>>>>>>>>>>>>> Successive registers hold the parts of the argument they would
>>>>>>>>>>>>> hold if its value were loaded into
>>>>>>>>>>>>> those registers from memory using an LDM instruction. The
>>>>>>>>>>>>> argument has now been allocated.
>>>>>>>>>>>>> [/quote]
>>>>>>>>>>>>>
>>>>>>>>>>>>> It must to R0 in our case. But currently it goes to stack.
>>>>>>>>>>>>> I also checked this case with GCC - it works as in AAPCS
>>>>>>>>>>>>> written.
>>>>>>>>>>>>>
>>>>>>>>>>>>> About refactoring. I propose do it in two stages, first rename
>>>>>>>>>>>>> "getVarArgsRegSaveSize" to "get/setByValRegsSaveSize", and
>>>>>>>>>>>>> detach
>>>>>>>>>>>>> "StoreByValRegs" functionality from "VarArgStyleRegisters"
>>>>>>>>>>>>> method.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I attached first refactor patch, Calling-Convention patch with
>>>>>>>>>>>>> test-case, and calling-convention .c test case.
>>>>>>>>>>>>> Try run .c file with "arm-linux-gnueabihf-gcc" and
>>>>>>>>>>>>> "./arm-linux-gnueabihf-clang -mcpu=cortex-a9".
>>>>>>>>>>>>>
>>>>>>>>>>>>> P.S.: My beagleboard is very very slow, "release" build takes
>>>>>>>>>>>>> about 15 hours, test-suite may be 15-20 hours too. I'll try ask
>>>>>>>>>>>>> somebody to present me faster device, but it take some time.
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Stepan.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Manman Ren wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Apr 12, 2013, at 5:15 AM, Stepan Dyatkovskiy wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hello Manman,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I reattached full patch with fixes.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 1. I've removedskipWastedByValRegs, addByValWastedRegs and
>>>>>>>>>>>>>>> Wasted flag.
>>>>>>>>>>>>>>> 2. Fixed usage of "offset" variable in LowerFormalArguments
>>>>>>>>>>>>>>> 3. And fixed regression test: added function with two byval
>>>>>>>>>>>>>>> structures.
>>>>>>>>>>>>>> The above looks good.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> *4.* One new change I've made is
>>>>>>>>>>>>>>> CC_ARM_AAPCS_Common rule.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Currently, for align == 4 and parameter type <=i32, we try to
>>>>>>>>>>>>>>> assign it to the first unallocated register from [r0-r3]
>>>>>>>>>>>>>>> range,
>>>>>>>>>>>>>>> but only if getNextStackOffset() == 0.
>>>>>>>>>>>>>>> I suppose that "getNextStackOffset() != 0" is treated as fact?
>>>>>>>>>>>>>>> that all registers are used? and params goes to the stack now.
>>>>>>>>>>>>>>> After byval structures will be implemented, the next case is
>>>>>>>>>>>>>>> possible:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> void @f(%i32_struct* byval, i32)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Here, we need to allocate stack for first parameter, but after
>>>>>>>>>>>>>>> we still have unallocated registers. And second parameter
>>>>>>>>>>>>>>> should be sent to r1. Current CC_ARM_AAPCS_Common
>>>>>>>>>>>>>>> implementation will send second parameter to the stack.
>>>>>>>>>>>>>>> Though, I'm still not sure about this change, just because may
>>>>>>>>>>>>>>> be there are more reasons to check nextStackOffset value.
>>>>>>>>>>>>>> Can we fix the above issue in a separate patch with additional
>>>>>>>>>>>>>> testing case?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> + // for byval parameters.
>>>>>>>>>>>>>>>>>> + {
>>>>>>>>>>>>>>>>>> + unsigned InRegsParamsCount =
>>>>>>>>>>>>>>>>>> State->getInRegsParamsCount();
>>>>>>>>>>>>>>>>>> + if (InRegsParamsCount) {
>>>>>>>>>>>>>>>>>> + unsigned RB, RE;
>>>>>>>>>>>>>>>>>> + State->getInRegsParamInfo(InRegsParamsCount-1, RB,
>>>>>>>>>>>>>>>>>> RE);
>>>>>>>>>>>>>>>>>> + assert(reg >= RE);
>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>> Is this only for assertion? Is this necessary?
>>>>>>>>>>>>>>>> Yes. I've just moved this check from "if" statement, since it
>>>>>>>>>>>>>>>> must be
>>>>>>>>>>>>>>>> true always.
>>>>>>>>>>>>>>> How do you think, do we need this assertion?
>>>>>>>>>>>>>> I don't really think it is necessary :)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I didn't present refactor patch, just because main patch
>>>>>>>>>>>>>>> didn't
>>>>>>>>>>>>>>> get LGTM community and hypothetically may contain some
>>>>>>>>>>>>>>> mistakes..
>>>>>>>>>>>>>> LGTM otherwise.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Manman
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -Stepan
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Stepan Dyatkovskiy wrote:
>>>>>>>>>>>>>>>> Manman Ren wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Stepan,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> One high-level question: do we need to insert the wasted
>>>>>>>>>>>>>>>>> record to
>>>>>>>>>>>>>>>>> ByValRegs and later on skip those records?
>>>>>>>>>>>>>>>>> It seems to me that we can only insert the records for the
>>>>>>>>>>>>>>>>> actual byval
>>>>>>>>>>>>>>>>> parameters.
>>>>>>>>>>>>>>>>> If that is true, we can remove addByValWastedRegs and
>>>>>>>>>>>>>>>>> skipWastedByValRegs.
>>>>>>>>>>>>>>>> Hm.. yes its reasonable. I'll try to fix.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>> + // "offset" value would be usefull if byval
>>>>>>>>>>>>>>>>>> parameter outsides
>>>>>>>>>>>>>>>>>> + // the registers area. Note, in that case
>>>>>>>>>>>>>>>>>> nextInRegsParam()
>>>>>>>>>>>>>>>>>> will not
>>>>>>>>>>>>>>>>>> + // skip any waste registers, since parameter
>>>>>>>>>>>>>>>>>> fillup
>>>>>>>>>>>>>>>>>> the whole
>>>>>>>>>>>>>>>>>> rest
>>>>>>>>>>>>>>>>>> + // of registers set.
>>>>>>>>>>>>>>>>>> + unsigned RegsUsed = RegEnd - RegBegin;
>>>>>>>>>>>>>>>>>> + if (!CCInfo.nextInRegsParam() &&
>>>>>>>>>>>>>>>>>> Flags.getByValSize() >
>>>>>>>>>>>>>>>>>> 4*RegsUsed )
>>>>>>>>>>>>>>>>>> + offset = RegEnd - RegBegin;
>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> - if (Flags.getByValSize() - 4*offset > 0) {
>>>>>>>>>>>>>>>>>> + if (offset) {
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> This still does not look right.
>>>>>>>>>>>>>>>>> There are 3 cases:
>>>>>>>>>>>>>>>>> If the byval parameter is in registers partially, we should
>>>>>>>>>>>>>>>>> create a
>>>>>>>>>>>>>>>>> COPY_STRUCT_BYVAL with the correct offset;
>>>>>>>>>>>>>>>>> If the byval parameter is all in registers, there is no need
>>>>>>>>>>>>>>>>> to create a
>>>>>>>>>>>>>>>>> COPY_STRUCT_BYVAL;
>>>>>>>>>>>>>>>>> If the byval parameter is all on stack, we should create a
>>>>>>>>>>>>>>>>> COPY_STRUCT_BYVAL with offset equal to 0.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> When looking at the above code, offset is default to
>>>>>>>>>>>>>>>>> zero, if
>>>>>>>>>>>>>>>>> CurByValIdx >= ByValArgsCount (this byval parameter is
>>>>>>>>>>>>>>>>> all on
>>>>>>>>>>>>>>>>> stack),
>>>>>>>>>>>>>>>>> offset will still be zero,
>>>>>>>>>>>>>>>>> and we will not create a COPY_STRUCT_BYVAL.
>>>>>>>>>>>>>>>>> The correct logic seems to be always updating offset inside
>>>>>>>>>>>>>>>>> the if and
>>>>>>>>>>>>>>>>> check Flags.getByValSize() > 4*offset
>>>>>>>>>>>>>>>>> if (CurByValIdx < ByValArgsCount) {
>>>>>>>>>>>>>>>>> …
>>>>>>>>>>>>>>>>> offset = RegEnd - RegBegin; //number of registers
>>>>>>>>>>>>>>>>> occupied
>>>>>>>>>>>>>>>>> by this
>>>>>>>>>>>>>>>>> byval parameter
>>>>>>>>>>>>>>>>> CCInfo.nextInRegsParam(); // go to the next byval
>>>>>>>>>>>>>>>>> parameter that is
>>>>>>>>>>>>>>>>> in registers partially or all
>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>> if (Flags.getByValSize() > 4*offset) {
>>>>>>>>>>>>>>>>> // Part of the byval parameter is on stack.
>>>>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>>> Oh.. I missed third case. Then we keep first variant here.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> - if ((!State->isFirstByValRegValid()) &&
>>>>>>>>>>>>>>>>>> - (ARM::R0 <= reg) && (reg <= ARM::R3)) {
>>>>>>>>>>>>>>>>>> + if ((ARM::R0 <= reg) && (reg <= ARM::R3)) {
>>>>>>>>>>>>>>>>>> + // All registers reserved for byval parameters should
>>>>>>>>>>>>>>>>>> be allocated.
>>>>>>>>>>>>>>>>>> + // Check that first unallocated register is higher
>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>> last one
>>>>>>>>>>>>>>>>>> used
>>>>>>>>>>>>>>>>>> + // for byval parameters.
>>>>>>>>>>>>>>>>>> + {
>>>>>>>>>>>>>>>>>> + unsigned InRegsParamsCount =
>>>>>>>>>>>>>>>>>> State->getInRegsParamsCount();
>>>>>>>>>>>>>>>>>> + if (InRegsParamsCount) {
>>>>>>>>>>>>>>>>>> + unsigned RB, RE;
>>>>>>>>>>>>>>>>>> + State->getInRegsParamInfo(InRegsParamsCount-1, RB,
>>>>>>>>>>>>>>>>>> RE);
>>>>>>>>>>>>>>>>>> + assert(reg >= RE);
>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>> Is this only for assertion? Is this necessary?
>>>>>>>>>>>>>>>> Yes. I've just moved this check from "if" statement, since it
>>>>>>>>>>>>>>>> must be
>>>>>>>>>>>>>>>> true always.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> -void
>>>>>>>>>>>>>>>>>> -ARMTargetLowering::VarArgStyleRegisters(CCState &CCInfo,
>>>>>>>>>>>>>>>>>> SelectionDAG
>>>>>>>>>>>>>>>>>> &DAG,
>>>>>>>>>>>>>>>>>> - DebugLoc dl,
>>>>>>>>>>>>>>>>>> SDValue &Chain,
>>>>>>>>>>>>>>>>>> - const Value
>>>>>>>>>>>>>>>>>> *OrigArg,
>>>>>>>>>>>>>>>>>> - unsigned
>>>>>>>>>>>>>>>>>> OffsetFromOrigArg,
>>>>>>>>>>>>>>>>>> - unsigned
>>>>>>>>>>>>>>>>>> ArgOffset,
>>>>>>>>>>>>>>>>>> - bool ForceMutable)
>>>>>>>>>>>>>>>>>> const {
>>>>>>>>>>>>>>>>>> +// Return: The frame index registers were stored into.
>>>>>>>>>>>>>>>>>> +int
>>>>>>>>>>>>>>>>>> +ARMTargetLowering::StoreByValRegs(CCState &CCInfo,
>>>>>>>>>>>>>>>>>> SelectionDAG &DAG,
>>>>>>>>>>>>>>>>>> + DebugLoc dl, SDValue
>>>>>>>>>>>>>>>>>> &Chain,
>>>>>>>>>>>>>>>>>> + const Value *OrigArg,
>>>>>>>>>>>>>>>>>> + unsigned
>>>>>>>>>>>>>>>>>> InRegsParamRecordIdx,
>>>>>>>>>>>>>>>>>> + unsigned
>>>>>>>>>>>>>>>>>> OffsetFromOrigArg,
>>>>>>>>>>>>>>>>>> + unsigned ArgOffset,
>>>>>>>>>>>>>>>>>> + bool ForceMutable)
>>>>>>>>>>>>>>>>>> const {
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> It would be much cleaner if we can move the refactoring of
>>>>>>>>>>>>>>>>> VarArgStyleRegisters to a separate patch.
>>>>>>>>>>>>>>>> For sure.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> P.S.: Today I had some troubles with electricity, so I'll try
>>>>>>>>>>>>>>>> present
>>>>>>>>>>>>>>>> new patch tomorrow.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> -Stepan.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>>>> llvm-commits mailing list
>>>>>>>>>>>>>>>> llvm-commits at cs.uiuc.edu
>>>>>>>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <pr15293-2013-04-12.patch>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> <stack-offset.c><VarArgsRegSaveSize-2013-04-16.patch><AAPCS-C.4-vs-VFP-2013-04-16.patch>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> <AAPCS-C.4-vs-VFP-2013-04-17.patch>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> <AAPCS-C.4-vs-VFP-2013-04-19.patch>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> <StoreByValRegs-2013-04-17.patch>
>>>>
>>>
>>
>> <StoreByValRegs-2013-04-28.patch>
>
More information about the llvm-commits
mailing list