RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Roland Westrelin-3

http://cr.openjdk.java.net/~roland/8186125/webrev.00/

Split if is missing support for graph shapes with the Opaque4Node that
was introduced for unsafe accesses by JDK-8176506.

In the test case, the 2 Unsafe accesses share a single Opaque4Node
before the if. When split if encounters the Cmp->Bol->Opaque4->If chain,
it only tries to clone Cmp->Bol when it should clone Cmp->Bol->Opaque4
to make one copy for each If.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Roland Westrelin-3

Anyone to review this fix?

Roland.

> http://cr.openjdk.java.net/~roland/8186125/webrev.00/
>
> Split if is missing support for graph shapes with the Opaque4Node that
> was introduced for unsafe accesses by JDK-8176506.
>
> In the test case, the 2 Unsafe accesses share a single Opaque4Node
> before the if. When split if encounters the Cmp->Bol->Opaque4->If chain,
> it only tries to clone Cmp->Bol when it should clone Cmp->Bol->Opaque4
> to make one copy for each If.
>
> Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Vladimir Kozlov
Can you explain (add comment) why it can be only CMove node?:

            if (use_c == blk1 || use_c == blk2) {
+              assert(use->is_CMove(), "unexpected node type");

Seems fine otherwise. I submitted testing.
What did you run to catch this problem?

Thanks,
Vladimir

On 10/30/17 10:02 AM, Roland Westrelin wrote:

>
> Anyone to review this fix?
>
> Roland.
>
>> http://cr.openjdk.java.net/~roland/8186125/webrev.00/
>>
>> Split if is missing support for graph shapes with the Opaque4Node that
>> was introduced for unsafe accesses by JDK-8176506.
>>
>> In the test case, the 2 Unsafe accesses share a single Opaque4Node
>> before the if. When split if encounters the Cmp->Bol->Opaque4->If chain,
>> it only tries to clone Cmp->Bol when it should clone Cmp->Bol->Opaque4
>> to make one copy for each If.
>>
>> Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Roland Westrelin-3

Thanks for reviewing this, Vladimir.

> Can you explain (add comment) why it can be only CMove node?:
>
>             if (use_c == blk1 || use_c == blk2) {
> +              assert(use->is_CMove(), "unexpected node type");

So either it's an If, a CMove or an Opaque1. That node is between the if
that we are going to split and the Region right above we are going to
split it through.

It can't be an If because then there would be some control flow between
the If to split and the Region which is impossible.

It can't be an Opaque1 because there can't be any control flow between
the If and the region (so no loop) and Opaque1 nodes are not commoned so
even if they are loops in the if branches, they would each have an
Opaque1 that should be assigned a control in the if branches.

Roland.

Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Roland Westrelin-3
In reply to this post by Vladimir Kozlov

> What did you run to catch this problem?

Forgot to answer that question: I think it was with the lucene test
suite.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Vladimir Kozlov
Thanks. I will add it to bug report.
Tests passed I will push changes.

Vladimir

On 11/9/17 6:56 AM, Roland Westrelin wrote:
>
>> What did you run to catch this problem?
>
> Forgot to answer that question: I think it was with the lucene test
> suite.
>
> Roland.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S): 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses

Roland Westrelin-3

> Thanks. I will add it to bug report.
> Tests passed I will push changes.

Thanks you for taking care of that?

Roland.