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

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Tue Aug 18 15:08:30 UTC 2020


>> http://cr.openjdk.java.net/~roland/8223051/webrev.03/

Looks good! Thanks a lot for taking care of it!

Some minor comments:

===============
src/hotspot/share/opto/callnode.cpp:

+      Node* new_in = old_sosn->clone(sosn_map);
+      if (old_unique != C->unique()) { // New node?

It's not a correctness issue, but strictly speaking it checks whether 
new nodes were allocated or not. It would be clearer to add a flag to 
SafePointScalarObjectNode::clone(Dict*) which signals that the returned 
node comes from the cache. Or just check that "new_in->_idx >= C->unique()".

I see that the code comes from macro.cpp, but IMO it's a good 
opportunity to clean it up a bit.

===============
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?

===============
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.

Best regards,
Vladimir Ivanov


>>
>> diff from previous patch:
>> http://cr.openjdk.java.net/~roland/8223051/webrev.02-03/
> 


More information about the hotspot-compiler-dev mailing list