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