[aarch64-port-dev ] RFR: 8136596: Remove MemBarRelease when final field's allocation is NoEscape or ArgEscape
Roland Westrelin
roland.westrelin at oracle.com
Fri Sep 18 14:49:17 UTC 2015
> On Sep 18, 2015, at 4:43 PM, Hui Shi <hui.shi at linaro.org> wrote:
>
> Thanks Edward!
> Thanks for sponsoring Roland! New patch is pasted below
Thanks.
>
> Add small method "bool AllocatedNode.does_not_escape_thread()", return true when allocate node will not escape thread. Patch looks clearer. In PhaseMacroExpand::expand_allocate_common, InitializationNode is already there, InitializationNode might be get again in AllocatedNode .does_not_escape_thread(). Is this acceptable?
I think it’s ok.
A comment that explains why we need to look at both the AllocateNode and InitializeNode would be nice.
Roland.
>
> http://people.linaro.org/~hui.shi/MemBarRelease_Escape/MemBarEscape_v2.patch
>
> diff -r 476739c20b35 src/share/vm/opto/callnode.hpp
> --- a/src/share/vm/opto/callnode.hpp Thu Sep 17 13:42:50 2015 -0700
> +++ b/src/share/vm/opto/callnode.hpp Fri Sep 18 15:27:46 2015 +0800
> @@ -907,6 +907,11 @@
>
> // Convenience for initialization->maybe_set_complete(phase)
> bool maybe_set_complete(PhaseGVN* phase);
> +
> + bool does_not_escape_thread() {
> + InitializeNode* init = NULL;
> + return _is_non_escaping || (((init = initialization()) != NULL) && init->does_not_escape());
> + }
> };
>
> //------------------------------AllocateArray---------------------------------
> diff -r 476739c20b35 src/share/vm/opto/macro.cpp
> --- a/src/share/vm/opto/macro.cpp Thu Sep 17 13:42:50 2015 -0700
> +++ b/src/share/vm/opto/macro.cpp Fri Sep 18 15:27:46 2015 +0800
> @@ -1512,7 +1512,8 @@
> // MemBarStoreStore so that stores that initialize this object
> // can't be reordered with a subsequent store that makes this
> // object accessible by other threads.
> - if (init == NULL || (!init->is_complete_with_arraycopy() && !init->does_not_escape())) {
> + if (!alloc->does_not_escape_thread() &&
> + (init == NULL || !init->is_complete_with_arraycopy())) {
> if (init == NULL || init->req() < InitializeNode::RawStores) {
> // No InitializeNode or no stores captured by zeroing
> // elimination. Simply add the MemBarStoreStore after object
> diff -r 476739c20b35 src/share/vm/opto/memnode.cpp
> --- a/src/share/vm/opto/memnode.cpp Thu Sep 17 13:42:50 2015 -0700
> +++ b/src/share/vm/opto/memnode.cpp Fri Sep 18 15:27:46 2015 +0800
> @@ -2945,7 +2945,7 @@
> // Final field stores.
> Node* alloc = AllocateNode::Ideal_allocation(in(MemBarNode::Precedent), phase);
> if ((alloc != NULL) && alloc->is_Allocate() &&
> - alloc->as_Allocate()->_is_non_escaping) {
> + alloc->as_Allocate()->does_not_escape_thread()) {
> // The allocated object does not escape.
> eliminate = true;
> }
>
> Regards
> Shi Hui
>
> On 17 September 2015 at 21:44, Roland Westrelin <roland.westrelin at oracle.com> wrote:
>
>
> > Could someone please sponsor this change and push it through JPRT as it involves changes to shared code.
> >
> > I have filed a JIRA report
> >
> > https://bugs.openjdk.java.net/browse/JDK-8136596
> >
> > and generated a Webber
> >
> > http://cr.openjdk.java.net/~enevill/8136596/webrev/
>
> I can sponsor that change. It would be nice if the mostly identical code in memnode.cpp and macro.cpp is factored in a single method in AllocateNode with a comment that explains why we need to test the status of both alloc and init.
>
> Thanks,
> Roland.
>
> >
> > This change was contributed by hui.shi at linaro.org.
> >
> > I have tested it with JTreg hotspot + langtools with no regressions and also with jcstress with no failures.
> >
> > Many thanks,
> > Ed.
> >
> >
> > On Wed, 2015-09-16 at 06:44 +0800, Hui Shi wrote:
> >> Thanks Vladimir!
> >>
> >>
> >> New patch keeps original "alloc->as_Allocate()->_is_non_escaping" check with InitializeNode->does_not_escape() check.
> >> Another similar place is PhaseMacroExpand::expand_allocate_common, where generate MemBarStoreStore for allocation node. If AllocateNode is not escape, it doesn't need generate MemBarStoreStore no matter its InitializeNode NULL or not.
> >>
> >>
> >> diff -r 720d0ff40323 src/share/vm/opto/macro.cpp
> >> --- a/src/share/vm/opto/macro.cpp Mon Sep 14 07:03:04 2015 +0000
> >> +++ b/src/share/vm/opto/macro.cpp Mon Sep 14 19:33:46 2015 +0800
> >> @@ -1512,7 +1512,8 @@
> >> // MemBarStoreStore so that stores that initialize this object
> >> // can't be reordered with a subsequent store that makes this
> >> // object accessible by other threads.
> >> - if (init == NULL || (!init->is_complete_with_arraycopy() && !init->does_not_escape())) {
> >> + if (!alloc->_is_non_escaping &&
> >> + (init == NULL || (!init->is_complete_with_arraycopy() && !init->does_not_escape()))) {
> >> if (init == NULL || init->req() < InitializeNode::RawStores) {
> >> // No InitializeNode or no stores captured by zeroing
> >> // elimination. Simply add the MemBarStoreStore after object
> >> diff -r 720d0ff40323 src/share/vm/opto/memnode.cpp
> >> --- a/src/share/vm/opto/memnode.cpp Mon Sep 14 07:03:04 2015 +0000
> >> +++ b/src/share/vm/opto/memnode.cpp Mon Sep 14 19:33:46 2015 +0800
> >> @@ -2944,8 +2944,11 @@
> >> } else if (opc == Op_MemBarRelease) {
> >> // Final field stores.
> >> Node* alloc = AllocateNode::Ideal_allocation(in(MemBarNode::Precedent), phase);
> >> + InitializeNode* init = NULL;
> >> if ((alloc != NULL) && alloc->is_Allocate() &&
> >> - alloc->as_Allocate()->_is_non_escaping) {
> >> + (alloc->as_Allocate()->_is_non_escaping ||
> >> + ((init = alloc->as_Allocate()->initialization()) != NULL &&
> >> + init->does_not_escape()))) {
> >> // The allocated object does not escape.
> >> eliminate = true;
> >> }
> >>
> >>
> >>
> >>
> >> Regards
> >> Shi Hui
> >>
> >> On 14 September 2015 at 01:37, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> >> Please keep original AllocateNode check and add new check with ||.
> >> There are cases when there is no InitializeNode associated with allocation (when there are no access to object) so we need to keep first check.
> >>
> >> Thanks,
> >> Vladimir
> >>
> >> On 9/8/15 5:41 AM, Hui Shi wrote:
> >> Hi JIT members,
> >>
> >>
> >> There might be better use of escape analysis result when optimizing
> >> MemBarRelease node for final field stores. Could anyone help review and
> >> comments?
> >>
> >> Patch in
> >> http://people.linaro.org/~hui.shi/MemBarRelease_Escape/MemBarEscape.patch
> >>
> >> __
> >>
> >> In hotspot, there are two different escape information recorded on a node.
> >>
> >> 1. AlllocateNode._is_non_escaping, true means allocation node’s escape
> >> state is noEscape.____
> >>
> >> 2. InitializeNode._does_not_escape, true means its allocation node’s
> >> escape state is noEscape or ArgEscape.____
> >>
> >> NoEscape has literal meaning. ArgEscape means allocation node is passed
> >> to callee but will not escape current thread. So ArgEscape is safe to
> >> remove MemBarRelase node for final field store in initialization method.____
> >>
> >> __ __
> >>
> >> Pasted test creates an Integer instance in foo, MemBarRelase node is
> >> created at the end of initialization method as Integer's value field is
> >> final. New instance is ArgEscaped because it is passed to bar (disable
> >> inlining bar). It fails to remove MemBarRelase node now, because
> >> AlllocateNode._is_non_escaping is used in MemBarNode::Ideal(PhaseGVN
> >> *phase, bool can_reshape) and this flag is false.____
> >>
> >> __ __
> >>
> >> public class TestInteger {____
> >>
> >> public static void main(String[] args) {____
> >>
> >> for(int i = 0; i < 1000000; i ++)____
> >>
> >> foo(i);____
> >>
> >> }____
> >>
> >> __ __
> >>
> >> static int foo(int i) {____
> >>
> >> Integer newi = new Integer(i);____
> >>
> >> return bar(newi);____
> >>
> >> }____
> >>
> >> __ __
> >>
> >> static int bar(Integer i) {____
> >>
> >> return i.intValue() + 2;____
> >>
> >> }____
> >>
> >> }____
> >>
> >> __ __
> >>
> >> This patch deletes MemBarRelease node when its allocation node is
> >> ArgEscape or NoEscape by checking its InitializeNode._does_not_escape
> >> flag.____
> >>
> >>
> >> Regards
> >>
> >> Shi Hui
> >>
> >>
> >>
> >
> >
>
>
More information about the aarch64-port-dev
mailing list