llvm dev - Apr 2008 - [LLVMdev] LiveVariables/LiveInterval on huge functions

On Mon, 14 Apr 2008, [ISO-8859-1] Török Edwin wrote:
>> Another question to ask, is why that function became so large in the
>> first place [X86DAGToDAGISel::SelectCode(llvm::SDOperand)]
>> We have inline limits, don't we?
>
> most of functions called by SelectCode get a -30000 cost reduction
> because they are internal.
> Even if Caller.size() is 40000, the penalty is only 2000, thus we still
> have negative costs.
>
> Removing the /20 factor from here improves the situation a lot, however
> I think there is  a reason for /20, and it can't be just removed:
> InlineCost += Caller->size()/20;
This sounds like unanticipated fallout from Evan's recent tweaks of the 
inliner.  Evan, thoughts?

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/

On Apr 14, 2008, at 10:43 AM, Chris Lattner wrote:
> On Mon, 14 Apr 2008, [ISO-8859-1] Török Edwin wrote:
>>> Another question to ask, is why that function became so large in
the
>>> first place [X86DAGToDAGISel::SelectCode(llvm::SDOperand)]
>>> We have inline limits, don't we?
>>
>> most of functions called by SelectCode get a -30000 cost reduction
>> because they are internal.
>> Even if Caller.size() is 40000, the penalty is only 2000, thus we  
>> still
>> have negative costs.
>>
>> Removing the /20 factor from here improves the situation a lot,  
>> however
>> I think there is  a reason for /20, and it can't be just removed:
>> InlineCost += Caller->size()/20;
>
> This sounds like unanticipated fallout from Evan's recent tweaks of  
> the inliner.  Evan, thoughts?
Previously the inliner assign each basic block cost of 20. So this  
line is simply estimating the number of caller basic blocks. My tweak  
simply removed the number of basic blocks from the equation so the  
cost of a callee is simply number of instructions * 5. I don't think  
it should / would impact this case. Edwin, can you revert 49061 and  
48725 to see if they have any impact?

The -30000 cost reduction for internal function does seem excessive  
though.

Evan
>
>
> -Chris
>
> -- 
> http://nondot.org/sabre/
> http://llvm.org/

On Mon, 14 Apr 2008, Evan Cheng wrote:
>>  This sounds like unanticipated fallout from Evan's recent tweaks
of the
>>  inliner.  Evan, thoughts?
>
> Previously the inliner assign each basic block cost of 20. So this line is 
> simply estimating the number of caller basic blocks. My tweak simply
removed
> the number of basic blocks from the equation so the cost of a callee is 
> simply number of instructions * 5. I don't think it should / would
impact
> this case. Edwin, can you revert 49061 and 48725 to see if they have any 
> impact?
>
> The -30000 cost reduction for internal function does seem excessive though.
Right, so now the cost estimate of the function is much lower than it was 
before.  This isn't itself a problem, but it means that the 30000 bonus 
for being called at one callsite should also be reduced to match, seem 
reasonable?

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/

Evan Cheng wrote:
> On Apr 14, 2008, at 10:43 AM, Chris Lattner wrote:
>
>   
>> On Mon, 14 Apr 2008, [ISO-8859-1] Török Edwin wrote:
>>     
>>>> Another question to ask, is why that function became so large
in the
>>>> first place [X86DAGToDAGISel::SelectCode(llvm::SDOperand)]
>>>> We have inline limits, don't we?
>>>>         
>>> most of functions called by SelectCode get a -30000 cost reduction
>>> because they are internal.
>>> Even if Caller.size() is 40000, the penalty is only 2000, thus we  
>>> still
>>> have negative costs.
>>>
>>> Removing the /20 factor from here improves the situation a lot,  
>>> however
>>> I think there is  a reason for /20, and it can't be just
removed:
>>> InlineCost += Caller->size()/20;
>>>       
>> This sounds like unanticipated fallout from Evan's recent tweaks of
>> the inliner.  Evan, thoughts?
>>     
>
> Previously the inliner assign each basic block cost of 20. So this  
> line is simply estimating the number of caller basic blocks. My tweak  
> simply removed the number of basic blocks from the equation so the  
> cost of a callee is simply number of instructions * 5. I don't think  
> it should / would impact this case. Edwin, can you revert 49061 and  
> 48725 to see if they have any impact?
>   
Reverting 49061 does have an impact. I can compile the testcase withing
1.1G of memory, and memory usage was rising slowly (except for a quick
500M->1G jump).
The patch couldn't be reverted cleanly, I had to manually revert parts
of it.

Best regards,
--Edwin