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

[Vladimir Kozlov]

There is no compilation bailout in Output() and I don't
want to add one. I think, to have the assert is enough.

And I don't think bailout limits are the problem.
We should finish code generation if the compilation passed
bailouts during optimizations and RA.

I agree that instead of simple doubling I can use something like this:

_nodes_limit = _nodes_limit + NodeLimitFudgeFactor +
                ncalls*3 + nloops*(OptoLoopAlignment-1)))

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
>