RFR: 8136596: Remove MemBarRelease when final field's allocation is NoEscape or ArgEscape

[Edward Nevill]

Hi,

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 webrev

http://cr.openjdk.java.net/~enevill/8136596/webrev/

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
>                 
> 
>