RFR(M): 8223051: support loops with long (64b) trip counts

[Vladimir Ivanov]

>> src/hotspot/share/opto/callnode.cpp:
>>
>>     // If you have back to back safepoints, remove one
>>      if( in(TypeFunc::Control)->is_SafePoint() )
>>        return in(TypeFunc::Control);
>>
>> -  if( in(0)->is_Proj() ) {
>> +  // Transforming long counted loops requires a safepoint node. Do not
>> +  // eliminate a safepoint until loop opts are over.
>> +  if (in(0)->is_Proj() && !phase->C->major_progress()) {
>>
>> Can you elaborate on this a bit? Why elimination of back-to-back
>> safepoints cause problems during new transformation? Is it because you
>> need specifically a SafePoint because CallNode doesn't fit?
> 
> The issue is with a call followed by a SafePointNode. A call captures
> the state before the call but we would need the state after the call
> otherwise on a deoptimization we would re-executed the call.

Sorry, I don't get it. Normally JVM state associated with a call is a 
state right after the call returns. Do you mean there are cases when 
call has reexecute bit set and hence it has JVM state before the call 
associated with it?

Anyway, it's trivial to convert between 2 states (before and after) and 
we already do that in some places (e.g., late inline prepares JVM state 
for the parser based on the state associated with CallStaticJava node).

>> ===============
>> src/hotspot/share/opto/loopnode.cpp:
>>
>> +void PhaseIdealLoop::add_empty_predicate(Deoptimization::DeoptReason
>> reason, Node* inner_head, IdealLoopTree* loop, SafePointNode* sfpt) {
>>
>> Nothing actionable at the moment, but it's unfortunate to see more and
>> more code being duplicated from GraphKit. I wish there were a way to
>> share implementation between GraphKit, PhaseIdealLoop, and
>> PhaseMacroExpand.
> 
> Actually, there might be a way. In the valhalla repo, Tobias pushed a
> change to GraphKit so it's possible to build one with an igvn
> argument. So we could do this:
> 
>      JVMState* jvms = cloned_sfpt->jvms()->clone_shallow(C);
>      SafePointNode* map = cloned_sfpt->clone()->as_SafePoint();
>      map->set_jvms(jvms);
>      jvms->set_map(map);
>      GraphKit kit(jvms, &_igvn);
>      kit.set_control(inner_head->in(LoopNode::EntryControl));
> 
>      kit.add_empty_predicates(0);
> 
>      _igvn.replace_input_of(inner_head, LoopNode::EntryControl, kit.control());
>      _igvn.remove_dead_node(map);
> 
> instead of:
> 
>      if (UseLoopPredicate) {
>        add_empty_predicate(Deoptimization::Reason_predicate, inner_head, outer_ilt, cloned_sfpt);
>      }
>      if (UseProfiledLoopPredicate) {
>        add_empty_predicate(Deoptimization::Reason_profile_predicate, inner_head, outer_ilt, cloned_sfpt);
>      }
>      add_empty_predicate(Deoptimization::Reason_loop_limit_check, inner_head, outer_ilt, cloned_sfpt);
> 
> and the new PhaseIdealLoop::add_empty_predicate() wouldn't be needed
> anymore.
> 
> One thing to consider is that new nodes for predicates are added by
> GraphKit now and they are not registered with PhaseIdealLoop. It may not
> be a problem because peeling sets major_progress so no further loop opts
> will be applied in this round.
> 
> Anyway, if we wanted to pursue this further, I think it would make sense
> to push Tobias' patch first.
> 
> What do you think?

Wow, it looks very promising! I'm perfectly fine with addressing it later.

Best regards,
Vladimir Ivanov