RFR(S): 8146792: Predicate moved after partial peel may lead to broken graph

[Roland Westrelin]

Hi Vladimir,

Thanks for looking at this.

> Now I think I understand.
> Note, there should be NO any control between loop's head and predicate check. I assume CastPP is attached to it because its original check was removed by dominated similar check (for example NULL check).
> 
> I think it is safe to move CastPP above original dummy predicate checks (one or two checks if there is loop limit checks) since Cast PP should not depend on them. It will solve the problem since moved check(new predicate) is always inserted before original dummy predicate (which will be removed later).

I first saw that problem with a CastPP but in the test case that I wrote for that bug, the pinned node is not a CastPP, it’s a StoreF. The predicate that is moved above tests a LoadF value that is memory dependent on the StoreF. I don’t see any reason the same problem couldn’t be reproduced with any data node.

With the test case, it would be safe to move the StoreF above the predicates I think. But in the general case, I don’t see how we can be sure that we don’t have:

- null check/range check for the StoreF moved out of loops as predicates
- partial peel that causes the StoreF to be pinned below the predicates
- loop predication that moves some data node that depends on the StoreF above it

Roland.



> 
> Thanks,
> Vladimir
> 
> On 1/11/16 7:07 AM, Roland Westrelin wrote:
>> http://cr.openjdk.java.net/~roland/8146792/webrev.00/
>> 
>> - partial peeling is applied to a loop
>> - the peeled section is optimized and leaves a pinned node between the loop predicates and the loop body but no control flow
>> - loop predicates are applied and a predicate that depends on the pinned node is moved out of the loop, before the pinned node, leading to a broken graph
>> 
>> This is the same issue that came up during review of 8139771. Vladimir suggested it gets reviewed separately. With the included test case it reproduces without the change from 8139771.
>> 
>> Roland.
>>