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

[Vladimir Ivanov]

>> I think the right way to fix the bug is to transform HaltNode after it
>> is added to RootNode (e.g., as done in AllocateArrayNode::Ideal() [1]).
>> In case HaltNode is eliminiated, RootNode should be put on worklist.
> 
> AFAICT, that would not work. PhaseGVN::transform() doesn't change uses
> of the node being transformed. The RootNode would need to be explicity
> pushed on the igvn worklist with a call to
> Compile::record_for_igvn(). But with the change to must_be_not_null(),
> there's no need for that. So I don't see anything that need fixing to be
> honest.

Yes, you are right. PhaseGVN::transform() doesn't register existing 
users for IGVN and the only way to make it work is to explicitly 
register RootNode when a HaltNode goes dead.

I understand that the change you have is enough to avoid the problematic 
case in GraphKit::must_be_not_null(). What I'd like to get rid of is the 
need for a correctness check to ensure that HaltNode can't go away. It's 
error-prone and can eventually get out of sync.

If you think it doesn't worth the effort, feel free to proceed with the 
current fix and I'll file a followup RFE.

Also, I'd like the bug to be handled separately (from 8231291). It'll 
simplify possible backporting: as I see from the history, the 
problematic code (JDK-8176506) was introduced in 10 and it may show up 
in 11u eventually.

>> I quickly looked through other places with the same idiom and
>> MemNode::Ideal_common() looks suspicious [3]. I don't see how it can
>> suffer from the same problem, but it would be nice to fix it as well.
>> I'm fine with filing a separate bug for it.
> 
> For the Halt node to be top, ctl would need to be top but that's handled
> before or frame would need to be top which doesn't seem possible so I
> don't see a problem there.

My point is it would be nice to avoid the need in such reasoning 
(especially for correctness purposes) in the first place.

Best regards,
Vladimir Ivanov