[rvm-commits] code: 5 new changesets

[<jik...@li...>]

details:   http://hg.code.sourceforge.net/p/jikesrvm/code/rev/009af8a1cc71
changeset: 10752:009af8a1cc71
user:      Erik Brangs <eri...@gm...>
date:      Fri May 23 15:27:00 2014 +0200
description:
Correct Javadoc for markDirty in ScratchMap.


details:   http://hg.code.sourceforge.net/p/jikesrvm/code/rev/acf2e8f7c2fb
changeset: 10753:acf2e8f7c2fb
user:      Erik Brangs <eri...@gm...>
date:      Fri May 23 15:28:39 2014 +0200
description:
RVM-1077 : Add a unit test for org.jikesrvm.compilers.opt.regalloc.ScratchMap.


details:   http://hg.code.sourceforge.net/p/jikesrvm/code/rev/b095895ef5a7
changeset: 10754:b095895ef5a7
user:      Erik Brangs <eri...@gm...>
date:      Fri May 23 15:52:13 2014 +0200
description:
Fix a bug that prevented removal of SymbolInterval instances in endSymbolicInterval().

This bug was revealed by FindBugs.


details:   http://hg.code.sourceforge.net/p/jikesrvm/code/rev/4874964faa69
changeset: 10755:4874964faa69
user:      Erik Brangs <eri...@gm...>
date:      Fri May 23 15:56:36 2014 +0200
description:
Change toString() in ScratchMap to use StringBuilder.


details:   http://hg.code.sourceforge.net/p/jikesrvm/code/rev/60aacb09bcdf
changeset: 10756:60aacb09bcdf
user:      Erik Brangs <eri...@gm...>
date:      Fri May 23 16:02:32 2014 +0200
description:
Change visibility in the Interval classes in ScratchMap to reflect current use of the classes.


diffstat:

 rvm/src/org/jikesrvm/compilers/opt/regalloc/ScratchMap.java          |   33 +-
 rvm/test-src/org/jikesrvm/compilers/opt/regalloc/ScratchMapTest.java |  176 ++++++++++
 2 files changed, 195 insertions(+), 14 deletions(-)

diffs (282 lines):

diff --git a/rvm/src/org/jikesrvm/compilers/opt/regalloc/ScratchMap.java b/rvm/src/org/jikesrvm/compilers/opt/regalloc/ScratchMap.java
--- a/rvm/src/org/jikesrvm/compilers/opt/regalloc/ScratchMap.java
+++ b/rvm/src/org/jikesrvm/compilers/opt/regalloc/ScratchMap.java
@@ -77,7 +77,7 @@
 
     SymbolicInterval i = (SymbolicInterval) pending.get(r);
     i.end = end;
-    pending.remove(i);
+    pending.remove(r);
   }
 
   /**
@@ -165,8 +165,12 @@
   }
 
   /**
-   * Note that at GC point s, the real value of register symb is cached in
-   * a dirty scratch register.
+   * Records that the real value of a symbolic register is cached in
+   * a dirty scratch register at a given instruction that is a GC point.
+   *
+   * @param s an instruction that is a GC point. Note: it is the caller's
+   *    responsibility to check this
+   * @param symb the symbolic register
    */
   public void markDirty(Instruction s, Register symb) {
     HashSet<Register> set = dirtyMap.get(s);
@@ -195,13 +199,14 @@
    */
   @Override
   public String toString() {
-    String result = "";
+    StringBuilder result = new StringBuilder();
     for (ArrayList<Interval> v : map.values()) {
       for (Interval i : v) {
-        result += i + "\n";
+        result.append(i);
+        result.append("\n");
       }
     }
-    return result;
+    return result.toString();
   }
 
   /**
@@ -211,27 +216,27 @@
     /**
      * The instruction before which the scratch range begins.
      */
-    Instruction begin;
+    protected Instruction begin;
     /**
      * The instruction before which the scratch range ends.
      */
-    Instruction end;
+    protected Instruction end;
     /**
      * The physical scratch register or register evicted.
      */
-    final Register scratch;
+    protected final Register scratch;
 
     /**
      * Initialize scratch register
      */
-    Interval(Register scratch) {
+    protected Interval(Register scratch) {
       this.scratch = scratch;
     }
 
     /**
      * Does this interval contain the instruction numbered n?
      */
-    final boolean contains(int n) {
+    protected final boolean contains(int n) {
       return (begin.scratch <= n && end.scratch > n);
     }
   }
@@ -242,11 +247,11 @@
    *
    * Note that this interval must not span a basic block.
    */
-  static final class SymbolicInterval extends Interval {
+  private static final class SymbolicInterval extends Interval {
     /**
      * The symbolic register
      */
-    final Register symbolic;
+    private final Register symbolic;
 
     SymbolicInterval(Register symbolic, Register scratch) {
       super(scratch);
@@ -270,7 +275,7 @@
    *
    * Note that this interval must not span a basic block.
    */
-  static final class PhysicalInterval extends Interval {
+  private static final class PhysicalInterval extends Interval {
     PhysicalInterval(Register scratch) {
       super(scratch);
     }
diff --git a/rvm/test-src/org/jikesrvm/compilers/opt/regalloc/ScratchMapTest.java b/rvm/test-src/org/jikesrvm/compilers/opt/regalloc/ScratchMapTest.java
new file mode 100644
--- /dev/null
+++ b/rvm/test-src/org/jikesrvm/compilers/opt/regalloc/ScratchMapTest.java
@@ -0,0 +1,176 @@
+/*
+ *  This file is part of the Jikes RVM project (http://jikesrvm.org).
+ *
+ *  This file is licensed to You under the Eclipse Public License (EPL);
+ *  You may not use this file except in compliance with the License. You
+ *  may obtain a copy of the License at
+ *
+ *      http://www.opensource.org/licenses/eclipse-1.0.php
+ *
+ *  See the COPYRIGHT.txt file distributed with this work for information
+ *  regarding copyright ownership.
+ */
+package org.jikesrvm.compilers.opt.regalloc;
+
+import static org.junit.Assert.*;
+import static org.hamcrest.CoreMatchers.*;
+import static org.jikesrvm.compilers.opt.ir.Operators.INT_ADD;
+import static org.jikesrvm.compilers.opt.ir.Operators.FENCE;
+
+import org.jikesrvm.classloader.TypeReference;
+import org.jikesrvm.compilers.opt.ir.Binary;
+import org.jikesrvm.compilers.opt.ir.Empty;
+import org.jikesrvm.compilers.opt.ir.Instruction;
+import org.jikesrvm.compilers.opt.ir.Register;
+import org.jikesrvm.compilers.opt.ir.operand.RegisterOperand;
+import org.jikesrvm.junit.runners.RequiresJikesRVM;
+import org.jikesrvm.junit.runners.VMRequirements;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+
+@RunWith(VMRequirements.class)
+public class ScratchMapTest {
+
+  private ScratchMap scratchMap;
+
+  @Before
+  public void createNewScratchMap() {
+    scratchMap = new ScratchMap();
+  }
+
+  @Test
+  public void newMapIsEmpty() {
+    ScratchMap scratchMap = new ScratchMap();
+    assertThat(scratchMap.isEmpty(), is(true));
+  }
+
+  @Category(RequiresJikesRVM.class) // because of TypeReference
+  @Test
+  public void markDirtyMarksRegistersAsDirty() {
+    Register resultReg = createRegister(0);
+    RegisterOperand result = new RegisterOperand(resultReg, TypeReference.Int);
+    Register op1Reg = new Register(1);
+    Register op2Reg = new Register(2);
+    RegisterOperand op1 = new RegisterOperand(op1Reg, TypeReference.Int);
+    RegisterOperand op2 = new RegisterOperand(op2Reg, TypeReference.Int);
+    Instruction add = Binary.create(INT_ADD, result, op1, op2);
+
+    scratchMap.markDirty(add, op2Reg);
+    assertThat(scratchMap.isDirty(add, op2Reg), is(true));
+  }
+
+  @Category(RequiresJikesRVM.class) // because of TypeReference
+  @Test
+  public void markingRegistersAsDirtyMoreThanOnceIsHarmless() {
+    Register resultReg = createRegister(0);
+    RegisterOperand result = new RegisterOperand(resultReg, TypeReference.Int);
+    Register op1Reg = new Register(1);
+    Register op2Reg = new Register(2);
+    RegisterOperand op1 = new RegisterOperand(op1Reg, TypeReference.Int);
+    RegisterOperand op2 = new RegisterOperand(op2Reg, TypeReference.Int);
+    Instruction add = Binary.create(INT_ADD, result, op1, op2);
+
+    scratchMap.markDirty(add, op2Reg);
+    scratchMap.markDirty(add, op2Reg);
+    assertThat(scratchMap.isDirty(add, op2Reg), is(true));
+  }
+
+  @Test
+  public void markDirtyDoesNotCheckThatRegisterIsUsedInInstruction() {
+    Register symb = createRegister(0);
+    Instruction inst = createInstruction();
+    scratchMap.markDirty(inst, symb);
+    assertThat(scratchMap.isDirty(inst, symb), is(true));
+  }
+
+  @Test
+  public void beginSymbolicIntervalCreatesANewInterval() {
+    Register symb = createRegister(0);
+    Register scratch = createRegister(0);
+    Instruction inst = createInstruction();
+    scratchMap.beginSymbolicInterval(symb, scratch, inst);
+    assertThat(scratchMap.isEmpty(), is(false));
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void endSymbolicIntervalRemovesInformationAboutScratchRegisterFromPendingMap() {
+    Register symb = createRegister(0);
+    Register scratch = createRegister(1);
+    Instruction begin = createInstruction();
+    scratchMap.beginSymbolicInterval(symb, scratch, begin);
+    Instruction end = createInstruction();
+    scratchMap.endSymbolicInterval(symb, end);
+
+    scratchMap.endSymbolicInterval(symb, end);
+  }
+
+  private Register createRegister(int regNum) {
+    return new Register(regNum);
+  }
+
+  @Test
+  public void beginSymbolicIntervalAllowsRegisterAndItsScratchRegisterToBeTheSame() {
+    Register symb = createRegister(0);
+    Instruction inst = createInstruction();
+    scratchMap.beginSymbolicInterval(symb, symb, inst);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void endSymbolicIntervalForNotStartedIntervalCausesNPE() {
+    Register symb = createRegister(0);
+    Instruction inst = createInstruction();
+    scratchMap.endSymbolicInterval(symb, inst);
+  }
+
+  private Instruction createInstruction() {
+    Instruction fence = Empty.create(FENCE);
+    return fence;
+  }
+
+  @Test
+  public void beginScratchIntervalCreatesANewInterval() {
+    Register scratch = createRegister(0);
+    Instruction inst = createInstruction();
+    scratchMap.beginScratchInterval(scratch, inst);
+    assertThat(scratchMap.isEmpty(), is(false));
+  }
+
+  @Test
+  public void beginScratchIntervalSavesInformationAboutScratchRegister() {
+    Register scratch = createRegister(0);
+    Instruction begin = createInstruction();
+    int instNumber = 2;
+    begin.scratch = instNumber;
+    scratchMap.beginScratchInterval(scratch, begin);
+    Instruction end = createInstruction();
+    end.scratch = instNumber + 1;
+    scratchMap.endScratchInterval(scratch, end);
+
+    Register scratchReg = scratchMap.getScratch(scratch, instNumber);
+    assertThat(scratchReg, is(scratch));
+    assertThat(scratchMap.isScratch(scratchReg, instNumber), is(true));
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void endScratchIntervalRemovesInformationAboutScratchRegisterFromPendingMap() {
+    Register scratch = createRegister(0);
+    Instruction begin = createInstruction();
+    scratchMap.beginScratchInterval(scratch, begin);
+    Instruction end = createInstruction();
+    scratchMap.endScratchInterval(scratch, end);
+
+    scratchMap.endScratchInterval(scratch, end);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void endScratchIntervalForNotStartedIntervalCausesNPE() {
+    Register scratch = createRegister(0);
+    Instruction inst = createInstruction();
+    scratchMap.endScratchInterval(scratch, inst);
+  }
+
+
+}