Bytecode DSL: tune starred arguments: remove unnecessary @ExplodeLoop and fix virtual call in PE

Author: steve-s

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

@@ -43,12 +43,15 @@
 import static com.oracle.graal.python.nodes.ErrorMessages.IS_NOT_A_SEQUENCE;
 import static com.oracle.graal.python.runtime.exception.PythonErrorType.TypeError;
 
+import com.oracle.graal.python.builtins.objects.bytes.PBytesLike;
 import com.oracle.graal.python.builtins.objects.common.SequenceNodesFactory.CachedGetObjectArrayNodeGen;
 import com.oracle.graal.python.builtins.objects.common.SequenceNodesFactory.GetObjectArrayNodeGen;
 import com.oracle.graal.python.builtins.objects.common.SequenceNodesFactory.SetSequenceStorageNodeGen;
 import com.oracle.graal.python.builtins.objects.ints.PInt;
+import com.oracle.graal.python.builtins.objects.list.PList;
 import com.oracle.graal.python.builtins.objects.str.PString;
 import com.oracle.graal.python.builtins.objects.str.StringNodes;
+import com.oracle.graal.python.builtins.objects.tuple.PTuple;
 import com.oracle.graal.python.lib.PySequenceCheckNode;
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.PNodeWithContext;
@@ -122,6 +125,32 @@ static boolean doWithStorage(Node inliningTarget, PSequence seq, long idx,
         }
     }
 
+    @GenerateUncached
+    @GenerateInline(inlineByDefault = true)
+    public abstract static class GetPSequenceStorageNode extends PNodeWithContext {
+        public abstract SequenceStorage execute(Node inliningTarget, PSequence seq);
+
+        @Specialization
+        static SequenceStorage doTuple(PTuple seq) {
+            return seq.getSequenceStorage();
+        }
+
+        @Specialization
+        static SequenceStorage doBytesLike(PBytesLike seq) {
+            return seq.getSequenceStorage();
+        }
+
+        @Specialization
+        static SequenceStorage doList(PList seq) {
+            return seq.getSequenceStorage();
+        }
+
+        @Specialization
+        static SequenceStorage doSequence(@SuppressWarnings("unused") PSequence seq) {
+            throw CompilerDirectives.shouldNotReachHere();
+        }
+    }
+
     @GenerateUncached
     @GenerateInline(inlineByDefault = true)
     public abstract static class GetSequenceStorageNode extends PNodeWithContext {
@@ -136,15 +165,10 @@ public final SequenceStorage executeCached(Object seq) {
             return execute(this, seq);
         }
 
-        @Specialization(guards = {"seq.getClass() == cachedClass"}, limit = "2")
-        static SequenceStorage doSequenceCached(PSequence seq,
-                        @Cached("seq.getClass()") Class<? extends PSequence> cachedClass) {
-            return CompilerDirectives.castExact(seq, cachedClass).getSequenceStorage();
-        }
-
-        @Specialization(replaces = "doSequenceCached")
-        static SequenceStorage doSequence(PSequence seq) {
-            return seq.getSequenceStorage();
+        @Specialization
+        static SequenceStorage doSequence(Node inliningTarget, PSequence seq,
+                        @Cached GetPSequenceStorageNode getPSequenceStorageNode) {
+            return getPSequenceStorageNode.execute(inliningTarget, seq);
         }
 
         // Note: this does not seem currently used but is good to accept foreign lists in more

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java

@@ -2084,8 +2084,14 @@ private void emitUnstar(ExprTy[] args) {
          * @param args the sequence of expressions to unstar
          */
         private void emitUnstar(Runnable initialElementsProducer, ExprTy[] args) {
-            if (len(args) == 0 && initialElementsProducer == null) {
+            if (initialElementsProducer == null && len(args) == 0) {
                 b.emitLoadConstant(PythonUtils.EMPTY_OBJECT_ARRAY);
+            } else if (initialElementsProducer == null && len(args) == 1 && args[0] instanceof ExprTy.Starred) {
+                // Optimization for single starred argument: we can just upack it. For generic
+                // algorithm see the next branch
+                b.beginUnpackStarred();
+                ((ExprTy.Starred) args[0]).value.accept(this);
+                b.endUnpackStarred();
             } else if (anyIsStarred(args)) {
                 /**
                  * We emit one or more arrays and concatenate them using Unstar. Each array

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java

@@ -256,6 +256,7 @@
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
 import com.oracle.truffle.api.profiles.InlinedBranchProfile;
 import com.oracle.truffle.api.profiles.InlinedConditionProfile;
+import com.oracle.truffle.api.profiles.InlinedIntValueProfile;
 import com.oracle.truffle.api.source.Source;
 import com.oracle.truffle.api.source.SourceSection;
 import com.oracle.truffle.api.strings.TruffleString;
@@ -2479,18 +2480,9 @@ public static PCell[] doMakeCellArray(@Variadic Object[] cells) {
     @Operation
     @ConstantOperand(type = int.class, specifyAtEnd = true)
     public static final class Unstar {
-        @Specialization
-        public static Object[] perform(@Variadic Object[] values,
-                        int length) {
-            if (length <= EXPLODE_LOOP_THRESHOLD) {
-                return doExploded(values, length);
-            } else {
-                return doRegular(values, length);
-            }
-        }
-
-        @ExplodeLoop
-        private static Object[] doExploded(Object[] values, int length) {
+        @Specialization(guards = "length != 1")
+        public static Object[] perform(@Variadic Object[] values, int length) {
+            assert length > 1; // we should emit load constant: empty array, or emit just unpack starred
             CompilerAsserts.partialEvaluationConstant(length);
             int totalLength = 0;
             for (int i = 0; i < length; i++) {
@@ -2505,21 +2497,6 @@ private static Object[] doExploded(Object[] values, int length) {
             }
             return result;
         }
-
-        private static Object[] doRegular(Object[] values, int length) {
-            int totalLength = 0;
-            for (int i = 0; i < length; i++) {
-                totalLength += ((Object[]) values[i]).length;
-            }
-            Object[] result = new Object[totalLength];
-            int idx = 0;
-            for (int i = 0; i < length; i++) {
-                int nl = ((Object[]) values[i]).length;
-                System.arraycopy(values[i], 0, result, idx, nl);
-                idx += nl;
-            }
-            return result;
-        }
     }
 
     @Operation
@@ -2561,8 +2538,9 @@ public static boolean isListOrTuple(PSequence obj, Node inliningTarget, InlinedC
         static Object[] fromListOrTuple(PSequence seq,
                         @Bind Node inliningTarget,
                         @SuppressWarnings("unused") @Cached InlinedConditionProfile isListProfile,
+                        @Exclusive @Cached SequenceNodes.GetPSequenceStorageNode getStorage,
                         @Exclusive @Cached SequenceStorageNodes.ToArrayNode toArrayNode) {
-            return toArrayNode.execute(inliningTarget, seq.getSequenceStorage());
+            return toArrayNode.execute(inliningTarget, getStorage.execute(inliningTarget, seq));
         }
 
         @Specialization(guards = "isNoValue(none)")