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

Evan Cheng evan.cheng at apple.com
Fri Apr 19 09:24:57 PDT 2013


On Apr 19, 2013, at 7:43 AM, Stephen Lin <swlin at post.harvard.edu> wrote:

> On Fri, Apr 19, 2013 at 10:42 AM, Stephen Lin <swlin at post.harvard.edu> wrote:
>> On Fri, Apr 19, 2013 at 3:09 AM, Stephen Lin <swlin at post.harvard.edu> wrote:
>>> No problem, I managed to get rid of most of the recursion, actually
>>> (see updated patch); it was an interesting exercise. The only
>>> recursion left is to handle extractions and insertions...I could
>>> probably do something with that too but it requires a lot more book
>>> keeping. Do you think it's worth it?
>>> 
>>> I've added tests to CodeGen/X86/tailcall-64.ll to test for the new
>>> positive cases. I thought of adding cases to verify that there aren't
>>> any false positives because I had a hard time thinking of meaningful
>>> false positive cases, but I can try harder to think of some. I also
>>> didn't add tests for other targets since this is all
>>> target-independent, but I don't mind doing that if you think it's
>>> worthwhile.
>>> 
>>> The entire LNT nightly test suite passes, except for an execution time
>>> limit on one test, which was already failing for me before this. Is
>>> there anything else I should test?
>>> 
>>> Stephen
>>> 
>> 
>> I've cleaned up the code a bit more.
>> - Stephen
> 
> (Would be nice to attach the patch, of course.)
> <tail-call-nested-struct.patch>

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.

Thanks,

Evan



More information about the llvm-commits mailing list