RFR(S): 8139771: Eliminating CastPP nodes at Phis when they all come from a unique input may cause crash

[Vladimir Kozlov]

The comment is wrong. I added it for next changes:

https://bugs.openjdk.java.net/browse/JDK-7004535

But later removed the change due to next bug but I did not updated the 
comment:

https://bugs.openjdk.java.net/browse/JDK-7068051

"The code added in 7004535 changes does not take into account that 
cloning/moving predicates below merge points invalidate jvm states 
recorded in corresponding uncommon traps. Phi nodes should be created 
for values referenced by predicate's uncommon traps when a predicate is 
cloned."

"Remove predicate cloning from loop peeling optimization and from split 
fall-in paths. Leave it in loop unswitching code which is safe. Don't 
allow split loop entry path in IGVN optimization for Phi nodes. And do 
not clone predicates below merge points in split-if optimization. Remove 
move_loop_predicate() and eliminate_loop_predicates() unused methods."

Thanks,
Vladimir

On 12/16/15 12:49 AM, Roland Westrelin wrote:
>>> For reference, current webrev:
>>>
>>> http://cr.openjdk.java.net/~roland/8139771/webrev.01/
>>>
>>>>> As you suggested I made CheckCastPP inherit from ConstraintCast. I also hit the following bug: one iteration of a loop is peeled which causes a CastPP to be pinned between the loop and the predicates. When a predicate that depends on the CastPP is moved out of the loop, it is moved above the CastPP. I fixed by marking all nodes that depend on a node pinned between a loop and the predicates as non loop invariant. I don’t think fixing it by moving the cast up above the predicates is a safe fix in general.
>>>>
>>>> Hmm. The test which depends on CastPP should be also peeled and it will dominate the test in main loop. If a test/predicate could be moved from main loop then it should be possible to use peeled one. What do you think?
>>>
>>> Let me take another look at this.
>>> Independently: so we never apply loop predication before peeling? Otherwise moving the peeled body before the loop predicate could be incorrect, right (predicates could have been moved out of the body before it’s peeled)?
>>
>> We never peel before predicates. Peeling does not know about them. The peeled iteration is placed between predicates and peeled loop head.
>
> The comment in PhaseIdealLoop::do_peeling() implies that the peeled iteration is above the predicates. We can apply loop  predication then peeling. If the peeled iteration is above the predicates, isn’t there a risk the peeled iteration is executed before a predicate it depends on for correctness?
>
> Roland.
>
>>
>> Vladimir
>>
>>>
>>> Roland.
>