RFR (XS): C2: Missing matrix update for getAndSet intrinsic

[Aleksey Shipilev]

On 03/14/2017 01:55 PM, Roman Kennke wrote:
> It might be better to simply call it with newval != NULL. I suspect that
> blocking can_move_pre_barrier() might prevent some optimizations, for
> example, getAndSet() pre-barrier doesn't actually need to load the
> pre-value when executed after the fact. 

I thought so too, but when I change the "moved" pre_barrier to accept new_val,
it crashes on other asserts within C2 (f.ex. split-if barfs at graph shape).
Doing this, basically:

diff -r 99d1d76141e7 src/share/vm/opto/library_call.cpp
--- a/src/share/vm/opto/library_call.cpp	Tue Mar 14 11:09:03 2017 +0100
+++ b/src/share/vm/opto/library_call.cpp	Tue Mar 14 14:55:51 2017 +0100
@@ -3115,12 +3115,12 @@
       load_store = _gvn.transform(new DecodeNNode(load_store,
load_store->get_ptr_type()));
     }
 #endif
-    if (can_move_pre_barrier()) {
+    if (can_move_pre_barrier() && kind == LS_get_set) {
       // Don't need to load pre_val. The old value is returned by load_store.
       // The pre_barrier can execute after the xchg as long as no safepoint
       // gets inserted between them.
       pre_barrier(false /* do_load */,
-                  control(), NULL, NULL, max_juint, NULL, NULL,
+                  control(), NULL, adr, max_juint, newval, NULL,
                   load_store /* pre_val */,
                   T_OBJECT);
     }

Yields:

  #  Internal Error
(/home/shade/trunks/shenandoah-jdk9/hotspot/src/share/vm/opto/split_if.cpp:287),
pid=30197, tid=30218
  #  assert(prior_n->is_Region()) failed: must be a post-dominating merge point

> And it also affects other intrinsics than getAndSet(). 

Really? can_move_pre_barrier() only affects LS_get_set, as far as I can see.

Thanks,
-Aleksey