RFR (L): 7153771: array bound check elimination for c1

[Tom Rodriguez]

optimistic_optim_trap_id is an odd name.  Maybe optimistic_range_check_id or since it's used more generally predicate_failed_trap_id?  Actually that appears to be a cluster of names.  Can you try to rationalize that a bit?

Maybe rename optimize_phase1 to optimize_blocks and optimize_phase2 to eliminate_null_checks?

The Assert instruction leaks memory, which I guess it ok.  Maybe there's some way it could rely on the CodeComments stuff to manage that storage?

Should this reuse C2 flags like RangeCheckElimination instead of introducing it's own?

c1_InstructionPrinter.cpp:

Maybe use [rc] instead of ! to indicate range checked instructions?

fix_next_instructions_block == fixup_block_pointers?

c1_ValueMap.cpp:

This is a duplicate of the check in GraphBuilder isn't it?  Could this be moved into Compilation maybe or some other shared location?

+  ciMethod * method = this->_gvn->compilation()->method(); 
+  bool optimistic = !TieredCompilation && method->method_data()->trap_count(Deoptimization::Reason_none) == 0;

Can you do a formatting cleanup of c1_RangeCheckElimination.?pp?  There's a lot of weird vertical whitespace and braces on the wrong line.  It's a little light on comments too.

Otherwise this looks ok.  Can you confirm that you are seeing roughly similar performance to the original work?

tom

On Mar 21, 2012, at 6:52 AM, Roland Westrelin wrote:

> Implements array bound check elimination for c1 as proposed in:
> http://www.christianwimmer.at/Publications/Wuerthinger09a/
> Code contributed by Thomas Würthinger.
> 
> http://cr.openjdk.java.net/~roland/7153771/webrev.00/
> 
> Roland.