Skip to content

Commit d41cf88

Browse files
timfeltimfel
committed
read refs cells with a node to have the assumption check in one place
1 parent fcd7379 commit d41cf88

File tree

5 files changed

+56
-35
lines changed

5 files changed

+56
-35
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cell/CellBuiltins.java

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.oracle.graal.python.builtins.PythonBuiltins;
5555
import com.oracle.graal.python.builtins.objects.PNone;
5656
import com.oracle.graal.python.builtins.objects.PNotImplemented;
57+
import com.oracle.graal.python.builtins.objects.cell.CellBuiltinsFactory.GetRefNodeGen;
5758
import com.oracle.graal.python.builtins.objects.type.PythonClass;
5859
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5960
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
@@ -64,6 +65,7 @@
6465
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
6566
import com.oracle.truffle.api.dsl.NodeFactory;
6667
import com.oracle.truffle.api.dsl.Specialization;
68+
import com.oracle.truffle.api.nodes.Node;
6769

6870
@CoreFunctions(extendClasses = PythonBuiltinClassType.PCell)
6971
public class CellBuiltins extends PythonBuiltins {
@@ -76,8 +78,10 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
7678
@GenerateNodeFactory
7779
public abstract static class EqNode extends PythonBuiltinNode {
7880
@Specialization
79-
public boolean eq(PCell self, PCell other) {
80-
return self.getRef().equals(other.getRef());
81+
public boolean eq(PCell self, PCell other,
82+
@Cached("create()") GetRefNode getRefL,
83+
@Cached("create()") GetRefNode getRefR) {
84+
return getRefL.execute(self).equals(getRefR.execute(other));
8185
}
8286

8387
@SuppressWarnings("unused")
@@ -94,8 +98,10 @@ public Object eq(Object self, Object other) {
9498
@GenerateNodeFactory
9599
public abstract static class NeqNode extends PythonBuiltinNode {
96100
@Specialization
97-
public boolean neq(PCell self, PCell other) {
98-
return !self.getRef().equals(other.getRef());
101+
public boolean neq(PCell self, PCell other,
102+
@Cached("create()") GetRefNode getRefL,
103+
@Cached("create()") GetRefNode getRefR) {
104+
return !getRefL.execute(self).equals(getRefR.execute(other));
99105
}
100106

101107
@SuppressWarnings("unused")
@@ -114,8 +120,9 @@ public abstract static class ReprNode extends PythonBuiltinNode {
114120
@Specialization
115121
@TruffleBoundary
116122
public String repr(PCell self,
123+
@Cached("create()") GetRefNode getRef,
117124
@Cached("create()") GetClassNode getClassNode) {
118-
Object ref = self.getRef();
125+
Object ref = getRef.execute(self);
119126
if (ref == null) {
120127
return String.format("<cell at %s: empty>", this.hashCode());
121128
}
@@ -136,8 +143,9 @@ public Object eq(Object self) {
136143
@GenerateNodeFactory
137144
public abstract static class CellContentsNode extends PythonBuiltinNode {
138145
@Specialization(guards = "isNoValue(none)")
139-
public Object get(PCell self, @SuppressWarnings("unused") PNone none) {
140-
Object ref = self.getRef();
146+
public Object get(PCell self, @SuppressWarnings("unused") PNone none,
147+
@Cached("create()") GetRefNode getRef) {
148+
Object ref = getRef.execute(self);
141149
if (ref == null) {
142150
throw raise(ValueError, "Cell is empty");
143151
}
@@ -150,4 +158,25 @@ public Object set(PCell self, Object ref) {
150158
return PNone.NONE;
151159
}
152160
}
161+
162+
public abstract static class GetRefNode extends Node {
163+
public abstract Object execute(PCell self);
164+
165+
@Specialization(guards = "self == cachedSelf", assumptions = "cachedSelf.isEffectivelyFinalAssumption()", limit = "1")
166+
Object cached(@SuppressWarnings("unused") PCell self,
167+
@SuppressWarnings("unused") @Cached("self") PCell cachedSelf,
168+
@Cached("self.getRef()") Object ref) {
169+
return ref;
170+
}
171+
172+
@Specialization(replaces = "cached")
173+
Object uncached(PCell self) {
174+
return self.getRef();
175+
}
176+
177+
public static GetRefNode create() {
178+
return GetRefNodeGen.create();
179+
}
180+
}
181+
153182
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cell/PCell.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,14 @@
4545

4646
import com.oracle.graal.python.PythonLanguage;
4747
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
48-
import com.oracle.graal.python.builtins.objects.PNone;
4948
import com.oracle.graal.python.builtins.objects.object.PythonBuiltinObject;
5049
import com.oracle.truffle.api.Assumption;
5150
import com.oracle.truffle.api.CompilerAsserts;
52-
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
5351
import com.oracle.truffle.api.Truffle;
5452

5553
public class PCell extends PythonBuiltinObject {
5654
private final Assumption effectivelyFinal = Truffle.getRuntime().createAssumption("cell is effectively final");
57-
@CompilationFinal private Object ref;
55+
private Object ref;
5856

5957
public PCell() {
6058
super(PythonLanguage.getCore().lookupType(PythonBuiltinClassType.PCell));
@@ -81,13 +79,6 @@ public Assumption isEffectivelyFinalAssumption() {
8179
return effectivelyFinal;
8280
}
8381

84-
public Object getPythonRef() {
85-
if (ref == null) {
86-
return PNone.NONE;
87-
}
88-
return ref;
89-
}
90-
9182
@Override
9283
public List<String> getAttributeNames() {
9384
ArrayList<String> arrayList = new ArrayList<>();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/LocalsStorage.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ private Object getValue(FrameSlot slot) {
6969
if (skipCells) {
7070
return null;
7171
}
72-
return ((PCell) value).getPythonRef();
72+
return ((PCell) value).getRef();
7373
}
7474
return value;
7575
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/superobject/SuperBuiltins.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.oracle.graal.python.builtins.PythonBuiltins;
4949
import com.oracle.graal.python.builtins.modules.BuiltinFunctions.IsInstanceNode;
5050
import com.oracle.graal.python.builtins.objects.PNone;
51+
import com.oracle.graal.python.builtins.objects.cell.CellBuiltins;
5152
import com.oracle.graal.python.builtins.objects.cell.PCell;
5253
import com.oracle.graal.python.builtins.objects.function.PArguments;
5354
import com.oracle.graal.python.builtins.objects.function.PKeyword;
@@ -151,7 +152,8 @@ public abstract static class SuperInitNode extends PythonVarargsBuiltinNode {
151152
@Child private IsSubtypeNode isSubtypeNode;
152153
@Child private IsInstanceNode isInstanceNode;
153154
@Child private GetClassNode getClassNode;
154-
@Child LookupAndCallBinaryNode getAttrNode;
155+
@Child private LookupAndCallBinaryNode getAttrNode;
156+
@Child private CellBuiltins.GetRefNode getRefNode;
155157

156158
@Override
157159
public Object varArgExecute(VirtualFrame frame, Object[] arguments, PKeyword[] keywords) throws VarargsBuiltinDirectInvocationNotSupported {
@@ -224,7 +226,7 @@ PNone initInPlace(VirtualFrame frame, SuperObject self, @SuppressWarnings("unuse
224226
}
225227
Object cls = readClass.execute(frame);
226228
if (isCellProfile.profile(cls instanceof PCell)) {
227-
cls = ((PCell) cls).getPythonRef();
229+
cls = getRefNode().execute((PCell) cls);
228230
}
229231
if (cls == PNone.NONE) {
230232
throw raise(PythonErrorType.RuntimeError, "super(): empty __class__ cell");
@@ -265,18 +267,26 @@ private PNone initFromFrame(SuperObject self, Frame target, Object obj) {
265267
try {
266268
cls = target.getObject(classSlot);
267269
if (cls instanceof PCell) {
268-
cls = ((PCell) cls).getPythonRef();
270+
cls = getRefNode().execute((PCell) cls);
269271
}
270272
} catch (FrameSlotTypeException e) {
271273
// fallthrough
272274
}
273275
}
274-
if (cls == PNone.NONE) {
276+
if (cls == null) {
275277
throw raise(PythonErrorType.RuntimeError, "super(): empty __class__ cell");
276278
}
277279
return init(self, cls, obj);
278280
}
279281

282+
private CellBuiltins.GetRefNode getRefNode() {
283+
if (getRefNode == null) {
284+
CompilerDirectives.transferToInterpreterAndInvalidate();
285+
getRefNode = CellBuiltins.GetRefNode.create();
286+
}
287+
return getRefNode;
288+
}
289+
280290
@SuppressWarnings("unused")
281291
@Fallback
282292
PNone initFallback(Object self, Object cls, Object obj) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/cell/ReadLocalCellNode.java

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import static com.oracle.graal.python.runtime.exception.PythonErrorType.NameError;
2929
import static com.oracle.graal.python.runtime.exception.PythonErrorType.UnboundLocalError;
3030

31+
import com.oracle.graal.python.builtins.objects.cell.CellBuiltins;
3132
import com.oracle.graal.python.builtins.objects.cell.PCell;
3233
import com.oracle.graal.python.nodes.PNodeWithContext;
3334
import com.oracle.graal.python.nodes.cell.ReadLocalCellNodeGen.ReadFromCellNodeGen;
@@ -78,21 +79,11 @@ public ReadFromCellNode(boolean isFreeVar, Object identifier) {
7879

7980
abstract Object execute(Object cell);
8081

81-
@Specialization(guards = "cachedCell == cell", limit = "1", assumptions = "cachedCell.isEffectivelyFinalAssumption()")
82-
Object readFinal(@SuppressWarnings("unused") PCell cell,
83-
@SuppressWarnings("unused") @Cached("cell") PCell cachedCell,
84-
@Cached("cell.getRef()") Object ref) {
85-
if (ref != null) {
86-
return ref;
87-
} else {
88-
throw raiseUnbound();
89-
}
90-
}
91-
92-
@Specialization(replaces = "readFinal")
82+
@Specialization
9383
Object read(PCell cell,
84+
@Cached("create()") CellBuiltins.GetRefNode getRef,
9485
@Cached("createClassProfile()") ValueProfile refTypeProfile) {
95-
Object ref = refTypeProfile.profile(cell.getRef());
86+
Object ref = refTypeProfile.profile(getRef.execute(cell));
9687
if (ref != null) {
9788
return ref;
9889
} else {

0 commit comments

Comments
 (0)