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

[Roland Westrelin]

Thanks for taking a look at this, Vladimir.

I'm ok with all your suggestions for which I don't make any comment below and will apply them.

> Did C1 compilation time increased? And how much?

I will send some measurements later.

> 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 operations?

Sure.

> 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.

I'll add ce->verify_oop_map(_info) to the new code and ImplicitNullCheckStub::emit_code().

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

Bad copy/paste. Will fix it.

> 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.

Compilation::current()->arena() is Thread::current()->resource_area() and new value stacks allocated during loop invariant code motion are used after this pass.

> c1_Instruction.hpp how you get float/double compares for range check?:
> 
> +  // unordered_is_true is valid for float/double compares only

The assert framework is supposed to be useful for things beyond range checks even if it's not the case right now.

> 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? :

That was part of the contribution and I assume a way to stress test the code. StressRangeCheckElimination is something I added to check that debug infos are correctly produced at the points where a deopt can be triggered.

> The research paper said about 55% improvement for scimark.LU. You got 18.7%.

If I remember correctly I observed huge perf increase with this code on scimark when support for OSR was used. The code that I sent for review doesn't include OSR support which is big and complex and that we decided to not keep.
Anyway, I'll run the experiment that you suggest.

> My main complain about code in c1_RangeCheckElimination.cpp is assignments inside condition checks:
> 
> if (c = lo->x()->as_Constant()) {
> 
> if ((op2 = v->as_Op2())) {
> 
> and others. Is it normal coding style in C1?

It's not. I'll get rid of them.

> Did you run JPRT and compiler regression tests?

I ran ctw, compiler, runtime, gc, java/lang reg tests, vm.jit.testlist, vm.regression.testlist, nsk.regression.testlist, nsk.stress.testlist, nsk.monitoring.testlist. I actually found a bug with this testing that I didn't hit last year when I sent the webrev the first time.
Does it make sense to run JPRT anyway?

Roland.