RFR(S): 8028471: PPC64 (part 215): opto: Extend ImplicitNullCheck optimization.
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu Nov 21 12:45:02 PST 2013
Okay. I will push it after sync is done.
Thanks,
Vladimir
On 11/21/13 12:39 PM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> ok, I see, narrow_oop_use_implicit_null_checks() is set as one would
> expect: at most true in heapbased mode.
>
> I removed the check for UseCompressedOops.
>
> In our VM, narrow_oop_use_implicit_null_checks() can be true in unscaled
> mode. I didn't contribute that as it requires too much shared changes.
>
>> My question was, is that what you want for AIX? Because you have next
>> comment:
>> "On AIX, no such memory operands exist, so we need not check for them."
> With 'need not', I mean that I must not implement it. On other platforms,
> a lot of opportunities for null checks could be lost, so I put the Unimplemented()
> there. I adapted the comment:
> // returns true everywhere else. On PPC, no such memory operands
> // exist, therefore we did not yet implement a check for such operands.
>
> I updated the webrev:
> http://cr.openjdk.java.net/~goetz/webrevs/8028471-0-zpage/
>
> Best regards,
> Goetz.
>
> -----Original Message-----
> From: ppc-aix-port-dev-bounces at openjdk.java.net [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
> Sent: Thursday, November 21, 2013 8:48 PM
> To: '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.
>
> On 11/21/13 12:30 AM, Lindenmaier, Goetz wrote:
>> 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:
>
> It is initialized by 'true':
>
> NarrowPtrStruct Universe::_narrow_oop = { NULL, 0, true };
>
> except special case for Win64:
>
>> #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
>
> which can be reset back to 'true' if 0 page is used for traps:
>
> Universe::set_narrow_oop_base(NULL);
> // Don't need guard page for implicit checks in indexed
> // addressing mode with zero based Compressed Oops.
> Universe::set_narrow_oop_use_implicit_null_checks(true);
>
>>
>>> You can use Matcher::gen_narrow_oop_implicit_null_checks() check
>>> instead.
>> No, that returns true if narrow_oop_use_complex_address() is set.
>
> But you said narrow_oop_use_complex_address() is false on AIX:
>
> "> but ppc has no loads that support this."
>
> Did I get it wrong? If it could be true then yes, we can't use it.
>
>> 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?
>
> Yes.
>
>>
>>>> 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.
>
> You may be misunderstood me (or I you :) ).
>
> This code is only executed for AIX: (!os::zero_page_read_protected()).
> So I am fine to have NOT_AIX(Unimplemented()) because our current
> platforms will not hit it.
>
> My question was about AIX. If Op_DecodeN and following NotNULL checks
> are false you return 'false' from accesses_heap_base_zone() and EXPLICIT
> check will be generated.
>
> My question was, is that what you want for AIX? Because you have next
> comment:
>
> "On AIX, no such memory operands exist, so we need not check for them."
>
> Which I read as AIX only have case with DecodeN so above checks should
> always succeed. That is why I am confused and asked if you need
> ShouldNotReachHere there.
>
> Thanks,
> Vladimir
>
>>
>> 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.
>>>>
More information about the ppc-aix-port-dev
mailing list