[PATCH] fix for PR16393: miscompile with struct byval
Manman Ren
mren at apple.com
Wed Jul 3 12:15:16 PDT 2013
On Jul 3, 2013, at 9:05 AM, Stepan Dyatkovskiy wrote:
> Hi Manman,
>
> After some learning what exactly happens. IMHO, your decision is the *only* way (that couldn't be called too hackish or insane :-)).
Have you tried other solutions?
>
> Though, may be we add SpAdj just as property for MachineBasicBlock class?
Compared to using a pseudo SET_SP_ADJ, adding SPAdj as a property for MBB at least solves the issue of making sure SPAdj is available at the
beginning of a basic block.
I am not so sure whether it works when we have transformations on MBB.
We also need to reset SPAdj when we try to eliminate call frame pseudo instructions in calculateCallsInformation.
Thanks,
Manman
>
> I've modified your patch, please find it in attachment.
>
> -Stepan.
>
> Manman Ren wrote:
>>
>> For a call site with struct byval, we will generate a sequence of instructions: STACKDOWN, STRUCT_BYVAL and STACKUP.
>> STRUCT_BYVAL later on is expanded to a for loop. This caused issue in PEI when trying to eliminate frame indices.
>>
>> BB1:
>> STACKDOWN
>> BB2:
>> memcpy
>> BB3:
>> STACKUP
>>
>> PEI currently assumes SPAdj is 0 at the beginning of a basic block, but this is not true for BB2 or BB3. The proposed patch tries
>> to fix the problem by inserting SET_SP_ADJ in BB2 and BB3:
>> BB1:
>> STACKDOWN
>> BB2:
>> SET_SP_ADJ
>> memcpy
>> BB3:
>> SET_SP_ADJ
>> STACKUP
>>
>> A target-independent opcode SET_SP_ADJ is introduced, and pseudo instruction STRUCT_BYVAL is modified to take one extra
>> argument (the amount of SP Adjustment). When we are expanding STRUCT_BYVAL to a for loop, we add SET_SP_ADJ to BB2 and BB3.
>>
>> PEI is also modified to handle SET_SP_ADJ. To make sure SET_SP_ADJ is always before any spill code that references frame indices,
>> SET_SP_ADJ is treated as a label.
>>
>> SET_SP_ADJ can be useful for other targets when STACKDOWN and STACKUP are not paired up in the same basic block.
>>
>> Comments and other suggestions are welcome.
>>
>> Thanks,
>> Manman
>>
>
> <2013-06-03-byval-spadj.patch>
More information about the llvm-commits
mailing list