llvm dev - Apr 2008 - [LLVMdev] RFC: PowerPC tail call optimization patch

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

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

Okay to commit :)

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
>

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