RFR(XS): 8147853: "assert(t->meet(t0) == t) failed: Not monotonic" with sun/util/calendar/zi/TestZoneInfo310.java

[Vladimir Kozlov]

Good fix.

Can we simple return type of in(EntryControl) phi's input in such case 
(backedge is top) without filter_speculative() and verification code 
under assert which is useless in this case, I think. We already have 
check for the counted loop, we only need to separate 
can_be_counted_loop() condition.

Thanks,
Vladimir

On 1/22/16 8:38 AM, Roland Westrelin wrote:
> During CCP, a Phi for the induction variable of a CountedLoop is processed repeatedly while the type of the backedge control is top so only the loop entry input is considered for computing the Phi’s type.
>
> The loop entry first has type int:1..3 so the Phi’s type is int:1..3
> then it has type int:1..4 so the Phi’s type is int:1..4
> then it has type int:1..5:www so the Phi’s type is int:1..5:www
> then it has type int:1..6:www so the Phi’s type is saturated to int:1..max-1:www
>
> The backedge control’s type is changed to non-top and the type of the Phi is recomputed. This time the special code for counted loop in PhiNode::Value():
>
>    CountedLoopNode* l = r->is_CountedLoop() ? r->as_CountedLoop() : NULL;
>    if (l && l->can_be_counted_loop(phase) &&
>        ((const Node*)l->phi() == this)) { // Trip counted loop!
>      // protect against init_trip() or limit() returning NULL
>      const Node *init   = l->init_trip();
>      const Node *limit  = l->limit();
>      const Node* stride = l->stride();
>      if (init != NULL && limit != NULL && stride != NULL) {
>        const TypeInt* lo = phase->type(init)->isa_int();
>        const TypeInt* hi = phase->type(limit)->isa_int();
>        const TypeInt* stride_t = phase->type(stride)->isa_int();
>        if (lo != NULL && hi != NULL && stride_t != NULL) { // Dying loops might have TOP here
>          assert(stride_t->_hi >= stride_t->_lo, "bad stride type");
>          const Type* res = NULL;
>          if (stride_t->_hi < 0) {          // Down-counter loop
>            swap(lo, hi);
>            return TypeInt::make(MIN2(lo->_lo, hi->_lo) , hi->_hi, 3);
>          } else if (stride_t->_lo >= 0) {
>            return TypeInt::make(lo->_lo, MAX2(lo->_hi, hi->_hi), 3);
>          }
>        }
>      }
>    }
>
>
> kicks in and it computes a type of: int:1..8:www. The type of the Phi was narrowed and the assert fires.
>
> I suggest we fix this by saturating the type of the Phi only once the type of the loop’s backedge is non top. This way, the special code for counted loop above has a chance to run and that should be enough to keep the types during CCP monotonic.
>
> http://cr.openjdk.java.net/~roland/8147853/webrev.00/
>
> Roland.
>