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]

It occurred to me later that it also side steps the handling of dead  
control if you do it early.  If the control of the cast has become top  
then we would replace it with the constant type instead of replacing  
it with top which doesn't seem right.

tom

On Aug 15, 2009, at 11:31 AM, Vladimir Kozlov wrote:

> 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