Request for reviews (S): 6860599: Relax nodes limit check for Output phase

[Vladimir Kozlov]

Tom Rodriguez wrote:
>>
>> I agree that instead of simple doubling I can use something like this:
>>
>> _nodes_limit = _nodes_limit + NodeLimitFudgeFactor +
>>               ncalls*3 + nloops*(OptoLoopAlignment-1)))
> 
> Can't you just a put in a check_node_count call before we start Output 
> to check that we have this margin left?

I am too greedy :)
I wanted a method to be compiled as it happens in product version
but then I realized that we bailout from Macro expansion phase
at the same situation. So it is fine to bailout from Output also.
Yes, I will use check_node_count to bailout from Output.

Thanks,
Vladimir

> 
> tom
> 
>>
>> Thanks,
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> On Jul 20, 2009, at 12:53 PM, Vladimir Kozlov wrote:
>>>> Tom,
>>>>
>>>> I do not want disable the assert since it may catch a situation
>>>> when something goes wrong in Output(), I want relax it.
>>> Oh.  Doubling it seems like disabling it to me.  Relaxing it would be 
>>> increasing it slightly or increasing the limit based on the amount we 
>>> actually expect it to increase by.  Isn't the real problem that the 
>>> bailout limits aren't triggering when they should?  I'd rather see 
>>> proper bailout bounds instead of just moving the MaxNodeLimit around.
>>> tom
>>>>
>>>> I can use an other flag instread of doubling:
>>>>
>>>>  void increase_nodes_limit() { _nodes_limit = CodeGenMaxNodeLimit; }
>>>>
>>>> And I can use nodes_limit() only in Node::verify_construction()
>>>> if everybody don't want changes in other places where MaxNodeLimit
>>>> is used.
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> Tom Rodriguez wrote:
>>>>> If the intent is just to suppress the assert after a certain point 
>>>>> in code generation, why don't you just add a flag in the Compile 
>>>>> for that?  Adding _node_limit and just doubling it seems arbitrary 
>>>>> and doesn't really capture intent.
>>>>> tom
>>>>> On Jul 20, 2009, at 9:32 AM, Vladimir Kozlov wrote:
>>>>>> Christian,
>>>>>>
>>>>>> The only nodes limit check done in Output is in
>>>>>> Node::verify_construction() and it is assert()
>>>>>> which only works in debug mode.
>>>>>> Did I understand your question correctly?
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>>
>>>>>> Christian Thalinger wrote:
>>>>>>> Vladimir Kozlov wrote:
>>>>>>>> http://cr.openjdk.java.net/~kvn/6860599/webrev.00
>>>>>>>>
>>>>>>>> Fixed 6860599: Relax nodes limit check for Output phase
>>>>>>>>
>>>>>>>> Problem:
>>>>>>>> I got several CTW cases when without EA C2 "gracefully"
>>>>>>>> bailout compilation when nodes limit check failed during
>>>>>>>> macro nodes expansion. And with EA it passed macro nodes
>>>>>>>> expansion but crashed with ASSERT during Output phase.
>>>>>>>> One byte MachNop nodes are used in debug mode for loops
>>>>>>>> and calls alignment in Output phase. As result for a big
>>>>>>>> method the node limit could be reached.
>>>>>>>>
>>>>>>>> Solution:
>>>>>>>> Increase nodes limit (double) for Output phase.
>>>>>>> What I don't understand with this patch is, it changes the node 
>>>>>>> limit
>>>>>>> but this is done for every output.  It's not limited to e.g. 
>>>>>>> debug mode.
>>>>>>> Is this what you indented?
>>>>>>> -- Christian
>