Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Omit unnecessary AMD64 TEST instruction #8539

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Omit unnecessary AMD64 TEST instruction #8539

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd49ced
Refactored DivRem peephole optimization into separate method
petermz Feb 22, 2024
3afe512
Omit amd64 TEST instruction when flags are already set
petermz Feb 26, 2024
37e1e75
Style fixes
petermz Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
139 changes: 89 additions & 50 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/amd64/AMD64NodeLIRBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,31 @@
import jdk.graal.compiler.core.gen.NodeLIRBuilder;
import jdk.graal.compiler.debug.GraalError;
import jdk.graal.compiler.core.common.util.CompilationAlarm;
import jdk.graal.compiler.graph.NodeClass;
import jdk.graal.compiler.lir.LIRFrameState;
import jdk.graal.compiler.lir.LIRValueUtil;
import jdk.graal.compiler.lir.amd64.AMD64Call;
import jdk.graal.compiler.lir.amd64.AMD64ControlFlow;
import jdk.graal.compiler.lir.gen.LIRGeneratorTool;
import jdk.graal.compiler.nodes.DeoptimizingNode;
import jdk.graal.compiler.nodes.FixedNode;
import jdk.graal.compiler.nodes.FixedWithNextNode;
import jdk.graal.compiler.nodes.IfNode;
import jdk.graal.compiler.nodes.IndirectCallTargetNode;
import jdk.graal.compiler.nodes.NodeView;
import jdk.graal.compiler.nodes.StructuredGraph;
import jdk.graal.compiler.nodes.ValueNode;
import jdk.graal.compiler.nodes.calc.AndNode;
import jdk.graal.compiler.nodes.calc.CompareNode;
import jdk.graal.compiler.nodes.calc.IntegerDivRemNode;
import jdk.graal.compiler.nodes.calc.IntegerDivRemNode.Op;
import jdk.graal.compiler.nodes.calc.OrNode;
import jdk.graal.compiler.nodes.calc.XorNode;
import jdk.graal.compiler.debug.Assertions;

import jdk.vm.ci.amd64.AMD64;
import jdk.vm.ci.amd64.AMD64Kind;
import jdk.vm.ci.meta.AllocatableValue;
import jdk.vm.ci.meta.PlatformKind;
import jdk.vm.ci.meta.Value;

public abstract class AMD64NodeLIRBuilder extends NodeLIRBuilder {
Expand All @@ -62,61 +71,91 @@ protected void emitIndirectCall(IndirectCallTargetNode callTarget, Value result,

@Override
protected boolean peephole(ValueNode valueNode) {
if (valueNode instanceof IntegerDivRemNode) {
AMD64ArithmeticLIRGenerator arithmeticGen = (AMD64ArithmeticLIRGenerator) gen.getArithmetic();
IntegerDivRemNode divRem = (IntegerDivRemNode) valueNode;
FixedNode node = divRem.next();
while (true) { // TERMINATION ARGUMENT: iterating next nodes
CompilationAlarm.checkProgress(valueNode.graph());
if (node instanceof IfNode) {
IfNode ifNode = (IfNode) node;
double probability = ifNode.getTrueSuccessorProbability();
if (probability == 1.0) {
node = ifNode.trueSuccessor();
} else if (probability == 0.0) {
node = ifNode.falseSuccessor();
} else {
break;
}
} else if (!(node instanceof FixedWithNextNode)) {
if (valueNode instanceof IntegerDivRemNode divRemNode) {
return peepholeDivRem(divRemNode);
} else if (valueNode instanceof IfNode ifNode) {
return peepholeIf(ifNode);
}
return false;
}

protected boolean peepholeDivRem(IntegerDivRemNode divRem) {
AMD64ArithmeticLIRGenerator arithmeticGen = (AMD64ArithmeticLIRGenerator) gen.getArithmetic();
FixedNode node = divRem.next();
while (true) { // TERMINATION ARGUMENT: iterating next nodes
CompilationAlarm.checkProgress(divRem.graph());
if (node instanceof IfNode ifNode) {
double probability = ifNode.getTrueSuccessorProbability();
if (probability == 1.0) {
node = ifNode.trueSuccessor();
} else if (probability == 0.0) {
node = ifNode.falseSuccessor();
} else {
break;
}
} else if (!(node instanceof FixedWithNextNode)) {
break;
}

FixedWithNextNode fixedWithNextNode = (FixedWithNextNode) node;
if (fixedWithNextNode instanceof IntegerDivRemNode) {
IntegerDivRemNode otherDivRem = (IntegerDivRemNode) fixedWithNextNode;
if (divRem.getOp() != otherDivRem.getOp() && divRem.getType() == otherDivRem.getType()) {
if (otherDivRem.getX() == divRem.getX() && otherDivRem.getY() == divRem.getY() && !hasOperand(otherDivRem)) {
Value[] results;
switch (divRem.getType()) {
case SIGNED:
results = arithmeticGen.emitSignedDivRem(operand(divRem.getX()), operand(divRem.getY()), state((DeoptimizingNode) valueNode));
break;
case UNSIGNED:
results = arithmeticGen.emitUnsignedDivRem(operand(divRem.getX()), operand(divRem.getY()), state((DeoptimizingNode) valueNode));
break;
default:
throw GraalError.shouldNotReachHereUnexpectedValue(divRem.getType()); // ExcludeFromJacocoGeneratedReport
}
switch (divRem.getOp()) {
case DIV:
assert otherDivRem.getOp() == Op.REM : Assertions.errorMessage(valueNode, otherDivRem);
setResult(divRem, results[0]);
setResult(otherDivRem, results[1]);
break;
case REM:
assert otherDivRem.getOp() == Op.DIV : Assertions.errorMessage(valueNode, otherDivRem);
setResult(divRem, results[1]);
setResult(otherDivRem, results[0]);
break;
default:
throw GraalError.shouldNotReachHereUnexpectedValue(divRem.getOp()); // ExcludeFromJacocoGeneratedReport
}
return true;
FixedWithNextNode fixedWithNextNode = (FixedWithNextNode) node;
if (fixedWithNextNode instanceof IntegerDivRemNode otherDivRem) {
if (divRem.getOp() != otherDivRem.getOp() && divRem.getType() == otherDivRem.getType()) {
if (otherDivRem.getX() == divRem.getX() && otherDivRem.getY() == divRem.getY() && !hasOperand(otherDivRem)) {
Value[] results = switch (divRem.getType()) {
case SIGNED -> arithmeticGen.emitSignedDivRem(operand(divRem.getX()), operand(divRem.getY()), state(divRem));
case UNSIGNED -> arithmeticGen.emitUnsignedDivRem(operand(divRem.getX()), operand(divRem.getY()), state(divRem));
default -> throw GraalError.shouldNotReachHereUnexpectedValue(divRem.getType()); // ExcludeFromJacocoGeneratedReport
};
switch (divRem.getOp()) {
case DIV:
assert otherDivRem.getOp() == Op.REM : Assertions.errorMessage(divRem, otherDivRem);
setResult(divRem, results[0]);
setResult(otherDivRem, results[1]);
break;
case REM:
assert otherDivRem.getOp() == Op.DIV : Assertions.errorMessage(divRem, otherDivRem);
setResult(divRem, results[1]);
setResult(otherDivRem, results[0]);
break;
default:
throw GraalError.shouldNotReachHereUnexpectedValue(divRem.getOp()); // ExcludeFromJacocoGeneratedReport
}
return true;
}
}
}
node = fixedWithNextNode.next();
}
return false;
}

private boolean peepholeIf(IfNode ifNode) {
if (ifNode.condition() instanceof CompareNode cmpNode) {
PlatformKind kind = gen.getLIRKind(cmpNode.getX().stamp(NodeView.DEFAULT)).getPlatformKind();
if (kind != AMD64Kind.SINGLE && kind != AMD64Kind.DOUBLE) {
Value c = operand(cmpNode.getX());
ValueNode opNode = cmpNode.getY();
if (!LIRValueUtil.isJavaConstant(c)) {
c = operand(opNode);
opNode = cmpNode.getX();
if (!LIRValueUtil.isJavaConstant(c)) {
return false;
}
}
if (opNode.hasExactlyOneUsage() && cmpNode.hasExactlyOneUsage() &&
LIRValueUtil.asJavaConstant(c).isDefaultForKind()) {
NodeClass<?> nodeClass = opNode.getNodeClass();
if (nodeClass == AndNode.TYPE ||
nodeClass == OrNode.TYPE ||
nodeClass == XorNode.TYPE) {
gen.append(new AMD64ControlFlow.BranchOp(
((CompareNode) ifNode.condition()).condition().asCondition(),
getLIRBlock(ifNode.trueSuccessor()),
getLIRBlock(ifNode.falseSuccessor()),
ifNode.probability(ifNode.trueSuccessor())));
return true;
}
}
node = fixedWithNextNode.next();
}
}
return false;
Expand Down