Revert "Remove SImmutableObject and replace by caching AST node"

This reverts commit 3c95a8c.

I don't yet have a good enough strategy to have better results than what Graal does based on the @CompilationFinal annotation.

Signed-off-by: Stefan Marr <[email protected]>

src/som/compiler/MixinDefinition.java

@@ -17,7 +17,8 @@
 import som.interpreter.nodes.ExpressionNode;
 import som.interpreter.nodes.SlotAccessNode.ClassSlotAccessNode;
 import som.interpreter.nodes.dispatch.AbstractDispatchNode;
-import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedSlotRead;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedImmutableSlotRead;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedMutableSlotRead;
 import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedSlotWrite;
 import som.interpreter.nodes.dispatch.DispatchGuard;
 import som.interpreter.nodes.dispatch.Dispatchable;
@@ -35,6 +36,8 @@
 import som.vmobjects.SInvokable;
 import som.vmobjects.SInvokable.SInitializer;
 import som.vmobjects.SObject;
+import som.vmobjects.SObject.SImmutableObject;
+import som.vmobjects.SObject.SMutableObject;
 import som.vmobjects.SObjectWithClass;
 import som.vmobjects.SSymbol;
 
@@ -494,7 +497,12 @@ public String toString() {
     public AbstractDispatchNode getDispatchNode(final Object receiver,
         final Object firstArg, final AbstractDispatchNode next) {
       SObject rcvr = (SObject) receiver;
-      return new CachedSlotRead(createNode(rcvr), DispatchGuard.create(rcvr), next);
+      if (rcvr instanceof SMutableObject) {
+        return new CachedMutableSlotRead(createNode(rcvr), DispatchGuard.create(rcvr), next);
+      } else {
+        assert rcvr instanceof SImmutableObject;
+        return new CachedImmutableSlotRead(createNode(rcvr), DispatchGuard.create(rcvr), next);
+      }
     }
 
     @Override

src/som/interpreter/nodes/IsValueCheckNode.java

@@ -5,7 +5,7 @@
 import som.interpreter.nodes.nary.UnaryExpressionNode;
 import som.primitives.ObjectPrims.IsValue;
 import som.vm.constants.KernelObj;
-import som.vmobjects.SObject;
+import som.vmobjects.SObject.SImmutableObject;
 
 import com.oracle.truffle.api.frame.VirtualFrame;
 
@@ -44,14 +44,14 @@ public Object executeEvaluated(final VirtualFrame frame, final Object receiver)
     }
 
     private Object specialize(final VirtualFrame frame, final Object receiver) {
-      if (!(receiver instanceof SObject)) {
+      if (!(receiver instanceof SImmutableObject)) {
         // can remove ourselves, this node is only used in initializers,
         // which are by definition monomorphic
         replace(self);
         return receiver;
       }
 
-      SObject rcvr = (SObject) receiver;
+      SImmutableObject rcvr = (SImmutableObject) receiver;
 
       if (rcvr.isValue()) {
         return replace(new ValueCheckNode(self)).
@@ -71,7 +71,7 @@ public ValueCheckNode(final ExpressionNode self) {
 
     @Override
     public Object executeEvaluated(final VirtualFrame frame, final Object receiver) {
-      SObject rcvr = (SObject) receiver;
+      SImmutableObject rcvr = (SImmutableObject) receiver;
 
       boolean allFieldsContainValues = allFieldsContainValues(rcvr);
       if (allFieldsContainValues) {
@@ -80,7 +80,7 @@ public Object executeEvaluated(final VirtualFrame frame, final Object receiver)
       return KernelObj.signalException("signalNotAValueWith:", receiver);
     }
 
-    private boolean allFieldsContainValues(final SObject rcvr) {
+    private boolean allFieldsContainValues(final SImmutableObject rcvr) {
       VM.thisMethodNeedsToBeOptimized("Should be optimized or on slowpath");
 
       if (rcvr.field1 == null) {

src/som/interpreter/nodes/SlotAccessNode.java

@@ -24,7 +24,7 @@ public static final class ClassSlotAccessNode extends AbstractFieldRead {
     @Child protected DirectCallNode superclassAndMixinResolver;
     @Child protected ClassInstantiationNode instantiation;
 
-    @Child protected AbstractFieldRead      read;
+    private final AbstractFieldRead  read;
     @Child protected AbstractFieldWriteNode write;
 
     public ClassSlotAccessNode(final MixinDefinition mixinDef,

src/som/interpreter/nodes/dispatch/CachedSlotAccessNode.java

@@ -2,7 +2,8 @@
 
 import som.interpreter.objectstorage.FieldAccess.AbstractFieldRead;
 import som.interpreter.objectstorage.FieldWriteNode.AbstractFieldWriteNode;
-import som.vmobjects.SObject;
+import som.vmobjects.SObject.SImmutableObject;
+import som.vmobjects.SObject.SMutableObject;
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.frame.VirtualFrame;
@@ -17,7 +18,7 @@ public CachedSlotAccessNode(final AbstractFieldRead read) {
     this.read = read;
   }
 
-  public static final class CachedSlotRead extends CachedSlotAccessNode {
+  public abstract static class CachedSlotRead extends CachedSlotAccessNode {
     @Child protected AbstractDispatchNode nextInCache;
 
     private final DispatchGuard           guard;
@@ -34,8 +35,9 @@ public CachedSlotRead(final AbstractFieldRead read,
     public Object executeDispatch(final VirtualFrame frame,
         final Object[] arguments) {
       try {
+        // TODO: make sure this cast is always eliminated, otherwise, we need two versions mut/immut
         if (guard.entryMatches(arguments[0])) {
-          return read.read(frame, (SObject) arguments[0]);
+          return read(frame, arguments[0]);
         } else {
           return nextInCache.executeDispatch(frame, arguments);
         }
@@ -46,12 +48,40 @@ public Object executeDispatch(final VirtualFrame frame,
       }
     }
 
+    protected abstract Object read(final VirtualFrame frame, final Object rcvr) throws InvalidAssumptionException;
+
     @Override
     public int lengthOfDispatchChain() {
       return 1 + nextInCache.lengthOfDispatchChain();
     }
   }
 
+  public static final class CachedImmutableSlotRead extends CachedSlotRead {
+
+    public CachedImmutableSlotRead(final AbstractFieldRead read,
+        final DispatchGuard guard, final AbstractDispatchNode nextInCache) {
+      super(read, guard, nextInCache);
+    }
+
+    @Override
+    protected Object read(final VirtualFrame frame, final Object rcvr) throws InvalidAssumptionException {
+      return read.read(frame, (SImmutableObject) rcvr);
+    }
+  }
+
+  public static final class CachedMutableSlotRead extends CachedSlotRead {
+
+    public CachedMutableSlotRead(final AbstractFieldRead read,
+        final DispatchGuard guard, final AbstractDispatchNode nextInCache) {
+      super(read, guard, nextInCache);
+    }
+
+    @Override
+    protected Object read(final VirtualFrame frame, final Object rcvr) throws InvalidAssumptionException {
+      return read.read(frame, (SMutableObject) rcvr);
+    }
+  }
+
   public static final class CachedSlotWrite extends AbstractDispatchNode {
     @Child protected AbstractDispatchNode   nextInCache;
     @Child protected AbstractFieldWriteNode write;
@@ -71,7 +101,7 @@ public Object executeDispatch(final VirtualFrame frame,
         final Object[] arguments) {
       try {
         if (guard.entryMatches(arguments[0])) {
-          return write.write((SObject) arguments[0], arguments[1]);
+          return write.write((SMutableObject) arguments[0], arguments[1]);
         } else {
           return nextInCache.executeDispatch(frame, arguments);
         }

src/som/interpreter/nodes/dispatch/DispatchGuard.java

@@ -3,7 +3,8 @@
 import som.interpreter.objectstorage.ClassFactory;
 import som.interpreter.objectstorage.ObjectLayout;
 import som.vmobjects.SClass;
-import som.vmobjects.SObject;
+import som.vmobjects.SObject.SImmutableObject;
+import som.vmobjects.SObject.SMutableObject;
 import som.vmobjects.SObjectWithClass.SObjectWithoutFields;
 
 import com.oracle.truffle.api.nodes.InvalidAssumptionException;
@@ -31,8 +32,12 @@ public static DispatchGuard create(final Object obj) {
       return new CheckSClass(((SClass) obj).getFactory());
     }
 
-    if (obj instanceof SObject) {
-      return new CheckSObject(((SObject) obj).getObjectLayout());
+    if (obj instanceof SMutableObject) {
+      return new CheckSMutableObject(((SMutableObject) obj).getObjectLayout());
+    }
+
+    if (obj instanceof SImmutableObject) {
+      return new CheckSImmutableObject(((SImmutableObject) obj).getObjectLayout());
     }
 
     return new CheckClass(obj.getClass());
@@ -96,19 +101,35 @@ public boolean entryMatches(final Object obj) throws InvalidAssumptionException
     }
   }
 
-  private static final class CheckSObject extends DispatchGuard {
+  private static final class CheckSMutableObject extends DispatchGuard {
+
+    private final ObjectLayout expected;
+
+    public CheckSMutableObject(final ObjectLayout expected) {
+      this.expected = expected;
+    }
+
+    @Override
+    public boolean entryMatches(final Object obj) throws InvalidAssumptionException {
+      expected.checkIsLatest();
+      return obj instanceof SMutableObject &&
+          ((SMutableObject) obj).getObjectLayout() == expected;
+    }
+  }
+
+  private static final class CheckSImmutableObject extends DispatchGuard {
 
     private final ObjectLayout expected;
 
-    public CheckSObject(final ObjectLayout expected) {
+    public CheckSImmutableObject(final ObjectLayout expected) {
       this.expected = expected;
     }
 
     @Override
     public boolean entryMatches(final Object obj) throws InvalidAssumptionException {
       expected.checkIsLatest();
-      return obj instanceof SObject &&
-          ((SObject) obj).getObjectLayout() == expected;
+      return obj instanceof SImmutableObject &&
+          ((SImmutableObject) obj).getObjectLayout() == expected;
     }
   }
 }

src/som/interpreter/objectstorage/FieldAccess.java

@@ -7,8 +7,6 @@
 import som.vm.constants.Nil;
 import som.vmobjects.SObject;
 
-import com.oracle.truffle.api.dsl.Cached;
-import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.InvalidAssumptionException;
 import com.oracle.truffle.api.nodes.Node;
@@ -55,52 +53,13 @@ public Object read(final VirtualFrame frame, final SObject obj) {
     }
   }
 
-  // Caching levels:
-  //  - receiver identity
-  //  - value identity
-  //  - normal read
-
-  public abstract static class ReadImmutableSlot extends AbstractFieldRead {
-
-    protected final StorageLocation location;
-
-    public ReadImmutableSlot(final SlotDefinition slot, final ObjectLayout layout) {
-      super(slot);
-      assert slot.isImmutable();
-      this.location = layout.getStorageLocation(slot);
-    }
-
-    public abstract Object execute(SObject obj);
-
-    @Override
-    public final Object read(final VirtualFrame frame, final SObject obj) throws InvalidAssumptionException {
-      return execute(obj);
-    }
-
-    @Specialization(guards = "rcvr == cachedRcvr")
-    public final Object sameRcvr(final SObject rcvr, @Cached("rcvr") final SObject cachedRcvr, @Cached("location.read(rcvr)") final Object val) {
-      return val;
-    }
-
-    @Specialization(guards = "location.read(rcvr) == val", contains = "sameRcvr")
-    public final Object sameValue(final SObject rcvr, @Cached("location.read(rcvr)") final Object val) {
-      return val;
-    }
-
-    @Specialization(contains = "sameValue")
-    public final Object normalRead(final SObject rcvr) {
-//      CompilerAsserts.neverPartOfCompilation("not sure, but I hope this is never part of compilation for any of my benchmarks");
-      return location.read(rcvr);
-    }
-  }
   public static final class ReadSetLongFieldNode extends AbstractFieldRead {
     private final LongStorageLocation storage;
     private final IntValueProfile primMarkProfile = IntValueProfile.createIdentityProfile();
 
     public ReadSetLongFieldNode(final SlotDefinition slot,
         final ObjectLayout layout) {
       super(slot);
-      assert !slot.isImmutable();
       this.storage = (LongStorageLocation) layout.getStorageLocation(slot);
     }
 
@@ -122,7 +81,6 @@ public static final class ReadSetOrUnsetLongFieldNode extends AbstractFieldRead
     public ReadSetOrUnsetLongFieldNode(final SlotDefinition slot,
         final ObjectLayout layout) {
       super(slot);
-      assert !slot.isImmutable();
       this.storage = (LongStorageLocation) layout.getStorageLocation(slot);
     }
 
@@ -144,7 +102,6 @@ public static final class ReadSetDoubleFieldNode extends AbstractFieldRead {
     public ReadSetDoubleFieldNode(final SlotDefinition slot,
         final ObjectLayout layout) {
       super(slot);
-      assert !slot.isImmutable();
       this.storage = (DoubleStorageLocation) layout.getStorageLocation(slot);
     }
 
@@ -167,7 +124,6 @@ public static final class ReadSetOrUnsetDoubleFieldNode extends AbstractFieldRea
     public ReadSetOrUnsetDoubleFieldNode(final SlotDefinition slot,
         final ObjectLayout layout) {
       super(slot);
-      assert !slot.isImmutable();
       this.storage = (DoubleStorageLocation) layout.getStorageLocation(slot);
     }
 
@@ -188,7 +144,6 @@ public static final class ReadObjectFieldNode extends AbstractFieldRead {
     public ReadObjectFieldNode(final SlotDefinition slot,
         final ObjectLayout layout) {
       super(slot);
-      assert !slot.isImmutable();
       this.storage = (AbstractObjectStorageLocation) layout.getStorageLocation(slot);
     }