Request for reviews (M): 7070134: Hotspot crashes with sigsegv from PorterStemmer

[Tom Rodriguez]

On Jul 26, 2011, at 1:09 PM, Vladimir Kozlov wrote:

> http://cr.openjdk.java.net/~kvn/7070134/webrev
> 
> Fixed 7070134: Hotspot crashes with sigsegv from PorterStemmer
> 
> It is an other case of "6831314: C2 may incorrectly change control of type nodes". Loop predicate RCE upper_bound check matches an other dominating RangeCheck and split_if optimization moves loads to dominating test. In the bug case (step4() method) two code branches have the same loads from the same array (b[]) and they are combined and moved above array's index check and above lower_bound predicate check.
> 
> The fix is band-aid to not move data nodes which are attached to a predicate test to a dominating test. It is allowed to do that during loop peeling and loop predicates generation since they duplicate all checks. I also switched off predicate RCE optimization for counted loops with '!=' test since there is no guarantee that loop index will be in the range [init, limit) if init > limit.
> 
> Added regression test. Refwokload (x64) shows no affect.

Why are the fixes in IfNode::dominated_by and PhaseIdealLoop::dominated_by different?  IfNode just picks a different node but PhaseIdealLoop gives up.  Can't PhaseIdealLoop pick a safe node?

loopPredicate.cpp:

Is this line really needed?

!       Node* upper_bound_bol = rc_predicate(loop, ctrl, scale, offset, init, limit, stride, rng, true);
!       Node* upper_bound_bol = rc_predicate(loop, lower_bound_proj, scale, offset, init, limit, stride, rng, true);

I'm not against it since it's valid and maybe conceptually more correct but I'm unclear how it could have any effect on correctness.

tom

> 
> 
>