Skip to content

Commit f49b7cd

Browse files
author
Xiaohong Gong
committed
Apply trapping nullcheck to uncompressed object for compressed pointer.
Trapping nullcheck might generate two uncompress instructions for the same compressed oop on AArch64. One is inserted by the backend when it emits nullcheck. If the object is a compressed pointer, it is uncompressed before the nullcheck is emitted. And another one is generated by the uncompression node used for memory access. These two instructions are duplicated with each other. The generated codes on AArch64 like: ldr w0, [x0,oracle#112] lsl x2, x0, oracle#3 ; uncompressing (first) ldr xzr, [x2] ; implicit exception: deoptimizes ...... ; fixed operations lsl x0, x0, oracle#3 ; uncompressing (second) str w1, [x0,oracle#12] A simple way to avoid this is to creat a new uncompression node for the nullcheck, and let the value numbering remove the duplicated one if possible. Since the address lowering of AMD64 can handle the uncompressing computation for address, the created uncompression node is wrapped to an address node and the nullcheck is finally applied on the address. With the modification, the codes above could be optimized to: ldr w0, [x0,oracle#112] lsl x0, x0, oracle#3 ; uncompressing ldr xzr, [x0] ; implicit exception: deoptimizes ...... ; fixed operations str w1, [x0,oracle#12] Change-Id: Iabfe47bbf984ed11c42555f84bdd0ccf2a5bdddb
1 parent cb33280 commit f49b7cd

File tree

9 files changed

+82
-18
lines changed

9 files changed

+82
-18
lines changed

compiler/src/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/type/AbstractPointerStamp.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -161,4 +161,8 @@ public JavaConstant nullConstant() {
161161
public JavaKind getStackKind() {
162162
return JavaKind.Illegal;
163163
}
164+
165+
public boolean isCompressed() {
166+
return false;
167+
}
164168
}

compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/phases/LowTier.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -70,10 +70,10 @@ public LowTier(OptionValues options) {
7070
appendPhase(new FixReadsPhase(true,
7171
new SchedulePhase(GraalOptions.StressTestEarlyReads.getValue(options) ? SchedulingStrategy.EARLIEST : SchedulingStrategy.LATEST_OUT_OF_LOOPS_IMPLICIT_NULL_CHECKS)));
7272

73-
appendPhase(canonicalizerWithoutGVN);
74-
7573
appendPhase(new UseTrappingNullChecksPhase());
7674

75+
appendPhase(canonicalizerWithoutGVN);
76+
7777
appendPhase(new DeadCodeEliminationPhase(Required));
7878

7979
appendPhase(new PropagateDeoptimizeProbabilityPhase());

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/AddressLoweringHotSpotSuitesProvider.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2017, Red Hat Inc. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -56,11 +56,13 @@ public Suites createSuites(OptionValues options) {
5656
Suites suites = super.createSuites(options);
5757

5858
ListIterator<BasePhase<? super LowTierContext>> findPhase = suites.getLowTier().findPhase(UseTrappingNullChecksPhase.class);
59-
if (findPhase == null) {
59+
if (findPhase != null) {
60+
findPhase.add(addressLowering);
61+
} else {
6062
findPhase = suites.getLowTier().findPhase(SchedulePhase.class);
63+
findPhase.previous();
64+
findPhase.add(addressLowering);
6165
}
62-
findPhase.previous();
63-
findPhase.add(addressLowering);
6466

6567
return suites;
6668
}

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/nodes/type/KlassPointerStamp.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -146,6 +146,7 @@ public LIRKind getLIRKind(LIRKindTool tool) {
146146
}
147147
}
148148

149+
@Override
149150
public boolean isCompressed() {
150151
return encoding != null;
151152
}

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/NullCheckNode.java

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -28,11 +28,18 @@
2828
import static org.graalvm.compiler.nodeinfo.NodeCycles.CYCLES_2;
2929
import static org.graalvm.compiler.nodeinfo.NodeSize.SIZE_2;
3030

31+
import org.graalvm.compiler.core.common.type.AbstractPointerStamp;
32+
import org.graalvm.compiler.core.common.type.Stamp;
3133
import org.graalvm.compiler.core.common.type.StampFactory;
34+
import org.graalvm.compiler.graph.Graph;
35+
import org.graalvm.compiler.graph.Node;
3236
import org.graalvm.compiler.graph.NodeClass;
3337
import org.graalvm.compiler.nodeinfo.NodeInfo;
38+
import org.graalvm.compiler.nodes.CompressionNode;
3439
import org.graalvm.compiler.nodes.DeoptimizingFixedWithNextNode;
40+
import org.graalvm.compiler.nodes.NodeView;
3541
import org.graalvm.compiler.nodes.ValueNode;
42+
import org.graalvm.compiler.nodes.memory.address.OffsetAddressNode;
3643
import org.graalvm.compiler.nodes.spi.LIRLowerable;
3744
import org.graalvm.compiler.nodes.spi.NodeLIRBuilderTool;
3845

@@ -47,6 +54,46 @@ public NullCheckNode(ValueNode object) {
4754
this.object = object;
4855
}
4956

57+
public static NullCheckNode create(ValueNode object) {
58+
// Try to uncompress a compressed object before applying null check.
59+
NullCheckNode nullCheck = tryUseUncompressedNullCheck(object);
60+
if (nullCheck != null) {
61+
return nullCheck;
62+
}
63+
return new NullCheckNode(object);
64+
}
65+
66+
private static NullCheckNode tryUseUncompressedNullCheck(ValueNode value) {
67+
Stamp stamp = value.stamp(NodeView.DEFAULT);
68+
// Do nothing if the value is not a compressed pointer.
69+
if (!(stamp instanceof AbstractPointerStamp) || !((AbstractPointerStamp) stamp).isCompressed()) {
70+
return null;
71+
}
72+
73+
// Copy an uncompressing node from one of the existed uncompressing usages.
74+
CompressionNode uncompressed = null;
75+
for (Node usage : value.usages()) {
76+
if (usage instanceof CompressionNode) {
77+
if (((CompressionNode) usage).getOp() == CompressionNode.CompressionOp.Uncompress) {
78+
uncompressed = (CompressionNode) usage;
79+
break;
80+
}
81+
}
82+
}
83+
// Do nothing if there is not an uncompressing usage. The uncompressing operation will be
84+
// handled at back-end.
85+
if (uncompressed == null) {
86+
return null;
87+
}
88+
89+
assert uncompressed.getValue().equals(value);
90+
Graph graph = value.graph();
91+
CompressionNode compression = (CompressionNode) uncompressed.copyWithInputs(false);
92+
compression = graph.addOrUniqueWithInputs(compression);
93+
OffsetAddressNode address = graph.addOrUniqueWithInputs(OffsetAddressNode.create(compression));
94+
return new NullCheckNode(address);
95+
}
96+
5097
public ValueNode getObject() {
5198
return object;
5299
}

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/memory/address/OffsetAddressNode.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -58,13 +58,18 @@ public OffsetAddressNode(ValueNode base, ValueNode offset) {
5858
super(TYPE);
5959
this.base = base;
6060
this.offset = offset;
61-
6261
assert base != null && (base.stamp(NodeView.DEFAULT) instanceof AbstractPointerStamp || IntegerStamp.getBits(base.stamp(NodeView.DEFAULT)) == 64) &&
6362
offset != null && IntegerStamp.getBits(offset.stamp(NodeView.DEFAULT)) == 64 : "both values must have 64 bits";
6463
}
6564

6665
public static OffsetAddressNode create(ValueNode base) {
67-
return new OffsetAddressNode(base, ConstantNode.forIntegerBits(PrimitiveStamp.getBits(base.stamp(NodeView.DEFAULT)), 0));
66+
ValueNode offset;
67+
if (base.stamp(NodeView.DEFAULT) instanceof AbstractPointerStamp) {
68+
offset = ConstantNode.forIntegerBits(64, 0);
69+
} else {
70+
offset = ConstantNode.forIntegerBits(PrimitiveStamp.getBits(base.stamp(NodeView.DEFAULT)), 0);
71+
}
72+
return new OffsetAddressNode(base, offset);
6873
}
6974

7075
@Override

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/type/NarrowOopStamp.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -115,4 +115,9 @@ public boolean equals(Object obj) {
115115

116116
@Override
117117
public abstract boolean isCompatible(Constant other);
118+
119+
@Override
120+
public boolean isCompressed() {
121+
return true;
122+
}
118123
}

compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/UseTrappingNullChecksPhase.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -265,7 +265,7 @@ private static void replaceWithTrappingNullCheck(AbstractDeoptimizeNode deopt, I
265265

266266
if (trappingNullCheck == null) {
267267
// Need to add a null check node.
268-
trappingNullCheck = deopt.graph().add(new NullCheckNode(value));
268+
trappingNullCheck = deopt.graph().add(NullCheckNode.create(value));
269269
deopt.graph().replaceSplit(ifNode, trappingNullCheck, nonTrappingContinuation);
270270
deopt.getDebug().log("Inserted NullCheckNode %s", trappingNullCheck);
271271
}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,10 @@
8989
import org.graalvm.compiler.phases.common.CanonicalizerPhase;
9090
import org.graalvm.compiler.phases.common.DeoptimizationGroupingPhase;
9191
import org.graalvm.compiler.phases.common.ExpandLogicPhase;
92-
import org.graalvm.compiler.phases.common.FixReadsPhase;
9392
import org.graalvm.compiler.phases.common.FrameStateAssignmentPhase;
9493
import org.graalvm.compiler.phases.common.LoopSafepointInsertionPhase;
9594
import org.graalvm.compiler.phases.common.LoweringPhase;
95+
import org.graalvm.compiler.phases.common.UseTrappingNullChecksPhase;
9696
import org.graalvm.compiler.phases.common.inlining.InliningPhase;
9797
import org.graalvm.compiler.phases.tiers.HighTierContext;
9898
import org.graalvm.compiler.phases.tiers.LowTierContext;
@@ -1300,7 +1300,7 @@ private static Suites modifySuites(SubstrateBackend backend, Suites suites, Feat
13001300
if (firstTier) {
13011301
lowTier.findPhase(ExpandLogicPhase.class).add(addressLoweringPhase);
13021302
} else {
1303-
lowTier.findPhase(FixReadsPhase.class).add(addressLoweringPhase);
1303+
lowTier.findPhase(UseTrappingNullChecksPhase.class).add(addressLoweringPhase);
13041304
}
13051305

13061306
if (SubstrateOptions.MultiThreaded.getValue()) {

0 commit comments

Comments
 (0)