[PATCH] Allow tail call optimization through multiple/nested struct extractions/insertions

[Stephen Lin]

Hmm, in retrospect, this was probably not really worth it, but I
extended this to handle reordering and regrouping of struct elements
(as long as the original order and grouping is reestablished before
the call).

I know it doesn't seem all that likely to be useful (I'm guessing some
functional code with record types *might* be able to take advantage of
it in some corner cases): I just found the limitation initially when
making a new test and thought it would be very quick fix.

It ended up being a bit more subtle, actually; but the work is done
now and there's no apparent downside, so I figured I might as well
submit it for review. Also, since it was more subtle than I expected,
I added more plausible false positive cases involving combinations of
nesting, grouping, and undefined values, just to be as certain as
possible it's doing the right thing. If there's any more cases anyone
can think of to add to be safe, please let me know.

Any comment or suggestion is appreciated. Also, I'm sure there's some
degenerate cases this doesn't handle fully, but this seems like the
cleanest way of handling my originally planned test and other similar
cases without carrying excessive amounts of state around.

-Stephen

On Sat, Apr 20, 2013 at 12:30 AM, Stephen Lin <swlin at post.harvard.edu> wrote:
> Committed as r179924, with extra tests to ensure tail calls are not
> generated in some plausible false positive cases.
>
> On Fri, Apr 19, 2013 at 7:06 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>>
>> On Apr 19, 2013, at 10:31 AM, Stephen Lin <swlin at post.harvard.edu> wrote:
>>
>>>> This part confuses a bit:
>>>>
>>>> +static bool sameNoopInput(const Value *V1, const Value *V2,
>>>> +                          SmallVectorImpl<unsigned> &Els1,
>>>> +                          SmallVectorImpl<unsigned> &Els2,
>>>> +                          const TargetLowering &TLI) {
>>>> +  using std::swap;
>>>> +  bool swapParity;
>>>> +  bool isSame = sameNoopInput(V1, V2, Els1, Els2, swapParity, TLI);
>>>> +  if (swapParity) {
>>>> +    // Revert to origin Els1 and Els2 to avoid confusing recursive calls
>>>> +    swap(V1, V2);
>>>> +    swap(Els1, Els2);
>>>> +  }
>>>> +  return isSame;
>>>> +}
>>>>
>>>> Why is it necessary to swap V1 and V2? They are not passed by reference. Otherwise, I don't see any issue with the patch. Please update it and if LNT testing looks fine please commit the patch.
>>>>
>>>
>>> Oh, that's embarrassing :) I got rid of that and consolidated it back
>>> into one function.
>>>
>>> Yes, LNT looks fine. Also, I don't have commit access, actually, so
>>> could you commit for me?
>>
>> Please follow the instruction to request commit access first:
>> http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
>>
>> You should commit it. Please do be on the look out for test failures. Like I said, the patch looks good but tail call can be tricky.
>>
>> Evan
>>
>>>
>>> Thanks,
>>> Stephen
>>> <tail-call-nested-struct.patch>
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tail-call-reorder-struct.patch
Type: application/octet-stream
Size: 15193 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130420/09b535a7/attachment.obj>