Merge pull request #972 from kmod/avoid_slices

Avoid creating most slice objects

src/codegen/compvars.cpp

@@ -343,6 +343,30 @@ public:
 
     CompilerVariable* getitem(IREmitter& emitter, const OpInfo& info, ConcreteCompilerVariable* var,
                               CompilerVariable* slice) override {
+        if (slice->getType() == UNBOXED_SLICE) {
+            UnboxedSlice slice_val = extractSlice(slice);
+
+            if (slice_val.step == NULL) {
+                static BoxedString* attr = internStringImmortal("__getitem__");
+                CompilerType* return_type
+                    = var->getType()->getattrType(attr, true)->callType(ArgPassSpec(1), { SLICE }, NULL);
+                assert(return_type->getConcreteType() == return_type);
+
+                llvm::Value* cstart, *cstop;
+                cstart = slice_val.start ? slice_val.start->makeConverted(emitter, UNKNOWN)->getValue()
+                                         : getNullPtr(g.llvm_value_type_ptr);
+                cstop = slice_val.stop ? slice_val.stop->makeConverted(emitter, UNKNOWN)->getValue()
+                                       : getNullPtr(g.llvm_value_type_ptr);
+
+                llvm::Value* r
+                    = emitter.createCall3(info.unw_info, g.funcs.apply_slice, var->getValue(), cstart, cstop);
+                emitter.checkAndPropagateCapiException(info.unw_info, r, getNullPtr(g.llvm_value_type_ptr));
+
+
+                return new ConcreteCompilerVariable(static_cast<ConcreteCompilerType*>(return_type), r, true);
+            }
+        }
+
         ConcreteCompilerVariable* converted_slice = slice->makeConverted(emitter, slice->getBoxType());
 
         ExceptionStyle target_exception_style = info.preferredExceptionStyle();
@@ -935,7 +959,6 @@ template <typename T> struct UnboxedVal {
     UnboxedVal(T val, ConcreteCompilerVariable* boxed) : val(std::move(val)), boxed(boxed) {}
 };
 
-// XXX: make this a over a unique_ptr<UnboxedVal>
 template <typename T, typename D> class UnboxedType : public ValuedCompilerType<std::shared_ptr<UnboxedVal<T>>> {
 public:
     // Subclasses need to implement:
@@ -2022,23 +2045,74 @@ public:
         static BoxedString* attr = internStringImmortal("__getitem__");
         bool no_attribute = false;
 
-        ExceptionStyle exception_style = info.preferredExceptionStyle();
+        if (slice->getType() == UNBOXED_SLICE) {
+            UnboxedSlice slice_val = extractSlice(slice);
+
+            // This corresponds to the case in apply_slice that calls into PySequence_GetSlice.
+            // Other cases will get handled by UNKNOWN.getitem
+            if (!slice_val.step && canStaticallyResolveGetattrs() && cls->tp_as_sequence
+                && cls->tp_as_sequence->sq_slice) {
+                if ((!slice_val.start || slice_val.start->getType() == INT || slice_val.start->getType() == BOXED_INT)
+                    && (!slice_val.stop || slice_val.stop->getType() == INT
+                        || slice_val.stop->getType() == BOXED_INT)) {
+
+                    CompilerType* return_type = getattrType(attr, true)->callType(ArgPassSpec(1), { SLICE }, NULL);
+                    assert(return_type->getConcreteType() == return_type);
+
+                    llvm::Value* start = NULL;
+                    if (!slice_val.start)
+                        start = getConstantInt(0, g.i64);
+                    else {
+                        if (slice_val.start->getType() == BOXED_INT)
+                            slice_val.start
+                                = makeUnboxedInt(emitter, static_cast<ConcreteCompilerVariable*>(slice_val.start));
+                        start = IntType::extractInt(slice_val.start);
+                    }
 
-        CompilerVariable* called_constant = tryCallattrConstant(emitter, info, var, attr, true, ArgPassSpec(1, 0, 0, 0),
-                                                                { slice }, NULL, &no_attribute, exception_style);
+                    llvm::Value* stop = NULL;
+                    if (!slice_val.stop)
+                        stop = getConstantInt(PY_SSIZE_T_MAX, g.i64);
+                    else {
+                        if (slice_val.stop->getType() == BOXED_INT)
+                            slice_val.stop
+                                = makeUnboxedInt(emitter, static_cast<ConcreteCompilerVariable*>(slice_val.stop));
+                        stop = IntType::extractInt(slice_val.stop);
+                    }
 
-        if (no_attribute) {
-            assert(called_constant->getType() == UNDEF);
+                    static llvm::FunctionType* ft = llvm::FunctionType::get(
+                        g.llvm_value_type_ptr, { g.llvm_value_type_ptr, g.i64, g.i64 }, false);
+                    llvm::Value* r = emitter.createCall3(
+                        info.unw_info, embedConstantPtr((void*)PySequence_GetSlice, ft->getPointerTo()),
+                        var->getValue(), start, stop);
+                    emitter.checkAndPropagateCapiException(info.unw_info, r, getNullPtr(g.llvm_value_type_ptr));
 
-            // Kind of hacky, but just call into getitem like normal.  except...
-            auto r = UNKNOWN->getitem(emitter, info, var, slice);
-            r->decvref(emitter);
-            // ... return the undef value, since that matches what the type analyzer thought we would do.
-            return called_constant;
+                    return new ConcreteCompilerVariable(static_cast<ConcreteCompilerType*>(return_type), r, true);
+                }
+            }
         }
 
-        if (called_constant)
-            return called_constant;
+        // Only try calling getitem if it's not a slice.  For the slice case, defer to UNKNOWN->getitem, which will
+        // call into apply_slice
+        if (slice->getType() != UNBOXED_SLICE || extractSlice(slice).step != NULL) {
+            ExceptionStyle exception_style = info.preferredExceptionStyle();
+
+            CompilerVariable* called_constant
+                = tryCallattrConstant(emitter, info, var, attr, true, ArgPassSpec(1, 0, 0, 0), { slice }, NULL,
+                                      &no_attribute, exception_style);
+
+            if (no_attribute) {
+                assert(called_constant->getType() == UNDEF);
+
+                // Kind of hacky, but just call into getitem like normal.  except...
+                auto r = UNKNOWN->getitem(emitter, info, var, slice);
+                r->decvref(emitter);
+                // ... return the undef value, since that matches what the type analyzer thought we would do.
+                return called_constant;
+            }
+
+            if (called_constant)
+                return called_constant;
+        }
 
         return UNKNOWN->getitem(emitter, info, var, slice);
     }
@@ -2696,6 +2770,68 @@ CompilerVariable* makeTuple(const std::vector<CompilerVariable*>& elts) {
     return new TupleType::VAR(type, alloc_var, true);
 }
 
+class UnboxedSliceType : public ValuedCompilerType<UnboxedSlice> {
+public:
+    std::string debugName() override { return "slice"; }
+
+    void drop(IREmitter& emitter, VAR* var) override {}
+    void grab(IREmitter& emitter, VAR* var) override {}
+
+    void assertMatches(UnboxedSlice slice) {}
+
+    int numFrameArgs() override { RELEASE_ASSERT(0, "unboxed slice should never get serialized"); }
+
+    Box* deserializeFromFrame(const FrameVals& vals) override {
+        RELEASE_ASSERT(0, "unboxed slice should never get serialized");
+    }
+
+    void serializeToFrame(VAR* v, std::vector<llvm::Value*>& stackmap_args) override {
+        RELEASE_ASSERT(0, "unboxed slice should never get serialized");
+    }
+
+    ConcreteCompilerType* getConcreteType() override { return SLICE; }
+    ConcreteCompilerType* getBoxType() override { return SLICE; }
+
+    bool canConvertTo(CompilerType* other) override { return other == this || other == SLICE || other == UNKNOWN; }
+
+    ConcreteCompilerVariable* makeConverted(IREmitter& emitter, VAR* var, ConcreteCompilerType* other_type) override {
+        assert(other_type == SLICE || other_type == UNKNOWN);
+
+        auto slice = var->getValue();
+
+        ConcreteCompilerVariable* cstart, *cstop, *cstep;
+        cstart = slice.start ? slice.start->makeConverted(emitter, slice.start->getBoxType()) : getNone();
+        cstop = slice.stop ? slice.stop->makeConverted(emitter, slice.stop->getBoxType()) : getNone();
+        cstep = slice.step ? slice.step->makeConverted(emitter, slice.step->getBoxType()) : getNone();
+
+        std::vector<llvm::Value*> args;
+        args.push_back(cstart->getValue());
+        args.push_back(cstop->getValue());
+        args.push_back(cstep->getValue());
+        llvm::Value* rtn = emitter.getBuilder()->CreateCall(g.funcs.createSlice, args);
+
+        cstart->decvref(emitter);
+        cstop->decvref(emitter);
+        cstep->decvref(emitter);
+        return new ConcreteCompilerVariable(SLICE, rtn, true);
+    }
+} _UNBOXED_SLICE;
+CompilerType* UNBOXED_SLICE = &_UNBOXED_SLICE;
+
+CompilerVariable* makeSlice(CompilerVariable* start, CompilerVariable* stop, CompilerVariable* step) {
+    return new UnboxedSliceType::VAR(&_UNBOXED_SLICE, UnboxedSlice{ start, stop, step }, true);
+}
+
+UnboxedSlice extractSlice(CompilerVariable* slice) {
+    assert(slice->getType() == UNBOXED_SLICE);
+    return static_cast<UnboxedSliceType::VAR*>(slice)->getValue();
+}
+
+ConcreteCompilerVariable* getNone() {
+    llvm::Constant* none = embedRelocatablePtr(None, g.llvm_value_type_ptr, "cNone");
+    return new ConcreteCompilerVariable(typeFromClass(none_cls), none, false);
+}
+
 class UndefType : public ConcreteCompilerType {
 public:
     std::string debugName() override { return "undefType"; }

src/codegen/compvars.h

@@ -411,6 +411,8 @@ public:
 // Emit the test for whether one variable 'is' another one.
 ConcreteCompilerVariable* doIs(IREmitter& emitter, CompilerVariable* lhs, CompilerVariable* rhs, bool negate);
 
+ConcreteCompilerVariable* getNone();
+
 // These functions all return an INT variable, from either an unboxed representation (makeInt) or
 // a boxed representation (makeUnboxedInt)
 CompilerVariable* makeInt(int64_t);
@@ -429,6 +431,13 @@ ConcreteCompilerVariable* makeLong(Box*);
 ConcreteCompilerVariable* makePureImaginary(Box*);
 CompilerVariable* makeStr(BoxedString*);
 CompilerVariable* makeUnicode(Box*);
+
+struct UnboxedSlice {
+    CompilerVariable* start, *stop, *step;
+};
+CompilerVariable* makeSlice(CompilerVariable* start, CompilerVariable* stop, CompilerVariable* step);
+UnboxedSlice extractSlice(CompilerVariable* slice);
+
 #if 0
 CompilerVariable* makeUnicode(IREmitter& emitter, llvm::StringRef);
 #endif

src/codegen/irgen/irgenerator.cpp

@@ -1078,10 +1078,6 @@ private:
         auto ellipsis_cls = Ellipsis->cls;
         return new ConcreteCompilerVariable(typeFromClass(ellipsis_cls), ellipsis, false);
     }
-    ConcreteCompilerVariable* getNone() {
-        llvm::Constant* none = embedRelocatablePtr(None, g.llvm_value_type_ptr, "cNone");
-        return new ConcreteCompilerVariable(typeFromClass(none_cls), none, false);
-    }
 
     llvm::Constant* embedParentModulePtr() {
         BoxedModule* parent_module = irstate->getSourceInfo()->parent_module;
@@ -1264,28 +1260,11 @@ private:
 
     CompilerVariable* evalSlice(AST_Slice* node, const UnwindInfo& unw_info) {
         CompilerVariable* start, *stop, *step;
-        start = node->lower ? evalExpr(node->lower, unw_info) : getNone();
-        stop = node->upper ? evalExpr(node->upper, unw_info) : getNone();
-        step = node->step ? evalExpr(node->step, unw_info) : getNone();
-
-        ConcreteCompilerVariable* cstart, *cstop, *cstep;
-        cstart = start->makeConverted(emitter, start->getBoxType());
-        cstop = stop->makeConverted(emitter, stop->getBoxType());
-        cstep = step->makeConverted(emitter, step->getBoxType());
-        start->decvref(emitter);
-        stop->decvref(emitter);
-        step->decvref(emitter);
+        start = node->lower ? evalExpr(node->lower, unw_info) : NULL;
+        stop = node->upper ? evalExpr(node->upper, unw_info) : NULL;
+        step = node->step ? evalExpr(node->step, unw_info) : NULL;
 
-        std::vector<llvm::Value*> args;
-        args.push_back(cstart->getValue());
-        args.push_back(cstop->getValue());
-        args.push_back(cstep->getValue());
-        llvm::Value* rtn = emitter.getBuilder()->CreateCall(g.funcs.createSlice, args);
-
-        cstart->decvref(emitter);
-        cstop->decvref(emitter);
-        cstep->decvref(emitter);
-        return new ConcreteCompilerVariable(SLICE, rtn, true);
+        return makeSlice(start, stop, step);
     }
 
     CompilerVariable* evalExtSlice(AST_ExtSlice* node, const UnwindInfo& unw_info) {

src/codegen/runtime_hooks.cpp

@@ -225,6 +225,7 @@ void initGlobalFuncs(GlobalState& g) {
     GET(yield);
     GET(getiterHelper);
     GET(hasnext);
+    GET(apply_slice);
 
     GET(unpackIntoArray);
     GET(raiseAttributeError);

src/codegen/runtime_hooks.h

@@ -37,7 +37,7 @@ struct GlobalFuncs {
         *createGenerator, *createSet;
     llvm::Value* getattr, *getattr_capi, *setattr, *delattr, *delitem, *delGlobal, *nonzero, *binop, *compare,
         *augbinop, *unboxedLen, *getitem, *getitem_capi, *getclsattr, *getGlobal, *setitem, *unaryop, *import,
-        *importFrom, *importStar, *repr, *exceptionMatches, *yield, *getiterHelper, *hasnext, *setGlobal;
+        *importFrom, *importStar, *repr, *exceptionMatches, *yield, *getiterHelper, *hasnext, *setGlobal, *apply_slice;
 
     llvm::Value* unpackIntoArray, *raiseAttributeError, *raiseAttributeErrorStr, *raiseAttributeErrorCapi,
         *raiseAttributeErrorStrCapi, *raiseNotIterableError, *raiseIndexErrorStr, *raiseIndexErrorStrCapi,

src/core/types.h

@@ -96,7 +96,7 @@ ConcreteCompilerType* typeFromClass(BoxedClass*);
 extern ConcreteCompilerType* UNBOXED_INT, *BOXED_INT, *LONG, *UNBOXED_FLOAT, *BOXED_FLOAT, *UNKNOWN, *BOOL, *STR, *NONE,
     *LIST, *SLICE, *MODULE, *DICT, *BOOL, *BOXED_BOOL, *BOXED_TUPLE, *SET, *FROZENSET, *CLOSURE, *GENERATOR,
     *BOXED_COMPLEX, *FRAME_INFO;
-extern CompilerType* UNDEF, *INT, *FLOAT;
+extern CompilerType* UNDEF, *INT, *FLOAT, *UNBOXED_SLICE;
 
 class CompilerVariable;
 template <class V> class ValuedCompilerVariable;

src/runtime/inline/link_forcer.cpp

@@ -100,6 +100,7 @@ void force() {
     FORCE(yield);
     FORCE(getiterHelper);
     FORCE(hasnext);
+    FORCE(apply_slice);
 
     FORCE(unpackIntoArray);
     FORCE(raiseAttributeError);

src/runtime/objmodel.cpp

@@ -5090,6 +5090,33 @@ static Box* callItemAttr(Box* target, BoxedString* item_str, Box* item, Box* val
     }
 }
 
+#define ISINDEX(x) ((x) == NULL || PyInt_Check(x) || PyLong_Check(x) || PyIndex_Check(x))
+
+extern "C" PyObject* apply_slice(PyObject* u, PyObject* v, PyObject* w) noexcept /* return u[v:w] */
+{
+    // TODO: add rewriting here
+
+    PyTypeObject* tp = u->cls;
+    PySequenceMethods* sq = tp->tp_as_sequence;
+
+    if (sq && sq->sq_slice && ISINDEX(v) && ISINDEX(w)) {
+        Py_ssize_t ilow = 0, ihigh = PY_SSIZE_T_MAX;
+        if (!_PyEval_SliceIndex(v, &ilow))
+            return NULL;
+        if (!_PyEval_SliceIndex(w, &ihigh))
+            return NULL;
+        return PySequence_GetSlice(u, ilow, ihigh);
+    } else {
+        PyObject* slice = PySlice_New(v, w, NULL);
+        if (slice != NULL) {
+            PyObject* res = PyObject_GetItem(u, slice);
+            Py_DECREF(slice);
+            return res;
+        } else
+            return NULL;
+    }
+}
+
 // This function decides whether to call the slice operator (e.g. __getslice__)
 // or the item operator (__getitem__).
 template <ExceptionStyle S, Rewritable rewritable>

src/runtime/objmodel.h

@@ -88,6 +88,7 @@ extern "C" Box* getitem(Box* value, Box* slice);
 extern "C" Box* getitem_capi(Box* value, Box* slice) noexcept;
 extern "C" void setitem(Box* target, Box* slice, Box* value);
 extern "C" void delitem(Box* target, Box* slice);
+extern "C" PyObject* apply_slice(PyObject* u, PyObject* v, PyObject* w) noexcept;
 extern "C" Box* getclsattr(Box* obj, BoxedString* attr);
 extern "C" Box* getclsattrMaybeNonstring(Box* obj, Box* attr);
 extern "C" Box* unaryop(Box* operand, int op_type);

test/extra/pyopenssl_test.py

@@ -10,5 +10,12 @@ PYOPENSSL_DIR = os.path.abspath(os.path.join(ENV_NAME, "site-packages", "OpenSSL
 packages = ["nose==1.3.7", "pycparser==2.13", "cryptography==1.0.1", "pyopenssl==0.15.1", "pyasn1==0.1.7", "idna==2.0", "six==1.9.0", "enum34==1.0.4", "ipaddress==1.0.14", "cffi==1.1.0"]
 create_virtenv(ENV_NAME, packages, force_create = True)
 
+# This particular test is bad; it depends on certain implementation details of the openssl library
+# it's linked against.  It fails in cpython and for other people as well
+# https://www.mail-archive.com/ports@openbsd.org/msg52063.html
+import subprocess
+subprocess.check_call(["sed", "-i", 's/\\(def test_digest.*\\)/\\1\\n        return/',
+    os.path.join(PYOPENSSL_DIR, "test", "test_crypto.py")])
+
 expected = [{'ran': 247, 'errors': 2}]
 run_test([NOSETESTS_EXE], cwd=PYOPENSSL_DIR, expected=expected)

test/tests/slice.py

@@ -163,9 +163,9 @@ print unicodestr[-2:]
 # Calling the slice operator directly does not have the same behavior
 # as using the slice notation []. Namely, it will not modify negative
 # indices.
-print numbers.__getslice__(0, -1);
-print letters.__getslice__(0, -1);
-print unicodestr.__getslice__(0, -1);
+print numbers.__getslice__(0, -1)
+print letters.__getslice__(0, -1)
+print unicodestr.__getslice__(0, -1)
 
 # Other
 class C(object):
@@ -188,3 +188,9 @@ C()[:,:]
 C()[1:2,3:4]
 C()[1:2:3,3:4:5]
 
+# Regression test:
+def f(i):
+    for j in [1, 2, 3][::2]:
+        pass
+for i in xrange(100000):
+    f(i)