C2: cyclic IR for JVMS

[Vladimir Kozlov]

Did you find why "the call node is live, though the loop was dead"?
I think I assumed that calls are safe - can't be in dead loop.
Your fix is reasonable but, first, fins why call is alive.

Thanks,
Vladimir

On 12/19/14 7:46 AM, Vladimir Ivanov wrote:
> So, if return value projection is not considered dead loop safe [1], the
> problem goes away.
>
> I'm curious whether it was an overlook in 6625997 [1] [2].
>
> Best regards,
> Vladimir Ivanov
>
> [1]
> diff --git a/src/share/vm/opto/multnode.hpp
> b/src/share/vm/opto/multnode.hpp
> --- a/src/share/vm/opto/multnode.hpp
> +++ b/src/share/vm/opto/multnode.hpp
> @@ -68,7 +68,7 @@
>     {
>       init_class_id(Class_Proj);
>       // Optimistic setting. Need additional checks in
> Node::is_dead_loop_safe().
> -    if (con != TypeFunc::Memory || src->is_Start())
> +    if ((con != TypeFunc::Memory && con != TypeFunc::Parms) ||
> src->is_Start())
>         init_flags(Flag_is_dead_loop_safe);
>       debug_only(check_con());
>     }
>
> [2] https://jbs.oracle.com/bugs/browse/JDK-6625997
> [3]
> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/diff/72f4a668df19/src/share/vm/opto/multnode.hpp
>
>
> On 12/18/14 10:11 PM, Vladimir Ivanov wrote:
>> Roland,
>>
>> The code is reachable by the time inlining occurs, but the call node is
>> broken.
>>
>> The transformation which breaks the code is performed in
>> RegionNode::Ideal()
>> Phi (Region) (Proj #5 (CallStaticJava ... Phi(backref)...))
>> ==>
>> (Proj #5 (CallStaticJava ... Proj(backref)...))
>>
>> The RegionNode and Phi are removed, but CallStaticJava is broken.
>>
>> It seems that if RegionNode::is_unreachable_region() detected that
>> RegionNode is dead, it would break the vicious circle (PhiNode is
>> replaced by top()).
>>
>> Not sure why the call node is live, though the loop was dead.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 12/18/14, 8:25 PM, Roland Westrelin wrote:
>>>> As my investigations show, it's a leftover from a loop.
>>>> CallStaticJava represents loop counter increment.
>>>>
>>>> The graph is as follows:
>>>> Phi (Region) (Proj #5 (CallStaticJava ... Phi(backref)...))
>>>>
>>>> The Region is dead, but RegionNode::is_unreachable_region doesn't
>>>> detect that. Node::simple_data_loop_check(Phi, Proj) doesn't find the
>>>> cycle, because most of Proj nodes have Flag_is_dead_loop_safe set [1].
>>>> And I don't understand why it is so. Is the check in
>>>> Node::is_dead_loop_safe() too weak or Flag_is_dead_loop_safe too
>>>> "optimistic"? :-)
>>>
>>> Isn’t the problem that you try to late inline a call that is dead and
>>> that the checks in LateInlineCallGenerator::do_late_inline():
>>>
>>>    if (call == NULL || call->outcnt() == 0 ||
>>>        call->in(0) == NULL || call->in(0)->is_top()) {
>>>      return;
>>>    }
>>>
>>>    const TypeTuple *r = call->tf()->domain();
>>>    for (int i1 = 0; i1 < method()->arg_size(); i1++) {
>>>      if (call->in(TypeFunc::Parms + i1)->is_top() &&
>>> r->field_at(TypeFunc::Parms + i1) != Type::HALF) {
>>>        assert(Compile::current()->inlining_incrementally(), "shouldn't
>>> happen during parsing");
>>>        return;
>>>      }
>>>    }
>>>
>>>    if (call->in(TypeFunc::Memory)->is_top()) {
>>>      assert(Compile::current()->inlining_incrementally(), "shouldn't
>>> happen during parsing");
>>>      return;
>>>    }
>>>
>>> don’t detect it?
>>> FWIW, I know there are rare cases where those tests are not
>>> sufficient. Ideally, you would need  Compile::inline_incrementally()
>>> to do:
>>> PhaseIdealLoop ideal_loop( igvn, false, true );
>>> after every inline but that’s very expensive.
>>>
>>> Roland.
>>>