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

[Roman Kennke]

Am 14.03.2017 um 14:56 schrieb 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
Grrrr!!
To be honest, there's no code path that usually returns false on
can_move_pre_barrier(). IMO, this should go to the trash bin.

>> And it also affects other intrinsics than getAndSet(). 
> Really? can_move_pre_barrier() only affects LS_get_set, as far as I can see.

I thought it also affects cas intrinsics. Must be a bad memory. :-P

Push it as-is, and also push it to Roland's to-do list? :-)

Roman