RFR: Refactor conc-mark-in-progress/satb-active flags to use gc-phase-in-progress

[Christine Flood]

I suspect this comment is wrong.

  } else if (UseShenandoahGC && ShenandoahKeepAliveBarrier) {4014
enter(); // g1_write may call runtime4015
shenandoah_write_barrier_pre(noreg,4016
  val /* pre_val */,4017                                  thread /*
thread */,4018                                  tmp,4019
                   true /* tosca_live */,4020
        true /* expand_call */);
4021     leave();
4022   }
4023 }

I don't understand all of the changes inside of graphKit but I think the
refactoring makes sense and doesn't violate the "do no harm to
non-shenandoah" prime directive.

Including  shenandoahHeap.hpp in ifnode.cpp is yucky.  Can we maybe clean
this up and just have ShenandoahSupport (only slightly less yucky)?  It
looks like all we need is the CONC_MARK constant.


Christine

On Mon, Sep 25, 2017 at 12:59 PM, Roman Kennke <rkennke at redhat.com> wrote:

> Am 17.09.2017 um 14:11 schrieb Roman Kennke:
>
>>
>> his builds on top of the remove-reduce-storeval-barrier-optimization
>> patch.
>>
>> It refactors our conc-mark-in-progress flag together with the
>> satb-in-progress-flag into our new gc-phase-in-progress bitfield.
>>
>> Notable stuff:
>>
>> - The patch removes a 2nd check for satb-in-progress in C1. I'll propose
>> this upstream asap.
>> - The G1/Shenandoah pre-barriers are refactored such that the satb-active
>> (G1) and bitfield-concmark-checks (Sheanndoah) are now separated, while
>> keeping the rest of the inlined SATB-queue-pushing code shared.
>> - I noticed the store-checks code is wrong in both x86 and aarch64. We
>> want to check the storeval-oops only when update-refs is in progress, but
>> do the check statically, we need to do the check at runtime though because
>> we might update refs during separate phase or during conc-mark. This should
>> be fixed, but separately (and probably benefits from folding
>> update-refs-in-progress into the bitfield too)
>> - It involves quite a bit of refactoring in C2. I am fairly confident
>> that it is correct. Would be good to have Roland take a look too. We should
>> also check with Roland whether or not this is sufficient to allow commoning
>> loads and checks of the bitfield, or if we need extra optimization passes
>> for this.
>>
>> Test: hotspot_gc_shenandoah both x86 and aarch64
>>
>> http://cr.openjdk.java.net/~rkennke/satb-concmark-flag/webrev.01/ <
>> http://cr.openjdk.java.net/%7Erkennke/satb-concmark-flag/webrev.01/>
>>
>>
>> Roman
>>
>> Little update (incorporating Shade's store-check fix):
>
> http://cr.openjdk.java.net/~rkennke/satb-concmark-flag/webrev.02/ <
> http://cr.openjdk.java.net/%7Erkennke/satb-concmark-flag/webrev.02/>
>
> Roman
>
>