From 6deaf80a8799930888bca4c71a7dbd7a0e24b22f Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 1 Sep 2023 10:07:41 +0200 Subject: [PATCH] Standardization of Vector.sort array access (#7697) Inspired by #6361 - let's use `ArrayLikeXyzNode` instead of `InteropLibrary`. --- .../builtin/ordering/SortVectorNode.java | 101 +++++++++--------- .../runtime/data/vector/ArrayLikeAtNode.java | 5 + .../data/vector/ArrayLikeLengthNode.java | 5 + 3 files changed, 62 insertions(+), 49 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java index 12f5e1c28208..ac8d1401a78a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java @@ -1,17 +1,5 @@ package org.enso.interpreter.node.expression.builtin.ordering; -import com.oracle.truffle.api.CompilerDirectives; -import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; -import com.oracle.truffle.api.dsl.Cached; -import com.oracle.truffle.api.dsl.Cached.Shared; -import com.oracle.truffle.api.dsl.GenerateUncached; -import com.oracle.truffle.api.dsl.Specialization; -import com.oracle.truffle.api.interop.InteropLibrary; -import com.oracle.truffle.api.interop.InvalidArrayIndexException; -import com.oracle.truffle.api.interop.UnsupportedMessageException; -import com.oracle.truffle.api.library.CachedLibrary; -import com.oracle.truffle.api.nodes.Node; - import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -26,7 +14,6 @@ import org.enso.interpreter.dsl.BuiltinMethod; import org.enso.interpreter.node.callable.dispatch.CallOptimiserNode; import org.enso.interpreter.node.callable.resolver.MethodResolverNode; -import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode; import org.enso.interpreter.node.expression.builtin.meta.EqualsNode; import org.enso.interpreter.node.expression.builtin.meta.TypeOfNode; import org.enso.interpreter.node.expression.builtin.text.AnyToTextNode; @@ -36,7 +23,9 @@ import org.enso.interpreter.runtime.callable.function.Function; import org.enso.interpreter.runtime.data.Type; import org.enso.interpreter.runtime.data.text.Text; +import org.enso.interpreter.runtime.data.vector.ArrayLikeAtNode; import org.enso.interpreter.runtime.data.vector.ArrayLikeHelpers; +import org.enso.interpreter.runtime.data.vector.ArrayLikeLengthNode; import org.enso.interpreter.runtime.error.DataflowError; import org.enso.interpreter.runtime.error.PanicException; import org.enso.interpreter.runtime.error.Warning; @@ -45,6 +34,18 @@ import org.enso.interpreter.runtime.library.dispatch.TypesLibrary; import org.enso.interpreter.runtime.state.State; +import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Cached.Shared; +import com.oracle.truffle.api.dsl.GenerateUncached; +import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.InvalidArrayIndexException; +import com.oracle.truffle.api.interop.UnsupportedMessageException; +import com.oracle.truffle.api.library.CachedLibrary; +import com.oracle.truffle.api.nodes.Node; + /** * Sorts a vector with elements that have only Default_Comparator, thus, only elements with a * builtin type, which is the most common scenario for sorting. @@ -97,7 +98,7 @@ public abstract Object execute( @Specialization( guards = { "interop.hasArrayElements(self)", - "areAllDefaultComparators(interop, hostValueToEnsoNode, comparators)", + "areAllDefaultComparators(lengthNode, atNode, comparators)", "interop.isNull(byFunc)", "interop.isNull(onFunc)" }, limit = "3") @@ -112,32 +113,23 @@ Object sortPrimitives( long problemBehavior, @Shared("lessThanNode") @Cached LessThanNode lessThanNode, @Shared("equalsNode") @Cached EqualsNode equalsNode, - @Cached HostValueToEnsoNode hostValueToEnsoNode, + @Shared("lengthNode") @Cached ArrayLikeLengthNode lengthNode, + @Shared("atNode") @Cached ArrayLikeAtNode atNode, @Shared("typeOfNode") @Cached TypeOfNode typeOfNode, @Shared("anyToTextNode") @Cached AnyToTextNode toTextNode, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { EnsoContext ctx = EnsoContext.get(this); Object[] elems; + long longSize = 0L; try { - long size = interop.getArraySize(self); - assert size < Integer.MAX_VALUE; - elems = new Object[(int) size]; + longSize = lengthNode.executeLength(self); + int size = Math.toIntExact(longSize); + elems = new Object[size]; for (int i = 0; i < size; i++) { - if (interop.isArrayElementReadable(self, i)) { - elems[i] = hostValueToEnsoNode.execute(interop.readArrayElement(self, i)); - } else { - CompilerDirectives.transferToInterpreter(); - throw new PanicException( - ctx.getBuiltins() - .error() - .makeUnsupportedArgumentsError( - new Object[] {self}, - "Cannot read array element at index " + i + " of " + self), - this); - } + elems[i] = atNode.executeAt(self, i); } - } catch (UnsupportedMessageException | InvalidArrayIndexException e) { - throw new IllegalStateException("Should not reach here", e); + } catch (ArithmeticException | InvalidArrayIndexException e) { + throw invalidArrayIndexException(e, longSize); } var javaComparator = createDefaultComparator( @@ -189,17 +181,18 @@ Object sortGeneric( @Shared("lessThanNode") @Cached LessThanNode lessThanNode, @Shared("equalsNode") @Cached EqualsNode equalsNode, @Shared("typeOfNode") @Cached TypeOfNode typeOfNode, + @Shared("lengthNode") @Cached ArrayLikeLengthNode lengthNode, + @Shared("atNode") @Cached ArrayLikeAtNode atNode, @Shared("anyToTextNode") @Cached AnyToTextNode toTextNode, @Cached MethodResolverNode methodResolverNode, - @Cached(value = "build()", uncached = "build()") HostValueToEnsoNode hostValueToEnsoNode, @Cached(value = "build()", uncached = "build()") CallOptimiserNode callNode) { var problemBehavior = ProblemBehavior.fromInt((int) problemBehaviorNum); // Split into groups - List elems = readInteropArray(interop, hostValueToEnsoNode, warningsLib, self); + List elems = readInteropArray(lengthNode, atNode, warningsLib, self); List comparators = - readInteropArray(interop, hostValueToEnsoNode, warningsLib, comparatorsArray); + readInteropArray(lengthNode, atNode, warningsLib, comparatorsArray); List compareFuncs = - readInteropArray(interop, hostValueToEnsoNode, warningsLib, compareFuncsArray); + readInteropArray(lengthNode, atNode, warningsLib, compareFuncsArray); List groups = splitByComparators(elems, comparators, compareFuncs); // Prepare input for DefaultSortComparator and GenericSortComparator and sort the elements @@ -388,15 +381,17 @@ private Object incomparableValuesError(Object left, Object right) { */ @SuppressWarnings("unchecked") private List readInteropArray( - InteropLibrary interop, - HostValueToEnsoNode hostValueToEnsoNode, + ArrayLikeLengthNode lengthNode, + ArrayLikeAtNode atNode, WarningsLibrary warningsLib, Object vector) { + var longSize = 0L; try { - int size = (int) interop.getArraySize(vector); + longSize = lengthNode.executeLength(vector); + int size = Math.toIntExact(longSize); List res = new ArrayList<>(size); for (int i = 0; i < size; i++) { - Object elem = hostValueToEnsoNode.execute(interop.readArrayElement(vector, i)); + Object elem = atNode.executeAt(vector, i); if (warningsLib.hasWarnings(elem)) { elem = warningsLib.removeWarnings(elem); } @@ -404,7 +399,7 @@ private List readInteropArray( } return res; } catch (UnsupportedMessageException | InvalidArrayIndexException | ClassCastException e) { - throw new IllegalStateException("Should not be reachable", e); + throw invalidArrayIndexException(e, longSize); } } @@ -454,20 +449,21 @@ private boolean isTrue(Object object) { /** Returns true iff the given array of comparators is all Default_Comparator */ boolean areAllDefaultComparators( - InteropLibrary interop, HostValueToEnsoNode hostValueToEnsoNode, Object comparators) { - assert interop.hasArrayElements(comparators); + ArrayLikeLengthNode lengthNode, ArrayLikeAtNode atNode, Object comparators + ) { var ctx = EnsoContext.get(this); + var longSize = 0L; try { - int compSize = (int) interop.getArraySize(comparators); + longSize = lengthNode.executeLength(comparators); + int compSize = (int) longSize; for (int i = 0; i < compSize; i++) { - assert interop.isArrayElementReadable(comparators, i); - Object comparator = hostValueToEnsoNode.execute(interop.readArrayElement(comparators, i)); + Object comparator = atNode.executeAt(comparators, i); if (!isDefaultComparator(comparator, ctx)) { return false; } } - } catch (UnsupportedMessageException | InvalidArrayIndexException e) { - throw new IllegalStateException("Should not be reachable", e); + } catch (ArithmeticException | InvalidArrayIndexException e) { + throw invalidArrayIndexException(e, longSize); } return true; } @@ -480,6 +476,13 @@ private boolean isNan(Object object) { return object instanceof Double dbl && dbl.isNaN(); } + private PanicException invalidArrayIndexException(Exception e, long size) { + var index = e instanceof InvalidArrayIndexException ex ? ex.getInvalidIndex() : 0L; + var ctx = EnsoContext.get(this); + var payload = ctx.getBuiltins().error().makeInvalidArrayIndex(index, size); + throw new PanicException(payload, this); + } + private enum ProblemBehavior { IGNORE, REPORT_WARNING, @@ -618,7 +621,7 @@ private int handleIncomparableValues(Object x, Object y) { } else if (isPrimitiveValue(y)) { return ascending ? 1 : -1; } else { - throw new IllegalStateException("Should not be reachable"); + throw CompilerDirectives.shouldNotReachHere(); } } else { // Values other than primitives are compared just by their type's FQN. diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeAtNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeAtNode.java index 24977e4db071..c85f32a903fd 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeAtNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeAtNode.java @@ -19,6 +19,11 @@ public static ArrayLikeAtNode create() { return ArrayLikeAtNodeGen.create(); } + @NeverDefault + public static ArrayLikeAtNode getUncached() { + return ArrayLikeAtNodeGen.getUncached(); + } + // // implementation // diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeLengthNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeLengthNode.java index e7b219dfefeb..38a5ce9daaf3 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeLengthNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/vector/ArrayLikeLengthNode.java @@ -17,6 +17,11 @@ public static ArrayLikeLengthNode create() { return ArrayLikeLengthNodeGen.create(); } + @NeverDefault + public static ArrayLikeLengthNode getUncached() { + return ArrayLikeLengthNodeGen.getUncached(); + } + // // implementation //