[aarch64-port-dev ] Proposed patch for C2 volatile generation (resent)

[Andrew Dinn]

Attached is a further update to the C2 volatile generation code.

Firstly, it includes updates to the predicates unnecessary_acquire and
needs_acquiring_load which detect MemBarAcquire and LoadX nodes that
have inserted as a result of an inlined unsafe volatile read.

Secondly, it factors out some of the common graph traversal operations
into two auxiliary helper functions, has_parent_membar and has_child_membar.

I have also cleaned up the comments to explain what is being looked for
more clearly as well as to include the new case of inlined unsafe
volatile read.

I have checked this in two ways

1) running netbeans and dumping then eyeballing the assembly code
generated for j.u.c class ConcurrentHashMap methods

2) running jcstress

The results are not clear cut:

1) netbeans ran fine and all the code I have eyeballed so far looks like
it has correctly removed unneeded membars and only unneeded membars
(there are still some obviously redundant dmbs which it would also be
good to remove but I have not pinned them down yet). I have not yet
compared every case against the corresponding case when the optimization
is disabled.

2) jcstress saw a fail on one test

org.openjdk.jcstress.tests.interrupt
   ObjectWaitInfiniteTest

TERMINATED 	6 	ACCEPTABLE 	The thread had sucessfully terminated.
STALE 	1 	FORBIDDEN 	Thread had failed to respond.

I have no idea what this means.

jcstress also saw some VM crashes

org.openjdk.jcstress.tests.interrupt
   ObjectWaitInfiniteTest

Exception in thread "main" java.lang.NoSuchMethodError:
sun.misc.Unsafe.fieldOffset(Ljava/lang/reflect/Field;)I
    . . .

I assume this is because recent jdk9 jigsaw changes have hidden this method?

The other VM crashes occurred in various tests

org.openjdk.jcstress.tests.seqcst.volatiles 	  	  	  	  	
   L1_L1__L2_S2_S1__S1__Test
   L1_L2_L1_S2__S1_L2__Test
   . . .

org.openjdk.jcstress.tests.memeffects.basic.atomicupdaters.AtomicIntegerFieldUpdater
	  	  	  	  	
   atomic_addAndGet_incrementAndGet_long
   . . .

org.openjdk.jcstress.tests.memeffects.basic.atomicupdaters.AtomicLongFieldUpdater
	  	  	  	  	
   atomic_CAS_CAS_short
   . . .

n.b. when I reran just these specific tests the crashes did not recur.

In all these cases the crash happened underneath a VM callout to
~RuntimeStub::resolve_virtual_call. Of course, that method has nothing
directly to do with the changes I made. It might be that the weaker sync
between Java threads is leading to synchronization errors between Java
and JVM code.

I am not sure that my changes are the cause of this error though. I
reverted the current patch to emulate the previous patch by omitting the
checks for inlined unsafe volatile gets (there's a const bool setting
which turns this extra check on or off). When I reran jcstress with that
tweak I still saw an error (once again underneath a VM callout to
~RuntimeStub::resolve_virtual_call). I am wondering if this might be a
problem with my machine's config, specifically the version of gcc
installed on my machine (4.8.2-16).

I am still waiting for the jcstress tests to complete. When they do I
will revert all my changes and see whether I still get crashes.
Meanwhile, Ed, could you try running jcstress with the attached patch to
see if the problem is my setup?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in UK and Wales under Company Registration No. 3798903
Directors: Michael Cunningham (USA), Matt Parson (USA), Charlie Peters
(USA), Michael O'Neill (Ireland)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 7991.patch
Type: text/x-patch
Size: 38314 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/aarch64-port-dev/attachments/20150414/45c16822/7991-0001.patch>