Skip to content

Commit 432309b

Browse files
fangererfangerer
committed
Refactor and move storage generalization code to nodes.
1 parent b09407a commit 432309b

19 files changed

+346
-343
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/TruffleCextBuiltins.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,13 @@ Object run(Object obj,
226226
abstract static class PyTuple_SetItem extends NativeBuiltin {
227227
@Specialization
228228
int doI(PTuple tuple, Object position, Object element,
229-
@Cached("create()") SequenceStorageNodes.SetItemNode setItemNode) {
229+
@Cached("createSetItem()") SequenceStorageNodes.SetItemNode setItemNode) {
230230
setItemNode.execute(tuple.getSequenceStorage(), position, element);
231231
return 0;
232232
}
233233

234234
protected static SequenceStorageNodes.SetItemNode createSetItem() {
235-
return SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forTupleAssign());
235+
return SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forTupleAssign(), "invalid item for assignment");
236236
}
237237
}
238238

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/array/ArrayBuiltins.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ Object doGeneric(Object self, @SuppressWarnings("unused") Object key, @SuppressW
238238
}
239239

240240
protected static SequenceStorageNodes.SetItemNode createSetItem() {
241-
return SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forArrayAssign());
241+
// TODO correct error message depending on array's element type
242+
return SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forArrayAssign(), "invalid item for assignment");
242243
}
243244
}
244245

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/bytes/ByteArrayBuiltins.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,6 @@ abstract static class TranslateNode extends PythonBuiltinNode {
613613

614614
@Child private SequenceStorageNodes.GetItemNode getSelfItemNode;
615615
@Child private SequenceStorageNodes.GetItemNode getTableItemNode;
616-
@Child private SequenceStorageNodes.SetItemNode setItemNode;
617616

618617
@Specialization
619618
PByteArray translate(PByteArray self, PBytes table, @SuppressWarnings("unused") PNone delete) {
@@ -711,7 +710,7 @@ Object doGeneric(Object self, Object idx, Object value) {
711710
private SequenceStorageNodes.SetItemNode getSetItemNode() {
712711
if (setItemNode == null) {
713712
CompilerDirectives.transferToInterpreterAndInvalidate();
714-
setItemNode = insert(SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forBytearray()));
713+
setItemNode = insert(SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forBytearray(), "an integer is required"));
715714
}
716715
return setItemNode;
717716
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/bytes/PByteArray.java

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,6 @@ public PByteArray(PythonClass cls, SequenceStorage store) {
5757
this.store = store;
5858
}
5959

60-
public void setItemNormalized(int index, Object value) {
61-
try {
62-
store.setItemNormalized(index, value);
63-
} catch (SequenceStoreException e) {
64-
store = store.generalizeFor(value, null);
65-
66-
try {
67-
store.setItemNormalized(index, value);
68-
} catch (SequenceStoreException ex) {
69-
throw new IllegalStateException();
70-
}
71-
}
72-
}
73-
7460
@Override
7561
public void setSlice(int start, int stop, int step, PSequence value) {
7662
final int normalizedStart = SequenceUtil.normalizeSliceStart(start, step, store.length());

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/PySequenceArrayWrapperMR.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,14 @@ abstract static class WriteArrayItemNode extends Node {
233233

234234
@Specialization
235235
Object doTuple(PBytes s, long idx, byte value,
236-
@Cached("create()") SequenceStorageNodes.SetItemNode setItemNode) {
236+
@Cached("createStorageSetItem()") SequenceStorageNodes.SetItemNode setItemNode) {
237237
setItemNode.executeLong(s.getSequenceStorage(), idx, value);
238238
return value;
239239
}
240240

241241
@Specialization
242242
Object doTuple(PSequence s, long idx, Object value,
243-
@Cached("create()") SequenceStorageNodes.SetItemNode setItemNode) {
243+
@Cached("createStorageSetItem()") SequenceStorageNodes.SetItemNode setItemNode) {
244244
setItemNode.execute(s.getSequenceStorage(), idx, getToJavaNode().execute(value));
245245
return value;
246246
}
@@ -252,8 +252,8 @@ Object doGeneric(Object tuple, Object idx, Object value,
252252
return value;
253253
}
254254

255-
protected static ListBuiltins.SetItemNode createListSetItem() {
256-
return ListBuiltinsFactory.SetItemNodeFactory.create();
255+
protected static SequenceStorageNodes.SetItemNode createStorageSetItem() {
256+
return SequenceStorageNodes.SetItemNode.create("invalid item for assignment");
257257
}
258258

259259
protected static LookupAndCallTernaryNode createSetItem() {

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

Lines changed: 222 additions & 79 deletions
Large diffs are not rendered by default.

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/ints/PInt.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,21 +143,29 @@ public static double doubleValue(boolean right) {
143143
}
144144

145145
public static int intValueExact(long val) {
146-
if (val != (int) val) {
146+
if (!isIntRange(val)) {
147147
throw new ArithmeticException();
148148
}
149149
return (int) val;
150150
}
151151

152+
public static boolean isByteRange(int val) {
153+
return val >= 0 && val < 256;
154+
}
155+
156+
public static boolean isByteRange(long val) {
157+
return val >= 0 && val < 256;
158+
}
159+
152160
public static byte byteValueExact(int val) {
153-
if (val < 0 || val >= 256) {
161+
if (!isByteRange(val)) {
154162
throw new ArithmeticException();
155163
}
156164
return (byte) val;
157165
}
158166

159167
public static byte byteValueExact(long val) {
160-
if (val < 0 || val >= 256) {
168+
if (!isByteRange(val)) {
161169
throw new ArithmeticException();
162170
}
163171
return (byte) val;
@@ -168,4 +176,8 @@ public byte byteValueExact() {
168176
return value.byteValueExact();
169177
}
170178

179+
public static boolean isIntRange(long val) {
180+
return val == (int) val;
181+
}
182+
171183
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java

Lines changed: 45 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import com.oracle.graal.python.builtins.objects.PNone;
6161
import com.oracle.graal.python.builtins.objects.PNotImplemented;
6262
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
63+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
6364
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.NormalizeIndexNode;
6465
import com.oracle.graal.python.builtins.objects.ints.PInt;
6566
import com.oracle.graal.python.builtins.objects.iterator.PDoubleSequenceIterator;
@@ -97,7 +98,6 @@
9798
import com.oracle.graal.python.runtime.sequence.storage.ObjectSequenceStorage;
9899
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
99100
import com.oracle.graal.python.runtime.sequence.storage.SequenceStoreException;
100-
import com.oracle.graal.python.runtime.sequence.storage.SetSequenceStorageItem;
101101
import com.oracle.graal.python.runtime.sequence.storage.TupleSequenceStorage;
102102
import com.oracle.truffle.api.CompilerDirectives;
103103
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -297,73 +297,49 @@ protected Object doGeneric(Object self, Object objectIdx) {
297297
public abstract static class SetItemNode extends PythonTernaryBuiltinNode {
298298

299299
@Child private NormalizeIndexNode normalize = NormalizeIndexNode.forListAssign();
300+
private final ConditionProfile generalizedProfile = ConditionProfile.createBinaryProfile();
300301

301302
@Specialization
302-
public PNone doPList(PList list, PSlice slice, Object value,
303+
public PNone doPList(PList primary, PSlice slice, Object value,
303304
@Cached("create()") ListNodes.SetSliceNode sliceNode) {
304-
return sliceNode.execute(list, slice, value);
305-
}
306-
307-
@Specialization(guards = "isIntStorage(primary)")
308-
public Object doPListInt(PList primary, int idx, int value) {
309-
IntSequenceStorage store = (IntSequenceStorage) primary.getSequenceStorage();
310-
store.setIntItemNormalized(normalize.execute(idx, store.length()), value);
311-
return PNone.NONE;
312-
}
313-
314-
@Specialization(guards = "isDoubleStorage(primary)")
315-
public Object doPListDouble(PList primary, int idx, double value) {
316-
DoubleSequenceStorage store = (DoubleSequenceStorage) primary.getSequenceStorage();
317-
store.setDoubleItemNormalized(normalize.execute(idx, store.length()), value);
318-
return PNone.NONE;
319-
}
320-
321-
@Specialization(guards = "isObjectStorage(primary)")
322-
public Object doPListObject(PList primary, int idx, Object value) {
323-
ObjectSequenceStorage store = (ObjectSequenceStorage) primary.getSequenceStorage();
324-
store.setItemNormalized(normalize.execute(idx, store.length()), value);
325-
return PNone.NONE;
305+
return sliceNode.execute(primary, slice, value);
326306
}
327307

328308
@Specialization
329-
public Object doPList(PList list, int idx, Object value,
330-
@Cached("create()") SetSequenceStorageItem setItem) {
331-
setItem.setItem(list, idx, value);
332-
return PNone.NONE;
333-
}
334-
335-
@Specialization(guards = "isIntStorage(primary)")
336-
public Object doPListInt(PList primary, long idx, int value) {
337-
IntSequenceStorage store = (IntSequenceStorage) primary.getSequenceStorage();
338-
store.setIntItemNormalized(normalize.execute(idx, store.length()), value);
309+
public Object doPListInt(PList primary, int idx, Object value,
310+
@Cached("createSetItem()") SequenceStorageNodes.SetItemNode setItemNode) {
311+
updateStorage(primary, setItemNode.execute(primary.getSequenceStorage(), idx, value));
339312
return PNone.NONE;
340313
}
341314

342-
@Specialization(guards = "isDoubleStorage(primary)")
343-
public Object doPListDouble(PList primary, long idx, double value) {
344-
DoubleSequenceStorage store = (DoubleSequenceStorage) primary.getSequenceStorage();
345-
store.setDoubleItemNormalized(normalize.execute(idx, store.length()), value);
315+
@Specialization
316+
public Object doPListLong(PList primary, long idx, Object value,
317+
@Cached("createSetItem()") SequenceStorageNodes.SetItemNode setItemNode) {
318+
updateStorage(primary, setItemNode.execute(primary.getSequenceStorage(), idx, value));
346319
return PNone.NONE;
347320
}
348321

349-
@Specialization(guards = "isObjectStorage(primary)")
350-
public Object doPListObject(PList primary, long idx, Object value) {
351-
ObjectSequenceStorage store = (ObjectSequenceStorage) primary.getSequenceStorage();
352-
store.setItemNormalized(normalize.execute(idx, store.length()), value);
322+
@Specialization
323+
public Object doPListPInt(PList primary, PInt idx, Object value,
324+
@Cached("createSetItem()") SequenceStorageNodes.SetItemNode setItemNode) {
325+
updateStorage(primary, setItemNode.execute(primary.getSequenceStorage(), idx, value));
353326
return PNone.NONE;
354327
}
355328

356-
@Specialization
357-
public Object doPList(PList list, long idx, Object value,
358-
@Cached("create()") SetSequenceStorageItem setItem) {
359-
setItem.setItem(list, idx, value);
360-
return PNone.NONE;
329+
private void updateStorage(PList primary, SequenceStorage newStorage) {
330+
if (generalizedProfile.profile(primary.getSequenceStorage() != newStorage)) {
331+
primary.setSequenceStorage(newStorage);
332+
}
361333
}
362334

363335
protected static SetItemNode create() {
364336
return ListBuiltinsFactory.SetItemNodeFactory.create();
365337
}
366338

339+
protected static SequenceStorageNodes.SetItemNode createSetItem() {
340+
return SequenceStorageNodes.SetItemNode.create(NormalizeIndexNode.forArrayAssign(), () -> ListGeneralizationNode.create());
341+
}
342+
367343
@Specialization
368344
protected Object doObjectIndex(PList self, Object objectIdx, Object value,
369345
@Cached("create()") IndexNode getIndexNode,
@@ -458,14 +434,15 @@ public PNone extendSequenceStore(PList list, Object source) throws SequenceStore
458434
}
459435

460436
@Specialization(guards = {"isPSequenceWithStorage(source)"})
461-
public PNone extendSequence(PList list, Object source) {
437+
public PNone extendSequence(PList list, Object source,
438+
@Cached("create()") SequenceStorageNodes.ListGeneralizationNode genNode) {
462439
SequenceStorage eSource = ((PSequence) source).getSequenceStorage();
463440
if (eSource.length() > 0) {
464441
SequenceStorage target = list.getSequenceStorage();
465442
try {
466443
target.extend(eSource);
467444
} catch (SequenceStoreException e) {
468-
target = target.generalizeFor(eSource.getItemNormalized(0), eSource);
445+
target = genNode.execute(target, e.getIndicationValue());
469446
list.setSequenceStorage(target);
470447
try {
471448
target.extend(eSource);
@@ -1057,9 +1034,26 @@ PList doPListObject(PList left, PList right) {
10571034
}
10581035

10591036
@Specialization
1060-
PList doPList(PList left, PList right) {
1037+
PList doPList(PList left, PList other,
1038+
@Cached("create()") SequenceStorageNodes.ListGeneralizationNode genNode) {
10611039
try {
1062-
return left.__add__(right);
1040+
SequenceStorage store = left.getSequenceStorage();
1041+
SequenceStorage otherStore = other.getSequenceStorage();
1042+
SequenceStorage newStore = store.copy();
1043+
1044+
try {
1045+
newStore.extend(otherStore);
1046+
} catch (SequenceStoreException e) {
1047+
newStore = genNode.execute(newStore, e.getIndicationValue());
1048+
1049+
try {
1050+
newStore.extend(otherStore);
1051+
} catch (SequenceStoreException e1) {
1052+
throw new IllegalStateException();
1053+
}
1054+
}
1055+
1056+
return factory().createList(left.getPythonClass(), newStore);
10631057
} catch (ArithmeticException e) {
10641058
throw raise(MemoryError);
10651059
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/PList.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -156,25 +156,6 @@ public final void extend(PList appendee) {
156156
}
157157
}
158158

159-
public final PList __add__(PList other) throws ArithmeticException {
160-
SequenceStorage otherStore = other.getSequenceStorage();
161-
SequenceStorage newStore = store.copy();
162-
163-
try {
164-
newStore.extend(otherStore);
165-
} catch (SequenceStoreException e) {
166-
newStore = newStore.generalizeFor(otherStore.getIndicativeValue(), otherStore);
167-
168-
try {
169-
newStore.extend(otherStore);
170-
} catch (SequenceStoreException e1) {
171-
throw new IllegalStateException();
172-
}
173-
}
174-
175-
return new PList(getPythonClass(), newStore);
176-
}
177-
178159
public final void insert(int index, Object value) {
179160
try {
180161
store.insertItem(index, value);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/BoolSequenceStorage.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public void setItemNormalized(int idx, Object value) throws SequenceStoreExcepti
103103
if (value instanceof Boolean) {
104104
setBoolItemNormalized(idx, (boolean) value);
105105
} else {
106-
throw SequenceStoreException.INSTANCE;
106+
throw new SequenceStoreException(value);
107107
}
108108
}
109109

@@ -116,7 +116,7 @@ public void insertItem(int idx, Object value) throws SequenceStoreException {
116116
if (value instanceof Boolean) {
117117
insertBoolItem(idx, (boolean) value);
118118
} else {
119-
throw SequenceStoreException.INSTANCE;
119+
throw new SequenceStoreException(value);
120120
}
121121
}
122122

@@ -158,7 +158,7 @@ public void setSliceInBound(int start, int stop, int step, SequenceStorage seque
158158
if (sequence instanceof BoolSequenceStorage) {
159159
setBoolSliceInBound(start, stop, step, (BoolSequenceStorage) sequence);
160160
} else {
161-
throw new SequenceStoreException();
161+
throw new SequenceStoreException(sequence);
162162
}
163163
}
164164

@@ -285,7 +285,7 @@ public void append(Object value) throws SequenceStoreException {
285285
if (value instanceof Boolean) {
286286
appendBool((boolean) value);
287287
} else {
288-
throw SequenceStoreException.INSTANCE;
288+
throw new SequenceStoreException(value);
289289
}
290290
}
291291

@@ -300,7 +300,7 @@ public void extend(SequenceStorage other) throws SequenceStoreException, Arithme
300300
if (other instanceof BoolSequenceStorage) {
301301
extendWithBoolStorage((BoolSequenceStorage) other);
302302
} else {
303-
throw SequenceStoreException.INSTANCE;
303+
throw new SequenceStoreException(other);
304304
}
305305
}
306306

0 commit comments

Comments
 (0)