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
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev