[LLVMdev] RFC: PowerPC tail call optimization patch

[Evan Cheng]

Thanks! Code looks much cleaner.

On Apr 24, 2008, at 10:40 AM, Arnold Schwaighofer wrote:

> The patch is in
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080421/061548.html
> .
>
> Okay to commit :)

Some comments. :-)

+static bool IsPossiblyOverwrittenArgumentOfTailCall(SDOperand Op,
+                                                    MachineFrameInfo  
* MFI) {
...

+       (FrameIdxNode = dyn_cast<FrameIndexSDNode>(Op.getOperand(1))) &&
+       (MFI->isFixedObjectIndex((FrameIdx = FrameIdxNode- 
 >getIndex()))) &&
+       (MFI->getObjectOffset(FrameIdx) >= 0))

I suggest to further refactor this. Perhaps add a utility static   
function in SelectionDAG called isFixedFrameObject. Then you can *not*  
embed the FrameIdx assignment in the test. :-)

Obligatory format nitpick below. :-)

  static void CheckDAGForTailCallsAndFixThem(SelectionDAG &DAG,
...
+      if (isMarkedTailCall)
+        if (Ret==NULL ||
+            !TLI.IsEligibleForTailCallOptimization(OpCall, OpRet,  
DAG)) {

How about?
     if (!isMarkedTailCall)
       continue;
     if (Ret == NULL ||
         !TLI.IsEligibleForTailCallOptimization(OpCall, OpRet, DAG)) {

This gets rid of one level of nesting.

Please commit after fixing these (and testing). Thanks.

Evan

>
>
> On Thu, Apr 24, 2008 at 7:35 PM, Arnold Schwaighofer
> <arnold.schwaighofer at gmail.com> wrote:
>> Another round :)
>>
>> I'll post the patch to llvm-commits cause i guess people might get
>> annoyed by my series of patches :).
>>
>> On Tue, Apr 22, 2008 at 10:34 PM, Evan Cheng <evan.cheng at apple.com>  
>> wrote:
>> .
>>>> +PPCTargetLowering::IsEligibleForTailCallOptimization(SDOperand  
>>>> Call,
>>>> ...
>>
>>> That's fine. Please break it into two parts and move the target
>>> independent part out.
>> Done - see CheckTailCallReturnConstraints() in TargetLowering. The
>> rest of the function is still target dependent. PowerPC currently can
>> not tail call optimize byval functions, no pic/got code for non-local
>> calls. On x86 no support for pic/got code on x86-64 for non-local
>> calls.
>>
>>
>>> I am not sure. Just thinking aloud. It would be nice if all of the
>>> common code can be handled by SelectionDAGISel.  For example, it  
>>> seems
>>> possible for SDIsel to determine which operands can be overwritten  
>>> and
>>> issued the copies there?
>>>
>>>
>> Yes the common code between x86/ppc could be moved to
>> SelectionDAGISel. See CheckDAGForTailCallsAndFixThem().
>>
>>> There seem to be quite a bit of duplicated code. Can you refactor?
>> Created LowerMemOpCallTo.
>>
>> -arnold
>>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev