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

[Vladimir Kozlov]

I am not familiar with C1 so my questions mostly general.

Did C1 compilation time increased? And how much?

We usually generate assert checks for debug builds only to get 
'optimized' build without asserts. Also AssertRangeCheckElimination is 
develop flag. So could you used #ifdef ASSERT instead of #ifndef PRODUCT 
for Assert operatioins?

Why in RangeCheckStub::emit_code() there is no ce->verify_oop_map(_info) 
in new code (in both sparc and x86 files )?
Also ImplicitNullCheckStub::emit_code() on sparc has verify_oop_map() 
call but x86 does not.

c1_LIRAssembler_sparc.cpp spacing: remove one empty line before new code 
and add new line after the code.

c1_Runtime1_sparc.cpp why set_info() message is "incompatible_class_cast"?

c1_Compilation.cpp the orders in TimerName and timer_name[] are not 
matching.

Why you removed ResourceMark? Is it because resource area is not used? 
Or some resource allocated objects are used after that code? How it is 
related to ValueStack objects?:

    if (UseGlobalValueNumbering) {
-    ResourceMark rm;
+    // No resource mark here! LoopInvariantCodeMotion can allocate 
ValueStack objects.


Why not use Deoptimization::Reason_range_check? You use Reason_none.

c1_GraphBuilder.cpp has repeated pattern (could be factored out):

   (is_bb || compilation()->is_optimistic()) ? copy_state_before() : NULL

Code style: open "{" should on for() line:

+      for (int j = 0; j < loop_header->number_of_preds(); j++)
+      {

c1_Instruction.hpp how you get float/double compares for range check?:

+  // unordered_is_true is valid for float/double compares only

c1_RangeCheckElimination.cpp, why you do RCE when 
AssertRangeCheckElimination is set but there are no arrays references in 
a loop? Is it typo and you wanted it for StressRangeCheckElimination 
instead? :

   46 void RangeCheckElimination::eliminate(IR *ir) {
   47   bool do_elimination = ir->compilation()->has_access_indexed();
   48   ASSERT_RANGE_CHECK_ELIMINATION(do_elimination = true);
   49   if (do_elimination) {
   50     RangeCheckEliminator rce(ir);
   51   }
   52 }
   53

I will look more on code in c1_RangeCheckElimination.cpp later.

Thanks,
Vladimir

On 2/20/13 8:51 AM, Roland Westrelin wrote:
> Here is a new version of the webrev that addresses Tom's comments.
>
> http://cr.openjdk.java.net/~roland/7153771/webrev.01/
>
>> Should this reuse C2 flags like RangeCheckElimination instead of introducing it's own?
>
> I only reused RangeCheckElimination. I wasn't sure which others it made sense to re-use.
>
> Roland.
>