RFC: Tailcall Improvement

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: Tailcall Improvement

Arnold Schwaighofer
Here is a patch to improve argument lowering for tail calls. Before  
this patch all outgoing arguments were move to the stack slot where  
they would go on a normal function call and from there moved back to  
the tail call stack slot to prevent overwriting of values.
After this patch only arguments that source from callers arguments  
(formal_arguments) are lowered this way.

I moved some code that would otherwise be duplicated to a new  
function 'GetMemCpyWithFlags'.

Okay to commit?

regards arnold



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

tailcall-improvement.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Tailcall Improvement

Evan Cheng-2
1. IsPossiblyOverriddenArgumentOfTailCall() should be marked static?

2. Please start all function level comments this way:
// IsPossiblyOverriddenArgumentOfTailCall - Check if ...

3.
+        if (IsPossiblyOverriddenArgumentOfTailCall(Arg)){
+          // Copy from stack slots to stack slot of a tail called  
function. This
+          // needs to be done because if we would lower the arguments  
directly
+          // to their real stack slot we might end up overwriting  
each other.
+          // Get source stack slot.
+          Source = DAG.getConstant(VA.getLocMemOffset(),
+                                             getPointerTy());
Funky indentation?

4.
+          if (StackPtr.Val == 0)
+            StackPtr = DAG.getRegister(getStackPtrReg(),  
getPointerTy());

This should be a CopyFromReg which reads the stack pointer register.

5.

+        SDOperand Source;
+        if (IsPossiblyOverriddenArgumentOfTailCall(Arg)){
              ....
+        } else {
+          Source=Arg;
+        }

I prefer this:

           SDOperand Source = Arg;
           if (IsPossiblyOverriddenArgumentOfTailCall(Arg)) {
             ...
           }
6.
      if (!MemOpChains2.empty())
        Chain = DAG.getNode(ISD::TokenFactor, MVT::Other,
-                          &MemOpChains2[0], MemOpChains.size());
+                          &MemOpChains2[0], MemOpChains2.size());

This looks like a bug fix? Please check this part in first.

Thanks!

Evan


On Jan 8, 2008, at 5:04 AM, Arnold Schwaighofer wrote:

> Here is a patch to improve argument lowering for tail calls. Before  
> this patch all outgoing arguments were move to the stack slot where  
> they would go on a normal function call and from there moved back to  
> the tail call stack slot to prevent overwriting of values.
> After this patch only arguments that source from callers arguments  
> (formal_arguments) are lowered this way.
>
> I moved some code that would otherwise be duplicated to a new  
> function 'GetMemCpyWithFlags'.
>
> Okay to commit?
>
> regards arnold
>
>
> <tailcall-
> improvement.patch>_______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev