Request for review (M): 6860469: remix_address_expressions sets incorrect control causing crash in split_if_with_block_post

Hiroshi Yamauchi yamauchi at google.com
Wed Jul 15 16:38:02 PDT 2009


Looks good to me as well.

Thanks!

On Wed, Jul 15, 2009 at 1:50 PM, Chuck Rasbold<rasbold at google.com> wrote:
> Looks good. Thanks.
>
> On Wed, Jul 15, 2009 at 1:37 PM, Tom Rodriguez <Thomas.Rodriguez at sun.com>
> wrote:
>>
>> http://cr.openjdk.java.net/~never/6860469 is what I'm going to push with
>> you and hiroshi marked as contributors.  I added the copyright and fixed the
>> test case run running with jtreg.  The class needed to be public and the
>> class name was missing from the @run line.
>>
>> tom
>>
>> On Jul 15, 2009, at 12:24 PM, Chuck Rasbold wrote:
>>
>>> I presume that Sun will add the copyright notice when the code is
>>> committed, and of course, I'm OK with that.
>>>
>>> IANAL, but I'm reluctant to add the notice in advance, mostly because I
>>> am not employed by Sun.
>>>
>>> On Wed, Jul 15, 2009 at 12:07 PM, Tom Rodriguez
>>> <Thomas.Rodriguez at sun.com> wrote:
>>> The test is missing the copyright notice but otherwise this looks good.
>>>  BTW, I ran a full ctw with this change both as is and as an assert that
>>> checked for cases where this would produce a different control and I only
>>> found 4 cases where it ever changed the answer and none of those triggered a
>>> failure in the way the test case does.  Hiroshi, thanks for tracking this
>>> down and Chuck, thanks for distilling a test case.
>>>
>>> tom
>>>
>>>
>>> On Jul 15, 2009, at 9:27 AM, Chuck Rasbold wrote:
>>>
>>> http://cr.openjdk.java.net/~rasbold/6860469/webrev.00
>>>
>>> Fixed 6860469: remix_address_expressions sets incorrect control causing
>>> crash in split_if_with_block_post
>>>
>>> Consult the control node of both inputs when deciding where to place
>>> new LShiftI node.
>>>
>>> Previously, we assumed the invar input should set the new node's
>>> control, and the scale input was irrelevant for control's sake.
>>> However, sometimes the invar input is a constant, allowing it to be
>>> higher in the dom tree than the scale.
>>>
>>> Fix provided by Hiroshi Yamauchi (yamauchi at google.com)
>>>
>>>
>>>
>>
>
>



More information about the hotspot-compiler-dev mailing list