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

[Tom Rodriguez]

Check out http://cr.openjdk.java.net/~never/6861513.

tom

On Jul 16, 2009, at 1:21 PM, Chuck Rasbold wrote:

> 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
>
> (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 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)
>
>
>
>
>
>