RFR(S): 8028471: PPC64 (part 215): opto: Extend ImplicitNullCheck optimization.

[Lindenmaier, Goetz]

Hi Vladimir,

> Without (UseCompressedOops && Universe::narrow_oop_base() > 0) check 
> Universe::narrow_oop_use_implicit_null_checks() is useless because it is 
> 'true' by default. That is why I asked to move check inside to combine 
> them in one place.
Yes, it is useless, but it will not be evaluated.  And I need it if narrow_oop_base()
is set.
But is it really true by default? See arguments.cpp: 
#ifdef _WIN64
    if (UseLargePages && UseCompressedOops) {
      // Cannot allocate guard pages for implicit checks in indexed addressing
      // mode, when large pages are specified on windows.
      // This flag could be switched ON if narrow oop base address is set to 0,
      // see code in Universe::initialize_heap().
      Universe::set_narrow_oop_use_implicit_null_checks(false);
    }
#endif //  _WIN64

> You can use Matcher::gen_narrow_oop_implicit_null_checks() check 
> instead. 
No, that returns true if narrow_oop_use_complex_address() is set.
I must know whether the page before the heapbased heap is protected.
There should be Universe::heapbased_base_page_is_protected(), 
that would be much more descriptive.

> Note Universe::narrow_oop_base() != NULL only if 
> UseCompressedOops is true.
Yes, you mean I should remove the test for UseCompressedOops?

>> No ShouldNotReachHere for AIX.  I tried to improve the comment
>> instead.
> So you want to return false if it is not DecodeN. Right?
No, if it's not DecodeN_NN:

    xn = loadn
    xo = xn == 0 ? 0 : xn<<3+base  // DecodeN
    loadI (xo+offset)
will not trap, as it accesses the zero page.

    xn = loadn
    xo = xn<<3+base  // DecodeN_NN
    loadI (xo+offset)
will trap, as it accesses the base page of the heapbased heap.

If you match the decode into the memory operand of the second 
Load, this check will not work, therefore I put the unimplemented()
there:
    xn = loadn
    loadI (xo<<3+base+offset)
but ppc has no loads that support this.

There is no obvious, platform independent way to distinguish
DecodeN_NN and DecodeN. 

Thanks for this detailed review!
Best regards,
  Goetz.



Thanks,
Vladimir

>
> Sorry for the wrong indents.
>
> Best regards,
>    Goetz.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Wednesday, November 20, 2013 9:04 PM
> To: Lindenmaier, Goetz; 'hotspot-dev at openjdk.java.net'; 'ppc-aix-port-dev at openjdk.java.net'
> Subject: Re: RFR(S): 8028471: PPC64 (part 215): opto: Extend ImplicitNullCheck optimization.
>
> Hi, Goetz
>
> Can you move all checks in lcm.cpp into new method and rename it to
> needs_explicit_null_check_for_read()? So the condition will be simple:
>
> if (!was_store && needs_explicit_null_check_for_read(val)) {
>     continue;
> }
>
> You should also add ShouldNotReachHere() for AIX near Unimplemented()
> for non AIX.
>
> And, please, fix indents in new method.
>
> Thanks,
> Vladimir
>
> On 11/20/13 2:11 AM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> ImplicitNullChecks did not work on platforms where the zero
>> page is only write protected.
>>
>> This change adds os property 'zero_page_read_protected' and extends
>> the ImplicitNullCheck optimization to consider only stores if
>> this property is not true. If a decoded compressed oop will access the
>> guard page before the heap, Loads work again.
>> This is needed on AIX, where the page at address '0' is only write-protected.
>>
>> Please review and test this change:
>> http://cr.openjdk.java.net/~goetz/webrevs/8028471-0-zpage/
>>
>> I tagged this S, as a majority of the patches apply to the ppc platform files.
>>
>> Best regards,
>>     Goetz.
>>