[9] RFR(S): 8174164: SafePointNode::_replaced_nodes breaks with irreducible loops

[Vladimir Kozlov]

Wow, so much changes.

Can you add _irreducible_loop field to CallGenerator instead of passing as argument?
Also instead of adding such field to 2 classes may be add it to base class GraphKit.

Thanks,
Vladimir

On 2/14/17 2:13 AM, Roland Westrelin wrote:
>
>> I assume test_helper() is inlined into test() in all these
>> cases. Right?
>
> Yes.
>
>> Why we do replacement on exit of method inlined() which not have any
>> code? Should we do it in Call node map before we inline and parse it?
>
> replaced nodes were introduced for this code pattern:
>
>     static void inlined1(Object o) {
>         A a = (A)o;
>     }
>
>     static void inlined2(Object o) {
>         inlined1(o);
>     }
>
>     static void inlined3(Object o) {
>         inlined2(o);
>     }
>
>     static void test(Object o) {
>         inlined3(o);
>         // use o that benefit from knowing it's of type A
>     }
>
> This works if we propagate the replaced nodes at the end of parsing.
>
>> Is not this bad (not inlining)?
>
> Sure it's a bad thing. But then again it works by accident because there
> happens to be a call to inlined(). What would be nice is that the
> virtual call be inlined with or without the call to inlined() (apply
> replaced nodes after the load?).
>
>> But I think it is smaller number of cases vs current fix. Also most
>> OSR methods are recompiled as normal nmethods so affect will be only
>> temporary.
>
> My test case is pretty simple and it results in irreducible
> loops. Replaced nodes were added for nashorn and I remember they had
> simple scripts with a loop that would only get OSR compiled. Anyway,
> here is the fix with a check for irreducible loops:
>
> http://cr.openjdk.java.net/~roland/8174164/webrev.01/
>
> Roland.
>