Skip to content

Commit 13c5df9

Browse files
steve-sppisl
steve-s
authored and
ppisl
committed
Fix Foreign nb_add slot to work for the reverse case
1 parent e37abec commit 13c5df9

File tree

3 files changed

+135
-22
lines changed

3 files changed

+135
-22
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/foreign/ForeignObjectBuiltins.java

Lines changed: 126 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@
7979
import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
8080

8181
import java.math.BigInteger;
82-
import java.util.Arrays;
8382
import java.util.List;
8483

8584
import com.oracle.graal.python.PythonLanguage;
@@ -105,6 +104,7 @@
105104
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotLen.LenBuiltinNode;
106105
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSetAttr.SetAttrBuiltinNode;
107106
import com.oracle.graal.python.builtins.objects.type.slots.TpSlotSizeArgFun.SqItemBuiltinNode;
107+
import com.oracle.graal.python.lib.PyNumberAddNode;
108108
import com.oracle.graal.python.lib.PyNumberAsSizeNode;
109109
import com.oracle.graal.python.lib.PyObjectReprAsTruffleStringNode;
110110
import com.oracle.graal.python.lib.PyObjectRichCompareBool;
@@ -279,6 +279,8 @@ static int len(Object self,
279279
}
280280
}
281281

282+
// TODO: remove once all dunder methods are converted to slots and to
283+
// NormalizeForeignForBinopNode
282284
abstract static class ForeignBinaryNode extends BinaryOpBuiltinNode {
283285
@Child private BinaryOpNode op;
284286
protected final boolean reverse;
@@ -408,36 +410,138 @@ public static PNotImplemented doGeneric(Object left, Object right) {
408410
}
409411
}
410412

411-
@Slot(value = SlotKind.nb_add, isComplex = true)
412-
@GenerateNodeFactory
413-
abstract static class AddNode extends ForeignBinaryNode {
414-
AddNode() {
415-
super(BinaryArithmetic.Add.create(), false);
413+
@GenerateInline
414+
@GenerateCached(false)
415+
abstract static class NormalizeForeignForBinopNode extends Node {
416+
public abstract Object execute(Node inliningTarget, Object value, boolean doArray);
417+
418+
@Specialization(guards = {"lib.isBoolean(obj)"})
419+
Object doBool(Object obj, @SuppressWarnings("unused") boolean doArray,
420+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib,
421+
@Shared @Cached(inline = false) GilNode gil) {
422+
gil.release(true);
423+
try {
424+
return lib.asBoolean(obj);
425+
} catch (UnsupportedMessageException e) {
426+
CompilerDirectives.transferToInterpreterAndInvalidate();
427+
throw new IllegalStateException("object does not unpack to boolean as it claims to");
428+
} finally {
429+
gil.acquire();
430+
}
416431
}
417432

418-
AddNode(boolean reverse) {
419-
super(BinaryArithmetic.Add.create(), reverse);
433+
@Specialization(guards = "lib.fitsInLong(obj)")
434+
Object doLong(Object obj, @SuppressWarnings("unused") boolean doArray,
435+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib,
436+
@Shared @Cached(inline = false) GilNode gil) {
437+
assert !lib.isBoolean(obj);
438+
gil.release(true);
439+
try {
440+
return lib.asLong(obj);
441+
} catch (UnsupportedMessageException e) {
442+
CompilerDirectives.transferToInterpreterAndInvalidate();
443+
throw new IllegalStateException("object does not unpack to long as it claims to");
444+
} finally {
445+
gil.acquire();
446+
}
420447
}
421448

422-
@Specialization(insertBefore = "doGeneric", guards = {"lib.hasArrayElements(left)", "lib.hasArrayElements(right)"})
423-
static Object doForeignArray(Object left, Object right,
424-
@Cached PythonObjectFactory factory,
425-
@CachedLibrary(limit = "3") InteropLibrary lib,
426-
@Cached PForeignToPTypeNode convert,
427-
@Cached GilNode gil) {
449+
@Specialization(guards = {"!lib.fitsInLong(obj)", "lib.fitsInBigInteger(obj)"})
450+
Object doBigInt(Object obj, @SuppressWarnings("unused") boolean doArray,
451+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib,
452+
@Shared @Cached(inline = false) GilNode gil,
453+
@Shared @Cached(inline = false) PythonObjectFactory factory) {
454+
assert !lib.isBoolean(obj);
428455
gil.release(true);
429456
try {
430-
Object[] unpackedLeft = unpackForeignArray(left, lib, convert);
431-
Object[] unpackedRight = unpackForeignArray(right, lib, convert);
432-
if (unpackedLeft != null && unpackedRight != null) {
433-
Object[] result = Arrays.copyOf(unpackedLeft, unpackedLeft.length + unpackedRight.length);
434-
System.arraycopy(unpackedRight, 0, result, unpackedLeft.length, unpackedRight.length);
435-
return factory.createList(result);
436-
}
457+
return factory.createInt(lib.asBigInteger(obj));
458+
} catch (UnsupportedMessageException e) {
459+
CompilerDirectives.transferToInterpreterAndInvalidate();
460+
throw new IllegalStateException("object does not unpack to BigInteger as it claims to");
437461
} finally {
438462
gil.acquire();
439463
}
440-
return PNotImplemented.NOT_IMPLEMENTED;
464+
}
465+
466+
@Specialization(guards = {"!lib.fitsInLong(obj)", "!lib.fitsInBigInteger(obj)", "lib.fitsInDouble(obj)"})
467+
Object doDouble(Object obj, @SuppressWarnings("unused") boolean doArray,
468+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib,
469+
@Shared @Cached(inline = false) GilNode gil) {
470+
assert !lib.isBoolean(obj);
471+
gil.release(true);
472+
try {
473+
return lib.asDouble(obj);
474+
} catch (UnsupportedMessageException e) {
475+
CompilerDirectives.transferToInterpreterAndInvalidate();
476+
throw new IllegalStateException("object does not unpack to double as it claims to");
477+
} finally {
478+
gil.acquire();
479+
}
480+
}
481+
482+
@Specialization(guards = {"!lib.fitsInLong(obj)", "!lib.fitsInBigInteger(obj)", "!lib.fitsInDouble(obj)", "lib.isString(obj)"})
483+
Object doString(Object obj, @SuppressWarnings("unused") boolean doArray,
484+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib,
485+
@Cached(inline = false) TruffleString.SwitchEncodingNode switchEncodingNode,
486+
@Shared @Cached(inline = false) GilNode gil) {
487+
assert !lib.isBoolean(obj);
488+
gil.release(true);
489+
try {
490+
return switchEncodingNode.execute(lib.asTruffleString(obj), TS_ENCODING);
491+
} catch (UnsupportedMessageException e) {
492+
throw CompilerDirectives.shouldNotReachHere(e);
493+
} finally {
494+
gil.acquire();
495+
}
496+
}
497+
498+
@Specialization(guards = {"doArray", "!lib.isBoolean(obj)", "!lib.fitsInLong(obj)", "!lib.fitsInBigInteger(obj)", //
499+
"!lib.fitsInDouble(obj)", "!lib.isString(obj)", "lib.hasArrayElements(obj)"})
500+
Object doArray(Object obj, @SuppressWarnings("unused") boolean doArray,
501+
@Shared @CachedLibrary(limit = "3") InteropLibrary lib,
502+
@Shared @Cached(inline = false) GilNode gil,
503+
@Cached(inline = false) PForeignToPTypeNode convert,
504+
@Shared @Cached(inline = false) PythonObjectFactory factory) {
505+
gil.release(true);
506+
try {
507+
return factory.createTuple(unpackForeignArray(obj, lib, convert));
508+
} finally {
509+
gil.acquire();
510+
}
511+
}
512+
513+
@Fallback
514+
@SuppressWarnings("unused")
515+
public static Object doGeneric(Object left, boolean doArray) {
516+
return null;
517+
}
518+
}
519+
520+
@Slot(value = SlotKind.nb_add, isComplex = true)
521+
@GenerateNodeFactory
522+
abstract static class AddNode extends BinaryOpBuiltinNode {
523+
@Specialization
524+
static Object doIt(VirtualFrame frame, Object left, Object right,
525+
@Bind("this") Node inliningTarget,
526+
@Cached IsForeignObjectNode isForeignLeft,
527+
@Cached IsForeignObjectNode isForeignRight,
528+
@Cached NormalizeForeignForBinopNode normalizeLeft,
529+
@Cached NormalizeForeignForBinopNode normalizeRight,
530+
@Cached PyNumberAddNode addNode) {
531+
boolean leftIsForeign = isForeignLeft.execute(inliningTarget, left);
532+
boolean rightIsForeign = isForeignRight.execute(inliningTarget, right);
533+
if (!leftIsForeign && !rightIsForeign) {
534+
return PNotImplemented.NOT_IMPLEMENTED;
535+
}
536+
537+
Object newLeft = normalizeLeft.execute(inliningTarget, left, true);
538+
Object newRight = normalizeRight.execute(inliningTarget, right, true);
539+
assert newLeft == null || !IsForeignObjectNode.executeUncached(newLeft) : newLeft;
540+
assert newRight == null || !IsForeignObjectNode.executeUncached(newRight) : newRight;
541+
if (newLeft == null || newRight == null) {
542+
return PNotImplemented.NOT_IMPLEMENTED;
543+
}
544+
return addNode.execute(frame, inliningTarget, newLeft, newRight);
441545
}
442546
}
443547

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlotBinaryOp.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ public Object execute(VirtualFrame frame, Object self, Object other) {
178178
}
179179
}
180180

181+
/**
182+
* Slots representing "reversible" binary operations on the Python side do not have two
183+
* versions, instead they should be able to handle the situation when "left" operand is not the
184+
* object that "owns" the slot. See also {@link com.oracle.graal.python.lib.CallBinaryOp1Node}.
185+
*/
181186
@GenerateInline(value = false, inherit = true)
182187
public abstract static class BinaryOpBuiltinNode extends PythonBinaryBuiltinNode {
183188
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/CallBinaryOp1Node.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ static Object doIt(VirtualFrame frame, Node inliningTarget, Object v, Object cla
105105
slotW = null;
106106
}
107107
}
108+
// Note: we call slotW with v as the receiver. This appears to be the semantics of
109+
// CPython reversible binop slots. This is supposed to allow the slot to handle
110+
// the reversible case, if the slot does not want to handle it, it should detect that
111+
// the first receiver argument is not of the right type and just return NotImplemented.
108112
if (slotV != null) {
109113
if (slotW != null && isSubtypeNode.execute(frame, classW, classV)) {
110114
assert !sameTypes;

0 commit comments

Comments
 (0)