Mercurial > hg > truffle
changeset 16073:b38191cd1665
removed code to make recording usages optional for ConstantNodes (GRAAL-508)
| author | Doug Simon <doug.simon@oracle.com> |
|---|---|
| date | Tue, 10 Jun 2014 18:52:20 +0200 |
| parents | 2023d6120416 |
| children | b6ab7e7fa0a5 |
| files | graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/FlowSensitiveReductionTest.java graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraalCompilerTest.java graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/NodeLIRBuilder.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/AheadOfTimeVerificationPhase.java graal/com.oracle.graal.lir/src/com/oracle/graal/lir/gen/LIRGenerator.java graal/com.oracle.graal.lir/src/com/oracle/graal/lir/gen/LIRGeneratorTool.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ConstantNode.java |
| diffstat | 7 files changed, 11 insertions(+), 286 deletions(-) [+] |
line wrap: on
line diff
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/FlowSensitiveReductionTest.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/FlowSensitiveReductionTest.java Tue Jun 10 18:52:20 2014 +0200 @@ -148,9 +148,7 @@ new FlowSensitiveReductionPhase(getMetaAccess()).apply(graph, context); new CanonicalizerPhase(true).apply(graph, context); for (ConstantNode constant : getConstantNodes(graph)) { - if (ConstantNodeRecordsUsages || !constant.gatherUsages(graph).isEmpty()) { - assertTrue("unexpected constant: " + constant, constant.asConstant().isNull() || constant.asConstant().asInt() > 0); - } + assertTrue("unexpected constant: " + constant, constant.asConstant().isNull() || constant.asConstant().asInt() > 0); } } @@ -186,9 +184,7 @@ new FlowSensitiveReductionPhase(getMetaAccess()).apply(graph, new PhaseContext(getProviders(), null)); new CanonicalizerPhase(true).apply(graph, new PhaseContext(getProviders(), null)); for (ConstantNode constant : getConstantNodes(graph)) { - if (ConstantNodeRecordsUsages || !constant.gatherUsages(graph).isEmpty()) { - assertTrue("unexpected constant: " + constant, constant.asConstant().isNull() || constant.asConstant().asInt() > 0); - } + assertTrue("unexpected constant: " + constant, constant.asConstant().isNull() || constant.asConstant().asInt() > 0); } }
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraalCompilerTest.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraalCompilerTest.java Tue Jun 10 18:52:20 2014 +0200 @@ -213,14 +213,8 @@ protected int countUnusedConstants(StructuredGraph graph) { int total = 0; for (ConstantNode node : getConstantNodes(graph)) { - if (!ConstantNodeRecordsUsages) { - if (node.gatherUsages(graph).isEmpty()) { - total++; - } - } else { - if (node.usages().isEmpty()) { - total++; - } + if (node.usages().isEmpty()) { + total++; } } return total;
--- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/NodeLIRBuilder.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/NodeLIRBuilder.java Tue Jun 10 18:52:20 2014 +0200 @@ -26,7 +26,6 @@ import static com.oracle.graal.compiler.GraalDebugConfig.*; import static com.oracle.graal.compiler.common.GraalOptions.*; import static com.oracle.graal.lir.LIR.*; -import static com.oracle.graal.nodes.ConstantNode.*; import java.util.*; import java.util.Map.Entry; @@ -44,7 +43,6 @@ import com.oracle.graal.lir.*; import com.oracle.graal.lir.StandardOp.JumpOp; import com.oracle.graal.lir.gen.*; -import com.oracle.graal.lir.gen.LIRGenerator.LoadConstant; import com.oracle.graal.lir.gen.LIRGenerator.Options; import com.oracle.graal.nodes.*; import com.oracle.graal.nodes.calc.*; @@ -135,48 +133,7 @@ if (nodeOperands == null) { return null; } - Value operand = nodeOperands.get(node); - if (operand == null) { - operand = getConstantOperand(node); - } - return operand; - } - - private Value getConstantOperand(ValueNode node) { - if (!ConstantNodeRecordsUsages) { - Constant value = node.asConstant(); - if (value != null) { - if (gen.canInlineConstant(value)) { - return setResult(node, value); - } else { - Variable loadedValue; - if (gen.getConstantLoads() == null) { - gen.setConstantLoads(new HashMap<>()); - } - LoadConstant load = gen.getConstantLoads().get(value); - assert gen.getCurrentBlock() instanceof Block; - if (load == null) { - int index = gen.getResult().getLIR().getLIRforBlock(gen.getCurrentBlock()).size(); - loadedValue = gen.emitMove(value); - LIRInstruction op = gen.getResult().getLIR().getLIRforBlock(gen.getCurrentBlock()).get(index); - gen.getConstantLoads().put(value, new LoadConstant(loadedValue, gen.getCurrentBlock(), index, op)); - } else { - AbstractBlock<?> dominator = ControlFlowGraph.commonDominator((Block) load.getBlock(), (Block) gen.getCurrentBlock()); - loadedValue = load.getVariable(); - if (dominator != load.getBlock()) { - load.unpin(gen.getResult().getLIR()); - } else { - assert load.getBlock() != gen.getCurrentBlock() || load.getIndex() < gen.getResult().getLIR().getLIRforBlock(gen.getCurrentBlock()).size(); - } - load.setBlock(dominator); - } - return loadedValue; - } - } - } else { - // Constant is loaded by ConstantNode.generate() - } - return null; + return nodeOperands.get(node); } public ValueNode valueForOperand(Value value) { @@ -252,10 +209,7 @@ if (Options.TraceLIRGeneratorLevel.getValue() >= 3) { TTY.println("LIRGen for " + instr); } - if (!ConstantNodeRecordsUsages && instr instanceof ConstantNode) { - // Loading of constants is done lazily by operand() - - } else if (instr instanceof ValueNode) { + if (instr instanceof ValueNode) { ValueNode valueNode = (ValueNode) instr; Value operand = getOperand(valueNode); if (operand == null) {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/AheadOfTimeVerificationPhase.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/AheadOfTimeVerificationPhase.java Tue Jun 10 18:52:20 2014 +0200 @@ -45,7 +45,7 @@ @Override protected boolean verify(StructuredGraph graph, PhaseContext context) { for (ConstantNode node : getConstantNodes(graph)) { - if (node.recordsUsages() || !node.gatherUsages(graph).isEmpty()) { + if (node.recordsUsages()) { if (isObject(node) && !isNullReference(node) && !isInternedString(node) && !isDirectMethodHandle(node) && !isBoundMethodHandle(node)) { throw new VerificationError("illegal object constant: " + node); }
--- a/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/gen/LIRGenerator.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/gen/LIRGenerator.java Tue Jun 10 18:52:20 2014 +0200 @@ -24,8 +24,6 @@ import static com.oracle.graal.api.code.ValueUtil.*; import static com.oracle.graal.api.meta.Value.*; -import static com.oracle.graal.compiler.common.GraalOptions.*; -import static com.oracle.graal.lir.LIR.*; import static com.oracle.graal.lir.LIRValueUtil.*; import java.util.*; @@ -42,7 +40,6 @@ import com.oracle.graal.lir.*; import com.oracle.graal.lir.StandardOp.BlockEndOp; import com.oracle.graal.lir.StandardOp.LabelOp; -import com.oracle.graal.lir.StandardOp.NoOp; import com.oracle.graal.options.*; /** @@ -64,97 +61,6 @@ private AbstractBlock<?> currentBlock; - /** - * Handle for an operation that loads a constant into a variable. The operation starts in the - * first block where the constant is used but will eventually be - * {@linkplain LIRGenerator#insertConstantLoads() moved} to a block dominating all usages of the - * constant. - */ - public static class LoadConstant implements Comparable<LoadConstant> { - /** - * The index of {@link #op} within {@link #block}'s instruction list or -1 if {@code op} is - * to be moved to a dominator block. - */ - int index; - - /** - * The operation that loads the constant. - */ - private final LIRInstruction op; - - /** - * The block that does or will contain {@link #op}. This is initially the block where the - * first usage of the constant is seen during LIR generation. - */ - AbstractBlock<?> block; - - /** - * The variable into which the constant is loaded. - */ - final Variable variable; - - public LoadConstant(Variable variable, AbstractBlock<?> block, int index, LIRInstruction op) { - this.variable = variable; - this.block = block; - this.index = index; - this.op = op; - } - - /** - * Sorts {@link LoadConstant} objects according to their enclosing blocks. This is used to - * group loads per block in {@link LIRGenerator#insertConstantLoads()}. - */ - public int compareTo(LoadConstant o) { - if (block.getId() < o.block.getId()) { - return -1; - } - if (block.getId() > o.block.getId()) { - return 1; - } - return 0; - } - - @Override - public String toString() { - return block + "#" + op; - } - - /** - * Removes the {@link #op} from its original location if it is still at that location. - */ - public void unpin(LIR lir) { - if (index >= 0) { - // Replace the move with a filler op so that the operation - // list does not need to be adjusted. - List<LIRInstruction> instructions = lir.getLIRforBlock(block); - instructions.set(index, new NoOp(null, -1)); - index = -1; - } - } - - public AbstractBlock<?> getBlock() { - return block; - } - - public void setBlock(AbstractBlock<?> block) { - this.block = block; - } - - public Variable getVariable() { - return variable; - } - - public int getIndex() { - return index; - } - - public void setIndex(int index) { - this.index = index; - } - } - - private Map<Constant, LoadConstant> constantLoads; - private LIRGenerationResult res; public LIRGenerator(CodeGenProviders providers, CallingConvention cc, LIRGenerationResult res) { @@ -409,83 +315,6 @@ @Override public void beforeRegisterAllocation() { - insertConstantLoads(); - } - - /** - * Moves deferred {@linkplain LoadConstant loads} of constants into blocks dominating all usages - * of the constant. Any operations inserted into a block are guaranteed to be immediately prior - * to the first control flow instruction near the end of the block. - */ - private void insertConstantLoads() { - if (constantLoads != null) { - // Remove loads where all usages are in the same block. - for (Iterator<Map.Entry<Constant, LoadConstant>> iter = constantLoads.entrySet().iterator(); iter.hasNext();) { - LoadConstant lc = iter.next().getValue(); - - // Move loads of constant outside of loops - if (OptScheduleOutOfLoops.getValue()) { - AbstractBlock<?> outOfLoopDominator = lc.block; - while (outOfLoopDominator.getLoop() != null) { - outOfLoopDominator = outOfLoopDominator.getDominator(); - } - if (outOfLoopDominator != lc.block) { - lc.unpin(res.getLIR()); - lc.block = outOfLoopDominator; - } - } - - if (lc.index != -1) { - assert res.getLIR().getLIRforBlock(lc.block).get(lc.index) == lc.op; - iter.remove(); - } - } - if (constantLoads.isEmpty()) { - return; - } - - // Sorting groups the loads per block. - LoadConstant[] groupedByBlock = constantLoads.values().toArray(new LoadConstant[constantLoads.size()]); - Arrays.sort(groupedByBlock); - - int groupBegin = 0; - while (true) { - int groupEnd = groupBegin + 1; - AbstractBlock<?> block = groupedByBlock[groupBegin].block; - while (groupEnd < groupedByBlock.length && groupedByBlock[groupEnd].block == block) { - groupEnd++; - } - int groupSize = groupEnd - groupBegin; - - List<LIRInstruction> ops = res.getLIR().getLIRforBlock(block); - int lastIndex = ops.size() - 1; - assert ops.get(lastIndex) instanceof BlockEndOp; - int insertionIndex = lastIndex; - for (int i = Math.max(0, lastIndex - MAX_EXCEPTION_EDGE_OP_DISTANCE_FROM_END); i < lastIndex; i++) { - if (getExceptionEdge(ops.get(i)) != null) { - insertionIndex = i; - break; - } - } - - if (groupSize == 1) { - ops.add(insertionIndex, groupedByBlock[groupBegin].op); - } else { - assert groupSize > 1; - List<LIRInstruction> moves = new ArrayList<>(groupSize); - for (int i = groupBegin; i < groupEnd; i++) { - moves.add(groupedByBlock[i].op); - } - ops.addAll(insertionIndex, moves); - } - - if (groupEnd == groupedByBlock.length) { - break; - } - groupBegin = groupEnd; - } - constantLoads = null; - } } /** @@ -573,12 +402,4 @@ public LIRGenerationResult getResult() { return res; } - - public Map<Constant, LoadConstant> getConstantLoads() { - return constantLoads; - } - - public void setConstantLoads(Map<Constant, LoadConstant> constantLoads) { - this.constantLoads = constantLoads; - } }
--- a/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/gen/LIRGeneratorTool.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/gen/LIRGeneratorTool.java Tue Jun 10 18:52:20 2014 +0200 @@ -22,8 +22,6 @@ */ package com.oracle.graal.lir.gen; -import java.util.*; - import com.oracle.graal.api.code.*; import com.oracle.graal.api.meta.*; import com.oracle.graal.compiler.common.*; @@ -31,7 +29,6 @@ import com.oracle.graal.compiler.common.cfg.*; import com.oracle.graal.compiler.common.spi.*; import com.oracle.graal.lir.*; -import com.oracle.graal.lir.gen.LIRGenerator.*; public interface LIRGeneratorTool extends ArithmeticLIRGenerator { @@ -55,10 +52,6 @@ void doBlockEnd(AbstractBlock<?> block); - Map<Constant, LoadConstant> getConstantLoads(); - - void setConstantLoads(Map<Constant, LoadConstant> constantLoads); - Value emitLoad(PlatformKind kind, Value address, LIRFrameState state); void emitStore(PlatformKind kind, Value address, Value input, LIRFrameState state);
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ConstantNode.java Tue Jun 10 18:50:26 2014 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ConstantNode.java Tue Jun 10 18:52:20 2014 +0200 @@ -67,32 +67,9 @@ return value; } - /** - * Used to measure the impact of ConstantNodes not recording their usages. This and all code - * predicated on this value being true will be removed at some point. - */ - public static final boolean ConstantNodeRecordsUsages = Boolean.parseBoolean(System.getProperty("graal.constantNodeRecordsUsages", "true")); - @Override public boolean recordsUsages() { - return ConstantNodeRecordsUsages; - } - - /** - * Computes the usages of this node by iterating over all the nodes in the graph, searching for - * those that have this node as an input. - */ - public List<Node> gatherUsages(StructuredGraph graph) { - assert !ConstantNodeRecordsUsages; - List<Node> usages = new ArrayList<>(); - for (Node node : graph.getNodes()) { - for (Node input : node.inputs()) { - if (input == this) { - usages.add(node); - } - } - } - return usages; + return true; } /** @@ -104,25 +81,15 @@ } /** - * Replaces this node at its usages with another node. If {@link #ConstantNodeRecordsUsages} is - * false, this is an expensive operation that should only be used in test/verification/AOT code. + * Replaces this node at its usages with another node. */ public void replace(StructuredGraph graph, Node replacement) { - if (!recordsUsages()) { - List<Node> usages = gatherUsages(graph); - for (Node usage : usages) { - usage.replaceFirstInput(this, replacement); - } - graph.removeFloating(this); - } else { - assert graph == graph(); - graph().replaceFloating(this, replacement); - } + assert graph == graph(); + graph().replaceFloating(this, replacement); } @Override public void generate(NodeLIRBuilderTool gen) { - assert ConstantNodeRecordsUsages : "LIR generator should generate constants per-usage"; if (gen.getLIRGeneratorTool().canInlineConstant(value) || onlyUsedInVirtualState()) { gen.setResult(this, value); } else {