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

[Tom Rodriguez]

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