RFR(M): 8073480: C2 should optimize explicit range checks

[Roland Westrelin]

Thanks for reviewing this Vladimir.

> In general this looks good.
> 
> There are a lot of BoolTest:: checks. May be we should add new static methods to BoolTest class.

What tests do you think should go in separate methods?

Simple tests like this:
bn->_test._test == BoolTest::le

Or complex tests like:
    in(1)->as_Bool()->_test._test != BoolTest::ne &&
    in(1)->as_Bool()->_test._test != BoolTest::eq &&
    in(1)->as_Bool()->_test._test != BoolTest::overflow &&
    in(1)->as_Bool()->_test._test != BoolTest::no_overflow;

?

> I see you use in several places (mostly in reroute_side_effect_free_unc()) igvn->transform() without overwriting initial node:
> 
> igvn->transform(use);
> 
> which may not be correct if for some reasons node is transformed. You need to do:
> 
> use = igvn->transform(use);

Right. Thanks for spotting that. But in the case of reroute_side_effect_free_unc, the nodes are connected together and then transformed so I should only need:

halt = igvn->transform(halt);

right?

> reroute_side_effect_free_unc() why clone uncommon trap and not use new Region node? You can pass flag to merge_uncommon_traps() ot indicate when Region node is available already.

reroute_side_effect_free_unc changes the type of the trap: it takes the unc call from the first if (most likely an Reason_unstable_if trap) and sets the reason to the reason of the unc for the middle if which, given the middle check is now restricted to a null check, should be Reason_null_check. Thinking more about this, I don’t think it’s required that we keep the null check reason for the middle unc for that machinery to work as expected but maybe keeping the null check can help when trying to diagnose why we hit a unc here?

Roland.

> 
> Thanks,
> Vladimir
> 
> On 3/12/15 10:34 AM, Roland Westrelin wrote:
>> Here is a new webrev for this:
>> 
>> http://cr.openjdk.java.net/~roland/8073480/webrev.01/
>> 
>> I took Vladimir’s comments into account (added test for null inputs in several places, strengthen the test to make sure a middle guard is a null check, renamed functions) and added code that look for a ConvI2L between the range check and a memory access that follows and annotate that ConvI2L with a tighter type so the movslq that Paul spotted are removed from the final code.
>> 
>> Roland.
>>