[aarch64-port-dev ] RFR: Shenandoah import

Aleksey Shipilev shade at redhat.com
Wed Oct 4 10:23:01 UTC 2017


On 10/04/2017 12:12 PM, Andrew Haley wrote:
> Some minor stuff:
> 
> --- a/src/share/vm/opto/macro.cpp
> +++ b/src/share/vm/opto/macro.cpp
> @@ -624,7 +624,7 @@
>                                     k < kmax && can_eliminate; k++) {
>            Node* n = use->fast_out(k);
>            if (!n->is_Store() && n->Opcode() != Op_CastP2X &&
> -              !n->is_g1_wb_pre_call()) {
> +              (!UseShenandoahGC || !n->is_g1_wb_pre_call())) {
> 
> This logic is super-confusing.  It could be
> 
>            if (! UseShenandoahGC &&
>                !n->is_Store() && n->Opcode() != Op_CastP2X &&
>                !n->is_g1_wb_pre_call()) {
> 
> it could even be
> 
>            if (! UseShenandoahGC &&
>                ! (n->is_Store() || n->Opcode() == Op_CastP2X ||
>                   n->is_g1_wb_pre_call()) {
> 
> but I guess that'd be a divergence from upstream, which we want to
> minimize.  in general, though, for things that we add we should reduce
> the complexity of boolean operations where it makes sense.

Yes, and I think the suggestions are silently breaking: they expose "n->is_g1_wb_pre_call()" when
UseShenandoahGC=false.

The actual difference against upstream after this patch is this:

-          if (!n->is_Store() && n->Opcode() != Op_CastP2X) {
+          if (!n->is_Store() && n->Opcode() != Op_CastP2X &&
+              (!UseShenandoahGC || !n->is_g1_wb_pre_call())) {

https://builds.shipilev.net/patch-openjdk-shenandoah-jdk8/2017-10-04-v41-vs-20d83f8419c4/src/share/vm/opto/macro.cpp.sdiff.html

I think it is a good style to make the "&& (expr)" additions to predicates where expr clearly
coverts itself into "true" when UseShenandoahGC is "false".

Thanks,
-Aleksey



More information about the aarch64-port-dev mailing list