review (M) for 7024475: loop doesn't terminate when compiled

[Vladimir Kozlov]

Add comment that guard is generated explicitly for main and post loops:

+   bool needs_guard = !cl->is_main_loop && !cl->is_post_loop();


+ if (bol->is_Bool() && bol->as_Bool()->_test._test == BoolTest::lt) {

should be:

+ if (bol->is_Bool() && bol->as_Bool()->_test._test ==
+                       cl->loopexit()->test_trip()) {

I will modify this code to skip cloned predicates for 7004535.

I think we need RFE to cleanup PrintOpto. I never used it since I thought it is 
duplicate of PrintCompilation.

And thank you for fixing find()! I also saw that it spend a lot of time in 
old_arena()->contains(n) when you have big graph. And we need this check only 
after Matcher, during Optimize we have only one arena. It would be nice to avoid 
call to noninlined contains() with a simple check. Also we pass VectorSets as 
references but inside find_recur() we take pointer, why not just pass pointers?

Why default value OSROnlyBCI is -1? I think it should be 0 since 0 should not be 
  used as compile_id. Otherwise it is confusing: based on the flag's comment it 
loops like you always exclude first OSR compilation.

Thanks,
Vladimir

Tom Rodriguez wrote:
> http://cr.openjdk.java.net/~never/7024475
> 
> 7024475: loop doesn't terminate when compiled
> Reviewed-by:
> 
> The code which replaces an empty loop with the final computed value
> only works correctly if the the loop is guaranteed to execute once
> since it replaces the conditionally executed loop body with a value.
> Loop transformations like peeling generally result in zero trip guards
> being generated but they aren't required to be there.  The fix is to
> make sure we have a guard by peeling the loop first.  This results in
> a small redundant test in some cases.  I added a check for obvious
> ones, which occurred in scimark SOR.  Main and post loops are
> constructed with zero trip guards so I assume one exists in those
> cases.  Tested with new test case, original failing program,
> refworkload and full CTW.
> 
> I also improved a few debugging features.  Node::find could get into
> an infinite loop walking debug_orig, so it should be check for a
> cycle.  I made some other efficiency improvements before I realized
> that the problem was a cycle.  I went ahead and kept those changes.
> IdealGraphPrinter should print out a few more debug fields and should
> dump the block structure if it exists.  I changed the printing after
> beautify loops so it only happens once instead of after each recursive
> call.  I added an OSROnlyBCI flag to control which OSR bcis can be
> compiled. I changed the assert in loop verification to allow verifying
> when the igvn worklist isn't empty.