Request for reviews (XS): 6866651: Regression: simple int sum crashes jvm (build 1.6.0_14-b08 and 1.7.0-ea-b59)

[Vladimir Kozlov]

OK. You convinced me.

Vladimir

Tom Rodriguez wrote:
> 
> On Aug 14, 2009, at 5:10 PM, Vladimir Kozlov wrote:
> 
>>
>>
>> Tom Rodriguez wrote:
>>> I don't really like this code.  My understanding is that this would 
>>> only occur for nodes that are dead because of CCP.  A dead node can 
>>> only have
>>
>> It is not true. I thought the same before, then I added the check and hit
>> immediately several cases when the node is alive and its type is 
>> constant:
> 
> I saw that those were being handled as well but ignored them for this 
> discussion since they don't seem to motivate this change.  They would 
> work just fine without this change.  I don't think short circuiting the 
> transforms of those nodes is worth duplicating the logic.
> 
> tom
> 
>> - CastII #int:1 after Parse because _gvn.set_type_bottom(ccast) in
>>  Parse::adjust_map_after_if().
>> - ConvI2L #long:1 in ConvI2LNode::Ideal() during next transform
>>  phase->transform( new (phase->C, 2) ConvI2LNode(y, 
>> TypeLong::make(rylo, ryhi, widen)) )
>>  because y = #int:1
>>
>> Vladimir
>>
>>> dead users so doesn't calling add_users_to worklist on a dead node 
>>> seem like a bad idea?  If we want dead nodes from CCP to be cleaned 
>>> up then shouldn't CCP be doing it?
>>> tom
>>> On Aug 14, 2009, at 3:46 PM, Vladimir Kozlov wrote:
>>>> I asked for it to short cut transformations when node's type is known
>>>> const or top. Yes, it is not needed for the bug fix but it could
>>>> reduce time spent in igvn.optimize().
>>>>
>>>> Vladimir
>>>>
>>>> Tom Rodriguez wrote:
>>>>> Why are you including the first part in this fix?  I don't see the 
>>>>> motivation for that change.
>>>>> tom
>>>>> On Aug 14, 2009, at 2:50 PM, changpeng fang - Sun Microsystems - 
>>>>> Santa Clara United States wrote:
>>>>>> http://cr.openjdk.java.net/~cfang/6866651/webrev.00/
>>>>>>
>>>>>> Fixed 6866651: Regression: simple int sum crashes jvm (build 
>>>>>> 1.6.0_14-b08 and 1.7.0-ea-b59)
>>>>>>
>>>>>> Problem:
>>>>>> set_req_X will do dead code elimination if the original input has 
>>>>>> no other use. However, it is possible to
>>>>>> have the current node (this) removed if a dead loop exists.  So,  
>>>>>> dead code elimination in set_req_X is not
>>>>>> safe, and may  cause undesired consequences (like the segfault in 
>>>>>> 6866651)
>>>>>>
>>>>>> Solution:
>>>>>> Instead of doing dead code elimination immediately in set_req_X, 
>>>>>> we put the dead node (original input) into
>>>>>> the worklist, and the dead node will eventually be eliminated when 
>>>>>> it is its turn to be processed.
>>>>>>
>>>>>> In addition,  before ideal transformations, we replace a node with 
>>>>>> a constant if we know it computes a constant
>>>>>> to skip unneeded optimizations on this node. Tests: JPRT, 
>>>>>> CompileTheWorld, Test case in bug report (Test.java in webrev)
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Changpeng
>