Request for Reviews (S): JDK-8003585 strength reduce or eliminate range checks for power-of-two sized arrays
Roland Westrelin
roland.westrelin at oracle.com
Mon Oct 5 10:51:31 UTC 2015
Here is a new webrev:
http://cr.openjdk.java.net/~roland/8003585/webrev.01/
Roland.
> On Oct 2, 2015, at 3:30 PM, Roland Westrelin <roland.westrelin at oracle.com> wrote:
>
> Hi Chris,
>
>> Thanks for picking it up! It mostly looks good to me. (Not a Reviewer)
>
> Thanks for looking at this again.
>
>> What I really needed with my earlier webrev was some instructions as to what test to write -- since the Java corelibs can come across this optimization a lot (e.g. HashMap), I didn't have a good idea of what kind of test really needs to be written.
>>
>> A couple of issues with this webrev:
>>
>> 1. In subnode.cpp, line 1346:
>>
>> 1344 } else if (_test._test == BoolTest::lt &&
>> 1345 cmp2->Opcode() == Op_AddI &&
>> 1346 cmp2->in(2)->find_int_con(1)) {
>> 1347 bound = cmp2->in(1);
>> 1348 }
>>
>> I think it should be
>> cmp2->in(2)->find_int_con(0) == 1
>> instead, because the value passed into this function is actually for a "fallback when no int constant is found". Passing the expected value (1) to it defeats the purpose.
>
> You’re right. Thanks for spotting that.
>
>> jint find_int_con(jint value_if_unknown) const {
>> const TypeInt* t = find_int_type();
>> return (t != NULL && t->is_con()) ? t->get_con() : value_if_unknown;
>> }
>>
>> 2. Formattign nitpick: could you please trim the spaces before the new's on lines 1368, 1369 and 1387
>
> Sure.
>
> I’ll send an updated webrev.
>
> Roland.
>
>>
>> Thanks,
>> Kris (OpenJDK username: krismo)
>>
>> On Wed, Sep 30, 2015 at 1:34 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:
>> I’m picking that one up. Here is a new webrev:
>>
>> http://cr.openjdk.java.net/~roland/8003585/webrev.00/
>>
>> The only change to c2 compared to the previous webrev is that ((x & m) u< m+1) is optimized the same way ((x & m) u<= m) is. Actually, I don’t think that C2 currently produces the ((x & m) u<= m) shape. The IfNode::fold_compares() logic produces the ((x & m) u< m+1) variant. I also added a test case to check the validity of the transformations and ran usual testing on the change.
>>
>> Roland.
More information about the hotspot-compiler-dev
mailing list