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

[Chuck Rasbold]

Let's follow the precedent.  Please correct.

If you are feeling generous, the Test in 6837094 could be fixed, too.

Thanks.

On Thu, Jul 16, 2009 at 11:42 AM, Tom Rodriguez <Thomas.Rodriguez at sun.com>wrote:

> Thanks for the explanation.  I'm happy to correct it if you like.
>
> tom
>
>
> On Jul 16, 2009, at 7:22 AM, Martin Buchholz wrote:
>
>  We're still working out legalities, but the precedent for legal notices
>> for a source file that is non-Sun-authored
>> is to not include the Sun copyright line
>>
>> Copyright 2009 Sun Microsystems, Inc.
>>
>> For example, here is a Google-authored file:
>>
>>
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/timsort/src/share/classes/java/util/TimSort.java.html<http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk7/timsort/src/share/classes/java/util/TimSort.java.html>
>>
>> (and files obtained from the public domain have no copyright line
>> inside their legal notice at all)
>>
>> It's no big deal either way, especially for tests,
>> as all contributors retain their rights under SCA.
>>
>> Martin
>>
>> On Wed, Jul 15, 2009 at 13:37, Tom Rodriguez <Thomas.Rodriguez at sun.com>
>> wrote:
>>
>>>
>>> http://cr.openjdk.java.net/~never/6860469<http://cr.openjdk.java.net/%7Enever/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<http://cr.openjdk.java.net/%7Erasbold/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)
>>>>
>>>>
>>>>
>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20090716/6fa4afcb/attachment.html