RFR: 8281429: PhiNode::Value() is too conservative for tripcount of CountedLoop

[Tobias Hartmann]

On Tue, 15 Mar 2022 16:02:54 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The type for the iv phi of a counted loop is computed from the types
> of the phi on loop entry and the type of the limit from the exit
> test. Because the exit test is applied to the iv after increment, the
> type of the iv phi is at least one less than the limit (for a positive
> stride, one more for a negative stride).
> 
> Also, for a stride whose absolute value is not 1 and constant init and
> limit values, it's possible to compute accurately the iv phi type.
> 
> This change caused a few failures and I had to make a few adjustments
> to loop opts code as well.

Looks good to me otherwise.

src/hotspot/share/opto/cfgnode.cpp line 1111:

> 1109:         const TypeInteger* stride_t = phase->type(stride)->isa_integer(l->bt());
> 1110:         if (lo != NULL && hi != NULL && stride_t != NULL) { // Dying loops might have TOP here
> 1111:           assert(stride_t->hi_as_long() == stride_t->lo_as_long(), "bad stride type");

Should we simply check `stride_t->is_con()` here?

src/hotspot/share/opto/cfgnode.cpp line 1127:

> 1125:                   julong uhi = static_cast<julong>(hi->lo_as_long());
> 1126:                   julong ulo = static_cast<julong>(lo->hi_as_long());
> 1127:                   julong diff = (uhi - ulo - 1) / (-stride_t->lo_as_long()) * (-stride_t->lo_as_long());

Suggestion:

                  julong diff = ((uhi - ulo - 1) / (-stride_t->lo_as_long())) * (-stride_t->lo_as_long());

src/hotspot/share/opto/cfgnode.cpp line 1142:

> 1140:                   julong uhi = static_cast<julong>(hi->lo_as_long());
> 1141:                   julong ulo = static_cast<julong>(lo->hi_as_long());
> 1142:                   julong diff = (uhi - ulo - 1) / stride_t->lo_as_long() * stride_t->lo_as_long();

Suggestion:

                  julong diff = ((uhi - ulo - 1) / stride_t->lo_as_long()) * stride_t->lo_as_long();

src/hotspot/share/opto/loopPredicate.cpp line 1069:

> 1067:                 inner_loop = inner_loop->_child;
> 1068:                 inner_head = inner_loop->_head->as_Loop();
> 1069:                 inner_head->verify_strip_mined(1);

Why did you remove the verification code?

src/hotspot/share/opto/loopnode.cpp line 850:

> 848:   const TypeInteger* lo = _igvn.type(init)->is_integer(bt);
> 849:   const TypeInteger* hi = _igvn.type(limit)->is_integer(bt);
> 850:   julong orig_iters;

Suggestion:

src/hotspot/share/opto/loopnode.cpp line 855:

> 853:   }
> 854:   assert(hi->hi_as_long() > lo->lo_as_long(), "no iterations?");
> 855:   orig_iters = hi->hi_as_long()  - lo->lo_as_long();

Suggestion:

  julong orig_iters = hi->hi_as_long() - lo->lo_as_long();

-------------

Marked as reviewed by thartmann (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7823