review (S) for 6930043: C2: SIGSEGV in javasoft.sqe.tests.lang.arr017.arr01702.arr01702.loop_forw(II)I

[Vladimir Kozlov]

Fine. But as we discussed you need to verify that invar.clone() does not have any side effects.

Thanks,
Vladimir

Tom Rodriguez wrote:
> That's what I get for doing edits just before generating the webrev.  I'd originally deleted the clone since it seems useless.  If it's invariant then cloning it doesn't change anything.  I chickened out at the last minuted and restored but didn't recompile.  I think I'll stick with deleting it.  I've regenerated the webrev.
> 
> tom
> 
> On Mar 12, 2010, at 3:05 PM, Vladimir Kozlov wrote:
> 
>> Tom,
>>
>> You removed lines 2225,2226 but where ctrl node comes from for next line?:
>>
>> 2243       ld_rng = (LoadRangeNode*)invar.clone(ld_rng, ctrl);
>>
>> Thanks,
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> http://cr.openjdk.java.net/~never/6930043
>>> 6930043: C2: SIGSEGV in javasoft.sqe.tests.lang.arr017.arr01702.arr01702.loop_forw(II)I
>>> Reviewed-by:
>>> The new loop predication code is missing logic to test that the
>>> initial value of the index is in range.  In many cases will be
>>> eliminated statically.  Tested with failing test.  Also tested that
>>> this new test doesn't affect the performance improvement we were
>>> seeing with scimark.
>>> src/share/vm/opto/loopTransform.cpp
>>> test/compiler/6930043/Test6930043.java
>