RFR(S): 8231291: C2: loop opts before EA should maximally unroll loops

[Roland Westrelin]

Hi Vladimir,

>>> Why you added check in must_be_not_null()? It is used only in library_call.cpp and should not relate to this changes.
>>> Did you find some issues?
>> 
>> I hit a crash when doing some CTW testing because an argument to
>> must_be_not_null() was already known to be not null. I suppose it's
>> unrelated to that change and something changed in the libraries instead.
>
> That's interesting. I initially thought it's just to simplify the IR and 
> avoid keeping redundant null check until loop opts are over.

must_be_not_null() is there for correctness. For some intrinsics, we
know that some parameter is not null and so the intrinsic implementation
skips the null check. Without must_be_not_null() calls, we, for
instance, add a load from an oop whose type is maybe a null
pointer. After some transformation we may end up with a load from
null. For instance if we have the following code shape:

for (;;) {
  if (p == null) {
    ..
  }
  v = p.f; // intrinsic. Load without a null check
}

after unswitching:

if (p == null) {
  for (;;) {
    v = null.f; // crashes the compiler
  }
} else {
  for (;;) {
    v = p.f;
  }
}

> Doesn't it signal about a more general problem with 
> GraphKit::must_be_not_null()?
>
> What if a value becomes non-null after GraphKit::must_be_not_null()? Is 
> there a chance to hit the very same problem you observed?

The problem I hit is that if the value passed to must_be_not_null is
already known to be not null. The null branch of must_be_not_null
optimizes out at parse time and:

C->root()->add_req(halt);

adds top to the RootNode which causes some failures during matching
because top is unexpected from Root.

Running RootNode::Ideal() would clean it up but it's not called. If the
null branch becomes dead after parsing then, AFAICT, RootNode::Ideal()
would be executed and the same problem would not happen.

Roland.