Skip to content

Commit 649568a

Browse files
tkrodrigueztkrodriguez
committed
[JDK-8338788] Add missing implicit null check
PullRequest: graal/18632
2 parents b339df9 + 6c6a4ff commit 649568a

File tree

5 files changed

+37
-22
lines changed

5 files changed

+37
-22
lines changed

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/amd64/z/AMD64HotSpotZAtomicReadAndWriteOp.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
package jdk.graal.compiler.hotspot.amd64.z;
2626

27+
import static jdk.graal.compiler.hotspot.amd64.z.AMD64HotSpotZBarrierSetLIRGenerator.zColor;
2728
import static jdk.vm.ci.code.ValueUtil.asRegister;
2829

2930
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
@@ -53,7 +54,8 @@ public AMD64HotSpotZAtomicReadAndWriteOp(Variable result, AMD64AddressValue load
5354

5455
@Override
5556
public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
56-
emitStoreBarrier(crb, masm, asRegister(result), asRegister(newValue), true, null);
57+
emitPreWriteBarrier(crb, masm, asRegister(result), null);
58+
zColor(crb, masm, asRegister(result), asRegister(newValue));
5759
masm.xchgq(asRegister(result), storeAddress.toAddress());
5860
Register ref = asRegister(result);
5961
AMD64HotSpotZBarrierSetLIRGenerator.zUncolor(crb, masm, ref);

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/amd64/z/AMD64HotSpotZBarrierSetLIRGenerator.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public ForeignCallsProvider getForeignCalls() {
104104
}
105105

106106
/**
107-
* Convert a normal oop into a colored pointer.
107+
* Convert a normal oop into a colored pointer in a single register.
108108
*/
109109
@SyncPort(from = "https://github.com/openjdk/jdk/blob/4acafb809c66589fbbfee9c9a4ba7820f848f0e4/src/hotspot/cpu/x86/gc/z/z_x86_64.ad#L37-L42", sha1 = "344c51c07478c916bdaabb0c697a053e7a2f64dd")
110110
public static void zColor(CompilationResultBuilder crb, AMD64MacroAssembler masm, Register ref) {
@@ -114,6 +114,15 @@ public static void zColor(CompilationResultBuilder crb, AMD64MacroAssembler masm
114114
crb.recordMark(HotSpotMarkId.Z_BARRIER_RELOCATION_FORMAT_STORE_GOOD_AFTER_OR);
115115
}
116116

117+
/**
118+
* Move a normal oop into a new register and convert it into a colored pointer.
119+
*/
120+
public static void zColor(CompilationResultBuilder crb, AMD64MacroAssembler masm, Register resultReg, Register writeValue) {
121+
Assembler.guaranteeDifferentRegisters(writeValue, resultReg);
122+
masm.movq(resultReg, writeValue);
123+
zColor(crb, masm, resultReg);
124+
}
125+
117126
/**
118127
* Convert a colored pointer into normal oop.
119128
*/
@@ -129,7 +138,7 @@ public static void zUncolor(CompilationResultBuilder crb, AMD64MacroAssembler ma
129138
*/
130139
@SyncPort(from = "https://github.com/openjdk/jdk/blob/4acafb809c66589fbbfee9c9a4ba7820f848f0e4/src/hotspot/cpu/x86/gc/z/zBarrierSetAssembler_x86.cpp#L304-L321", sha1 = "9a628c1771df79ae8b4cee89d2863fbd4a4964bc")
131140
@SyncPort(from = "https://github.com/openjdk/jdk/blob/4acafb809c66589fbbfee9c9a4ba7820f848f0e4/src/hotspot/cpu/x86/gc/z/zBarrierSetAssembler_x86.cpp#L370-L414", sha1 = "7688e7aeab5f1aa413690066355a17c18a4273fa")
132-
public static void emitStoreBarrier(CompilationResultBuilder crb,
141+
public static void emitPreWriteBarrier(CompilationResultBuilder crb,
133142
AMD64MacroAssembler masm,
134143
LIRInstruction op,
135144
GraalHotSpotVMConfig config,
@@ -472,9 +481,12 @@ public void emitStore(LIRGeneratorTool lirTool,
472481
tool.getResult().getFrameMapBuilder().callsMethod(callTarget.getOutgoingCallingConvention());
473482
boolean emitPreWriteBarrier = !location.isInit() || barrierType == BarrierType.POST_INIT_WRITE;
474483
tool.append(new AMD64HotSpotZPreWriteBarrierOp(isConstantNull ? Value.ILLEGAL : tool.asAllocatable(value), addressValue, tmp, tmp2, config, callTarget, result, storeKind,
475-
emitPreWriteBarrier, nullCheckState));
484+
emitPreWriteBarrier, emitPreWriteBarrier ? state : null));
485+
if (emitPreWriteBarrier) {
486+
// The pre write barrier performed any necessary null check
487+
nullCheckState = null;
488+
}
476489
writeValue = result;
477-
nullCheckState = null;
478490
}
479491
if (isConstantNull) {
480492
tool.append(new ZStoreNullOp(QWORD, storeAddress, nullCheckState));

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/amd64/z/AMD64HotSpotZCompareAndSwapOp.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
package jdk.graal.compiler.hotspot.amd64.z;
2626

27+
import static jdk.graal.compiler.hotspot.amd64.z.AMD64HotSpotZBarrierSetLIRGenerator.zColor;
2728
import static jdk.vm.ci.code.ValueUtil.asRegister;
2829

2930
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
@@ -78,9 +79,9 @@ public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
7879
GraalError.guarantee(accessKind == AMD64Kind.QWORD, "ZGC only supports uncompressed oops");
7980
assert LIRValueUtil.differentRegisters(cmpValue, newValue, storeAddress);
8081

81-
emitStoreBarrier(crb, masm, asRegister(tmp), asRegister(newValue), true, null);
82-
Register ref1 = asRegister(cmpValue);
83-
AMD64HotSpotZBarrierSetLIRGenerator.zColor(crb, masm, ref1);
82+
emitPreWriteBarrier(crb, masm, asRegister(tmp), null);
83+
zColor(crb, masm, asRegister(tmp), asRegister(newValue));
84+
AMD64HotSpotZBarrierSetLIRGenerator.zColor(crb, masm, asRegister(cmpValue));
8485
if (crb.target.isMP) {
8586
masm.lock();
8687
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/amd64/z/AMD64HotSpotZPreWriteBarrierOp.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@
2424
*/
2525
package jdk.graal.compiler.hotspot.amd64.z;
2626

27+
import static jdk.graal.compiler.hotspot.amd64.z.AMD64HotSpotZBarrierSetLIRGenerator.zColor;
2728
import static jdk.graal.compiler.lir.LIRInstruction.OperandFlag.ILLEGAL;
2829
import static jdk.graal.compiler.lir.LIRInstruction.OperandFlag.REG;
2930
import static jdk.vm.ci.code.ValueUtil.asRegister;
30-
import static jdk.vm.ci.code.ValueUtil.isIllegal;
31+
import static jdk.vm.ci.code.ValueUtil.isRegister;
3132

3233
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
3334
import jdk.graal.compiler.core.common.spi.ForeignCallLinkage;
@@ -37,6 +38,7 @@
3738
import jdk.graal.compiler.lir.LIRInstructionClass;
3839
import jdk.graal.compiler.lir.amd64.AMD64AddressValue;
3940
import jdk.graal.compiler.lir.asm.CompilationResultBuilder;
41+
import jdk.vm.ci.code.Register;
4042
import jdk.vm.ci.meta.AllocatableValue;
4143
import jdk.vm.ci.meta.Value;
4244

@@ -64,10 +66,17 @@ protected AMD64HotSpotZPreWriteBarrierOp(Value writeValue,
6466
this.writeValue = writeValue;
6567
this.emitPreWriteBarrier = emitPreWriteBarrier;
6668
this.state = state;
69+
assert emitPreWriteBarrier || state == null : "implicit null check is only performed with preWriteBarrier";
6770
}
6871

6972
@Override
7073
public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
71-
emitStoreBarrier(crb, masm, asRegister(result), isIllegal(writeValue) ? null : asRegister(writeValue), emitPreWriteBarrier, state);
74+
Register resultReg = asRegister(result);
75+
if (emitPreWriteBarrier) {
76+
emitPreWriteBarrier(crb, masm, resultReg, state);
77+
}
78+
if (isRegister(writeValue)) {
79+
zColor(crb, masm, resultReg, asRegister(writeValue));
80+
}
7281
}
7382
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/amd64/z/AMD64HotSpotZStoreBarrieredOp.java

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,10 @@
2424
*/
2525
package jdk.graal.compiler.hotspot.amd64.z;
2626

27-
import static jdk.graal.compiler.hotspot.amd64.z.AMD64HotSpotZBarrierSetLIRGenerator.zColor;
2827
import static jdk.graal.compiler.lir.LIRInstruction.OperandFlag.COMPOSITE;
2928
import static jdk.graal.compiler.lir.LIRInstruction.OperandFlag.REG;
3029
import static jdk.vm.ci.code.ValueUtil.asRegister;
3130

32-
import jdk.graal.compiler.asm.Assembler;
3331
import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler;
3432
import jdk.graal.compiler.core.common.spi.ForeignCallLinkage;
3533
import jdk.graal.compiler.hotspot.GraalHotSpotVMConfig;
@@ -76,15 +74,8 @@ protected AMD64HotSpotZStoreBarrieredOp(LIRInstructionClass<? extends AMD64HotSp
7674
this.storeKind = storeKind;
7775
}
7876

79-
protected void emitStoreBarrier(CompilationResultBuilder crb, AMD64MacroAssembler masm, Register resultReg, Register writeValue, boolean emitPreWriteBarrier, LIRFrameState state) {
80-
if (emitPreWriteBarrier) {
81-
AMD64HotSpotZBarrierSetLIRGenerator.emitStoreBarrier(crb, masm, this, config, storeAddress.toAddress(), resultReg, storeKind, asRegister(tmp), asRegister(tmp2), callTarget,
82-
state);
83-
}
84-
if (writeValue != null) {
85-
Assembler.guaranteeDifferentRegisters(writeValue, resultReg);
86-
masm.movq(resultReg, writeValue);
87-
zColor(crb, masm, resultReg);
88-
}
77+
protected void emitPreWriteBarrier(CompilationResultBuilder crb, AMD64MacroAssembler masm, Register resultReg, LIRFrameState state) {
78+
AMD64HotSpotZBarrierSetLIRGenerator.emitPreWriteBarrier(crb, masm, this, config, storeAddress.toAddress(), resultReg, storeKind, asRegister(tmp), asRegister(tmp2), callTarget,
79+
state);
8980
}
9081
}

0 commit comments

Comments
 (0)