[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