Skip to content

Commit

Permalink
Add more checks to ensure PlatformKinds are compatible.
Browse files Browse the repository at this point in the history
  • Loading branch information
teshull committed Sep 27, 2022
1 parent 46235c9 commit 38752d2
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ private Variable emitCompareAndSwap(boolean isLogicVariant, LIRKind accessKind,
reinterpretedNewValue = arithmeticLIRGen.emitReinterpret(integerAccessKind, newValue);
}
AArch64Kind memKind = (AArch64Kind) integerAccessKind.getPlatformKind();
Variable result = newVariable(integerAccessKind);
Variable result = newVariable(toRegisterKind(integerAccessKind));
AllocatableValue allocatableExpectedValue = asAllocatable(reinterpretedExpectedValue);
AllocatableValue allocatableNewValue = asAllocatable(reinterpretedNewValue);
append(new CompareAndSwapOp(memKind, memoryOrder, isLogicVariant, result, allocatableExpectedValue, allocatableNewValue, asAllocatable(address)));
Expand All @@ -218,16 +218,16 @@ private Variable emitCompareAndSwap(boolean isLogicVariant, LIRKind accessKind,
}

@Override
public Value emitAtomicReadAndWrite(Value address, ValueKind<?> kind, Value newValue) {
Variable result = newVariable(kind);
append(new AtomicReadAndWriteOp((AArch64Kind) kind.getPlatformKind(), result, asAllocatable(address), asAllocatable(newValue)));
public Value emitAtomicReadAndWrite(LIRKind accessKind, Value address, Value newValue) {
Variable result = newVariable(toRegisterKind(accessKind));
append(new AtomicReadAndWriteOp((AArch64Kind) accessKind.getPlatformKind(), result, asAllocatable(address), asAllocatable(newValue)));
return result;
}

@Override
public Value emitAtomicReadAndAdd(Value address, ValueKind<?> kind, Value delta) {
Variable result = newVariable(kind);
append(AArch64AtomicMove.createAtomicReadAndAdd(this, (AArch64Kind) kind.getPlatformKind(), result, asAllocatable(address), delta));
public Value emitAtomicReadAndAdd(LIRKind accessKind, Value address, Value delta) {
Variable result = newVariable(toRegisterKind(accessKind));
append(AArch64AtomicMove.createAtomicReadAndAdd(this, (AArch64Kind) accessKind.getPlatformKind(), result, asAllocatable(address), delta));
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,18 +290,18 @@ public void emitCompareAndSwapBranch(ValueKind<?> kind, AMD64AddressValue addres
}

@Override
public Value emitAtomicReadAndAdd(Value address, ValueKind<?> kind, Value delta) {
Variable result = newVariable(kind);
public Value emitAtomicReadAndAdd(LIRKind accessKind, Value address, Value delta) {
Variable result = newVariable(toRegisterKind(accessKind));
AMD64AddressValue addressValue = asAddressValue(address);
append(new AMD64Move.AtomicReadAndAddOp((AMD64Kind) kind.getPlatformKind(), result, addressValue, asAllocatable(delta)));
append(new AMD64Move.AtomicReadAndAddOp((AMD64Kind) accessKind.getPlatformKind(), result, addressValue, asAllocatable(delta)));
return result;
}

@Override
public Value emitAtomicReadAndWrite(Value address, ValueKind<?> kind, Value newValue) {
Variable result = newVariable(kind);
public Value emitAtomicReadAndWrite(LIRKind accessKind, Value address, Value newValue) {
Variable result = newVariable(toRegisterKind(accessKind));
AMD64AddressValue addressValue = asAddressValue(address);
append(new AMD64Move.AtomicReadAndWriteOp((AMD64Kind) kind.getPlatformKind(), result, addressValue, asAllocatable(newValue)));
append(new AMD64Move.AtomicReadAndWriteOp((AMD64Kind) accessKind.getPlatformKind(), result, addressValue, asAllocatable(newValue)));
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,12 @@ public static boolean verifyMoveKinds(ValueKind<?> dst, ValueKind<?> src, Regist
if (srcPlatformKind.equals(dstPlatformKind)) {
return true;
}
// if the register category matches it should be fine, although the kind is different
return config.getRegisterCategory(srcPlatformKind).equals(config.getRegisterCategory(dstPlatformKind));
/*
* The register category should match and srcPlatformKind must be at least as large as
* dstPlatformKind. This size restriction is necessary since we don't preserve
* information about whether to perform a sign or zero extend.
*/
return config.getRegisterCategory(srcPlatformKind).equals(config.getRegisterCategory(dstPlatformKind)) && srcPlatformKind.getSizeInBytes() >= dstPlatformKind.getSizeInBytes();
}
// reference information mismatch
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,22 +126,22 @@ abstract class BlockScope implements AutoCloseable {
/**
* Emit an atomic read-and-add instruction.
*
* @param accessKind the access kind for the value to be written
* @param address address of the value to be read and written
* @param valueKind the access kind for the value to be written
* @param delta the value to be added
*/
default Value emitAtomicReadAndAdd(Value address, ValueKind<?> valueKind, Value delta) {
default Value emitAtomicReadAndAdd(LIRKind accessKind, Value address, Value delta) {
throw GraalError.unimplemented();
}

/**
* Emit an atomic read-and-write instruction.
*
* @param accessKind the access kind for the value to be written
* @param address address of the value to be read and written
* @param valueKind the access kind for the value to be written
* @param newValue the new value to be written
*/
default Value emitAtomicReadAndWrite(Value address, ValueKind<?> valueKind, Value newValue) {
default Value emitAtomicReadAndWrite(LIRKind accessKind, Value address, Value newValue) {
throw GraalError.unimplemented();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import static org.graalvm.compiler.nodeinfo.NodeCycles.CYCLES_8;
import static org.graalvm.compiler.nodeinfo.NodeSize.SIZE_2;

import org.graalvm.compiler.core.common.LIRKind;
import org.graalvm.compiler.core.common.type.Stamp;
import org.graalvm.compiler.graph.NodeClass;
import org.graalvm.compiler.nodeinfo.NodeInfo;
Expand All @@ -44,7 +45,6 @@
import org.graalvm.word.LocationIdentity;

import jdk.vm.ci.meta.Value;
import jdk.vm.ci.meta.ValueKind;

/**
* Represents the lowered version of an atomic read-and-add operation like
Expand All @@ -56,12 +56,10 @@ public final class LoweredAtomicReadAndAddNode extends FixedAccessNode implement
public static final NodeClass<LoweredAtomicReadAndAddNode> TYPE = NodeClass.create(LoweredAtomicReadAndAddNode.class);
@Input ValueNode delta;
@OptionalInput(State) FrameState stateAfter;
private final ValueKind<?> valueKind;

public LoweredAtomicReadAndAddNode(AddressNode address, LocationIdentity location, ValueNode delta, ValueKind<?> valueKind, BarrierType barrierType) {
public LoweredAtomicReadAndAddNode(AddressNode address, LocationIdentity location, ValueNode delta, BarrierType barrierType) {
super(TYPE, address, location, delta.stamp(NodeView.DEFAULT).unrestricted(), barrierType);
this.delta = delta;
this.valueKind = valueKind;
}

@Override
Expand All @@ -83,7 +81,8 @@ public boolean hasSideEffect() {

@Override
public void generate(NodeLIRBuilderTool gen) {
Value result = gen.getLIRGeneratorTool().emitAtomicReadAndAdd(gen.operand(getAddress()), valueKind, gen.operand(delta()));
LIRKind accessKind = gen.getLIRGeneratorTool().getLIRKind(getAccessStamp(NodeView.DEFAULT));
Value result = gen.getLIRGeneratorTool().emitAtomicReadAndAdd(accessKind, gen.operand(getAddress()), gen.operand(delta()));
gen.setResult(this, result);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import static org.graalvm.compiler.nodeinfo.NodeCycles.CYCLES_8;
import static org.graalvm.compiler.nodeinfo.NodeSize.SIZE_2;

import jdk.vm.ci.meta.ValueKind;
import org.graalvm.compiler.core.common.LIRKind;
import org.graalvm.compiler.core.common.type.Stamp;
import org.graalvm.compiler.graph.NodeClass;
import org.graalvm.compiler.nodeinfo.NodeInfo;
Expand All @@ -56,12 +56,10 @@ public final class LoweredAtomicReadAndWriteNode extends FixedAccessNode impleme
public static final NodeClass<LoweredAtomicReadAndWriteNode> TYPE = NodeClass.create(LoweredAtomicReadAndWriteNode.class);
@Input ValueNode newValue;
@OptionalInput(State) FrameState stateAfter;
private final ValueKind<?> valueKind;

public LoweredAtomicReadAndWriteNode(AddressNode address, LocationIdentity location, ValueNode newValue, ValueKind<?> valueKind, BarrierType barrierType) {
public LoweredAtomicReadAndWriteNode(AddressNode address, LocationIdentity location, ValueNode newValue, BarrierType barrierType) {
super(TYPE, address, location, newValue.stamp(NodeView.DEFAULT).unrestricted(), barrierType);
this.newValue = newValue;
this.valueKind = valueKind;
}

@Override
Expand All @@ -84,9 +82,16 @@ public boolean hasSideEffect() {
@Override
public void generate(NodeLIRBuilderTool gen) {
Value emitted = gen.operand(getNewValue());
// In case this node works with compressed objects, the newValue's kind must be used.
ValueKind<? extends ValueKind<?>> actualKind = newValue.stamp(NodeView.DEFAULT).getStackKind().isObject() ? emitted.getValueKind() : this.valueKind;
Value result = gen.getLIRGeneratorTool().emitAtomicReadAndWrite(gen.operand(getAddress()), actualKind, emitted);

LIRKind accessKind;
if (getNewValue().stamp(NodeView.DEFAULT).getStackKind().isObject()) {
// In case this node works with compressed objects, the newValue's kind must be used.
accessKind = (LIRKind) emitted.getValueKind();
} else {
accessKind = gen.getLIRGeneratorTool().getLIRKind(getAccessStamp(NodeView.DEFAULT));
}

Value result = gen.getLIRGeneratorTool().emitAtomicReadAndWrite(accessKind, gen.operand(getAddress()), emitted);
gen.setResult(this, result);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import java.util.BitSet;
import java.util.List;

import org.graalvm.compiler.core.common.LIRKind;
import org.graalvm.compiler.core.common.memory.MemoryOrderMode;
import org.graalvm.compiler.core.common.spi.ForeignCallsProvider;
import org.graalvm.compiler.core.common.spi.MetaAccessExtensionProvider;
Expand Down Expand Up @@ -735,8 +734,7 @@ protected void lowerAtomicReadAndWriteNode(AtomicReadAndWriteNode n) {

AddressNode address = graph.unique(new OffsetAddressNode(n.object(), n.offset()));
BarrierType barrierType = barrierSet.guessStoreBarrierType(n.object(), newValue);
LIRKind lirAccessKind = LIRKind.fromJavaKind(target.arch, valueKind);
LoweredAtomicReadAndWriteNode memoryRead = graph.add(new LoweredAtomicReadAndWriteNode(address, n.getKilledLocationIdentity(), newValue, lirAccessKind, barrierType));
LoweredAtomicReadAndWriteNode memoryRead = graph.add(new LoweredAtomicReadAndWriteNode(address, n.getKilledLocationIdentity(), newValue, barrierType));
memoryRead.setStateAfter(n.stateAfter());

ValueNode readValue = implicitLoadConvert(graph, valueKind, memoryRead);
Expand All @@ -753,8 +751,7 @@ protected void lowerAtomicReadAndAddNode(AtomicReadAndAddNode n) {

AddressNode address = graph.unique(new OffsetAddressNode(n.object(), n.offset()));
BarrierType barrierType = barrierSet.guessStoreBarrierType(n.object(), delta);
LIRKind lirAccessKind = LIRKind.fromJavaKind(target.arch, valueKind);
LoweredAtomicReadAndAddNode memoryRead = graph.add(new LoweredAtomicReadAndAddNode(address, n.getKilledLocationIdentity(), delta, lirAccessKind, barrierType));
LoweredAtomicReadAndAddNode memoryRead = graph.add(new LoweredAtomicReadAndAddNode(address, n.getKilledLocationIdentity(), delta, barrierType));
memoryRead.setStateAfter(n.stateAfter());

ValueNode readValue = implicitLoadConvert(graph, valueKind, memoryRead);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,13 +712,13 @@ public void emitMembar(int barriers) {
}

@Override
public Value emitAtomicReadAndWrite(Value address, ValueKind<?> valueKind, Value newValue) {
public Value emitAtomicReadAndWrite(LIRKind accessKind, Value address, Value newValue) {
LLVMValueRef atomicRMW = builder.buildAtomicXchg(getVal(address), getVal(newValue));
return new LLVMVariable(atomicRMW);
}

@Override
public Value emitAtomicReadAndAdd(Value address, ValueKind<?> valueKind, Value delta) {
public Value emitAtomicReadAndAdd(LIRKind accessKind, Value address, Value delta) {
LLVMValueRef atomicRMW = builder.buildAtomicAdd(getVal(address), getVal(delta));
return new LLVMVariable(atomicRMW);
}
Expand Down

0 comments on commit 38752d2

Please sign in to comment.