changeset 23122:a720fbf1810a

TraceRA: fix stack-to-stack moves in the assignment phase.
author Josef Eisl <josef.eisl@jku.at>
date Mon, 30 Nov 2015 15:01:06 +0100
parents d736437ef3df
children fa5100c27dac
files graal/com.oracle.graal.lir/src/com/oracle/graal/lir/alloc/trace/TraceLinearScanAssignLocationsPhase.java graal/com.oracle.graal.lir/src/com/oracle/graal/lir/alloc/trace/TraceRegisterAllocationFixupPhase.java
diffstat 2 files changed, 14 insertions(+), 41 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/alloc/trace/TraceLinearScanAssignLocationsPhase.java	Mon Nov 30 14:50:33 2015 +0100
+++ b/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/alloc/trace/TraceLinearScanAssignLocationsPhase.java	Mon Nov 30 15:01:06 2015 +0100
@@ -150,7 +150,7 @@
                  * instruction is a branch, spill moves are inserted before this branch and so the
                  * wrong operand would be returned (spill moves at block boundaries are not
                  * considered in the live ranges of intervals).
-                 * 
+                 *
                  * Solution: use the first opId of the branch target block instead.
                  */
                 final LIRInstruction instr = allocator.getLIR().getLIRforBlock(block).get(allocator.getLIR().getLIRforBlock(block).size() - 1);
@@ -187,8 +187,7 @@
                      * this can happen when spill-moves are removed in eliminateSpillMoves
                      */
                     hasDead = true;
-                } else if (assignLocations(op)) {
-                    instructions.set(j, null);
+                } else if (assignLocations(op, instructions, j)) {
                     hasDead = true;
                 }
             }
@@ -203,10 +202,12 @@
          * Assigns the operand of an {@link LIRInstruction}.
          *
          * @param op The {@link LIRInstruction} that should be colored.
-         * @return {@code true} if the instruction should be deleted.
+         * @param j The index of {@code op} in the {@code instructions} list.
+         * @param instructions The instructions of the current block.
+         * @return {@code true} if the instruction was deleted.
          */
-        private boolean assignLocations(LIRInstruction op) {
-            assert op != null;
+        private boolean assignLocations(LIRInstruction op, List<LIRInstruction> instructions, int j) {
+            assert op != null && instructions.get(j) == op;
             if (TraceRAshareSpillInformation.getValue()) {
                 if (op instanceof BlockEndOp) {
                     ((BlockEndOp) op).forEachOutgoingValue(colorOutgoingIncomingValues);
@@ -225,6 +226,7 @@
                      * kicked out in LinearScanWalker.splitForSpilling(). When kicking out such an
                      * interval this move operation was already generated.
                      */
+                    instructions.set(j, null);
                     return true;
                 }
             }
@@ -241,6 +243,12 @@
             if (op instanceof ValueMoveOp) {
                 ValueMoveOp move = (ValueMoveOp) op;
                 if (move.getInput().equals(move.getResult())) {
+                    instructions.set(j, null);
+                    return true;
+                }
+                if (isStackSlotValue(move.getInput()) && isStackSlotValue(move.getResult())) {
+                    // rewrite stack to stack moves
+                    instructions.set(j, spillMoveFactory.createStackMove(move.getResult(), move.getInput()));
                     return true;
                 }
             }
--- a/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/alloc/trace/TraceRegisterAllocationFixupPhase.java	Mon Nov 30 14:50:33 2015 +0100
+++ b/graal/com.oracle.graal.lir/src/com/oracle/graal/lir/alloc/trace/TraceRegisterAllocationFixupPhase.java	Mon Nov 30 15:01:06 2015 +0100
@@ -22,8 +22,6 @@
  */
 package com.oracle.graal.lir.alloc.trace;
 
-import static com.oracle.graal.lir.LIRValueUtil.isStackSlotValue;
-
 import java.util.List;
 
 import jdk.vm.ci.code.TargetDescription;
@@ -32,10 +30,7 @@
 import com.oracle.graal.debug.Debug;
 import com.oracle.graal.debug.Indent;
 import com.oracle.graal.lir.LIR;
-import com.oracle.graal.lir.LIRInstruction;
-import com.oracle.graal.lir.StandardOp.ValueMoveOp;
 import com.oracle.graal.lir.gen.LIRGenerationResult;
-import com.oracle.graal.lir.gen.LIRGeneratorTool.MoveFactory;
 import com.oracle.graal.lir.ssi.SSIUtil;
 
 /**
@@ -46,11 +41,7 @@
 
     @Override
     protected <B extends AbstractBlockBase<B>> void run(TargetDescription target, LIRGenerationResult lirGenRes, List<B> codeEmittingOrder, List<B> linearScanOrder, TraceAllocationContext context) {
-        MoveFactory spillMoveFactory = context.spillMoveFactory;
         LIR lir = lirGenRes.getLIR();
-        if (replaceStackToStackMoves(lir, spillMoveFactory)) {
-            Debug.dump(lir, "After fixing stack to stack moves");
-        }
         /*
          * Incoming Values are needed for the RegisterVerifier, otherwise SIGMAs/PHIs where the Out
          * and In value matches (ie. there is no resolution move) are falsely detected as errors.
@@ -65,31 +56,5 @@
                 SSIUtil.removeOutgoing(lir, block);
             }
         }
-
     }
-
-    /**
-     * Fixup stack to stack moves introduced by stack arguments.
-     *
-     * TODO (je) find a better solution.
-     */
-    private static boolean replaceStackToStackMoves(LIR lir, MoveFactory spillMoveFactory) {
-        boolean changed = false;
-        for (AbstractBlockBase<?> block : lir.getControlFlowGraph().getBlocks()) {
-            List<LIRInstruction> instructions = lir.getLIRforBlock(block);
-            for (int i = 0; i < instructions.size(); i++) {
-                LIRInstruction inst = instructions.get(i);
-
-                if (inst instanceof ValueMoveOp) {
-                    ValueMoveOp move = (ValueMoveOp) inst;
-                    if (isStackSlotValue(move.getInput()) && isStackSlotValue(move.getResult())) {
-                        instructions.set(i, spillMoveFactory.createStackMove(move.getResult(), move.getInput()));
-                        changed = true;
-                    }
-                }
-            }
-        }
-        return changed;
-    }
-
 }