More small improvements

src/asm_writing/rewriter.cpp

@@ -1205,8 +1205,6 @@ void RewriterVar::bumpUse() {
         }
 
         this->locations.clear();
-    } else {
-        assert(vrefcount > 0);
     }
 }
 
@@ -1721,6 +1719,33 @@ void Rewriter::_checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val) {
     assertConsistent();
 }
 
+void RewriterVar::addAssertLastActionAction(int num_left) {
+    static int n = 0;
+    n++;
+    // printf("addAssertLastActionAction(); %d\n", n);
+
+    int this_n = n;
+
+    // The rewriter adds a phony use of all constants to keep them alive:
+    if (this->is_constant)
+        num_left++;
+
+    rewriter->addAction([=]() {
+        if (this->next_use + num_left != this->uses.size()) {
+            printf("n: %d\n", this_n);
+            printf("this: %p\n", this);
+            printf("uses: %p\n", &this->uses);
+            // for (auto idx : this->uses) {
+            // printf("Action: %p\n", rewriter->actions[idx].action.func);
+            // }
+            printf("%ld\n", this->uses.size());
+            raise(SIGTRAP);
+        }
+        RELEASE_ASSERT(this->next_use + num_left == this->uses.size(), "%d %d %ld\n", this->next_use, num_left,
+                       this->uses.size());
+    }, {}, ActionType::NORMAL);
+}
+
 assembler::Indirect Rewriter::indirectFor(Location l) {
     assert(l.type == Location::Scratch || l.type == Location::Stack);
 

src/asm_writing/rewriter.h

@@ -246,19 +246,26 @@ public:
         return this;
     }
 
+    // Assert that there are exactly num_left actions get added after this call.
+    // Meant for debugging.  It's a way to cross between the two rewriter phases.
+    void addAssertLastActionAction(int num_left=0);
+
     RewriterVar* decvref() {
         assert(reftype == RefType::OWNED || reftype == RefType::BORROWED);
         assert(vrefcount > 0);
         vrefcount--;
 
         if (vrefcount == 0) {
-            // Assert that there are no more uses of the this at the machine-code level.
-            // This is kind of weird to cross this abstraction boundary like this, but
-            // I think it's a good safety net.
-            assert(next_use == uses.size());
 
             if (reftype == RefType::OWNED)
                 decref();
+
+            // Assert that there are no more uses of the this at the machine-code level.
+            // This is kind of weird to cross this abstraction boundary like this, but
+            // I think it's a good safety net.
+#ifndef NDEBUG
+            addAssertLastActionAction();
+#endif
         }
         return this;
     }
@@ -279,6 +286,9 @@ public:
             // See the comment in decvref.
             // This is similar, except that we want there to be exactly one more use of this variable:
             // whatever consumes the vref.  We want that to happen after the materializeVref call.
+#ifndef NDEBUG
+            addAssertLastActionAction(1);
+#endif
         }
         return this;
     }
@@ -710,6 +720,30 @@ public:
     friend class RewriterVar;
 };
 
+struct AutoDecvref {
+private:
+    RewriterVar* var;
+
+public:
+    AutoDecvref(RewriterVar* var) : var(var) {}
+    ~AutoDecvref() {
+        if (var)
+            var->decvref();
+    }
+
+    operator RewriterVar*() {
+        return var;
+    }
+    RewriterVar* operator->() {
+        return var;
+    }
+
+    void operator=(RewriterVar* new_var) {
+        assert(!var);
+        var = new_var;
+    }
+};
+
 void setSlowpathFunc(uint8_t* pp_addr, void* func);
 
 struct GRCompare {

src/codegen/baseline_jit.cpp

@@ -163,7 +163,7 @@ RewriterVar* JitFragmentWriter::imm(void* val) {
 }
 
 RewriterVar* JitFragmentWriter::emitAugbinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type) {
-    return emitPPCall((void*)augbinop, { lhs, rhs, imm(op_type) }, 2, 320, node);
+    return emitPPCall((void*)augbinop, { lhs, rhs, imm(op_type) }, 2, 320, node)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitBinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type) {
@@ -199,7 +199,7 @@ RewriterVar* JitFragmentWriter::emitCallattr(AST_expr* node, RewriterVar* obj, B
     if (keyword_names)
         call_args.push_back(imm(keyword_names));
 
-    return emitPPCall((void*)callattr, call_args, 2, 640, node, type_recorder);
+    return emitPPCall((void*)callattr, call_args, 2, 640, node, type_recorder)->setType(RefType::OWNED);
 #else
     // We could make this faster but for now: keep it simple, stupid...
     RewriterVar* attr_var = imm(attr);
@@ -230,7 +230,7 @@ RewriterVar* JitFragmentWriter::emitCallattr(AST_expr* node, RewriterVar* obj, B
 
 RewriterVar* JitFragmentWriter::emitCompare(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type) {
     // TODO: can directly emit the assembly for Is/IsNot
-    return emitPPCall((void*)compare, { lhs, rhs, imm(op_type) }, 2, 240, node);
+    return emitPPCall((void*)compare, { lhs, rhs, imm(op_type) }, 2, 240, node)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitCreateDict(const llvm::ArrayRef<RewriterVar*> keys,
@@ -296,7 +296,7 @@ RewriterVar* JitFragmentWriter::emitExceptionMatches(RewriterVar* v, RewriterVar
 }
 
 RewriterVar* JitFragmentWriter::emitGetAttr(RewriterVar* obj, BoxedString* s, AST_expr* node) {
-    return emitPPCall((void*)getattr, { obj, imm(s) }, 2, 512, node, getTypeRecorderForNode(node));
+    return emitPPCall((void*)getattr, { obj, imm(s) }, 2, 512, node, getTypeRecorderForNode(node))->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitGetBlockLocal(InternedString s, int vreg) {
@@ -318,7 +318,7 @@ RewriterVar* JitFragmentWriter::emitGetBoxedLocals() {
 }
 
 RewriterVar* JitFragmentWriter::emitGetClsAttr(RewriterVar* obj, BoxedString* s) {
-    return emitPPCall((void*)getclsattr, { obj, imm(s) }, 2, 512);
+    return emitPPCall((void*)getclsattr, { obj, imm(s) }, 2, 512)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitGetGlobal(Box* global, BoxedString* s) {
@@ -334,7 +334,7 @@ RewriterVar* JitFragmentWriter::emitGetGlobal(Box* global, BoxedString* s) {
 }
 
 RewriterVar* JitFragmentWriter::emitGetItem(AST_expr* node, RewriterVar* value, RewriterVar* slice) {
-    return emitPPCall((void*)getitem, { value, slice }, 2, 512, node);
+    return emitPPCall((void*)getitem, { value, slice }, 2, 512, node)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitGetLocal(InternedString s, int vreg) {
@@ -431,7 +431,7 @@ RewriterVar* JitFragmentWriter::emitRuntimeCall(AST_expr* node, RewriterVar* obj
 }
 
 RewriterVar* JitFragmentWriter::emitUnaryop(RewriterVar* v, int op_type) {
-    return emitPPCall((void*)unaryop, { v, imm(op_type) }, 2, 160);
+    return emitPPCall((void*)unaryop, { v, imm(op_type) }, 2, 160)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitUnpackIntoArray(RewriterVar* v, uint64_t num) {
@@ -445,16 +445,16 @@ RewriterVar* JitFragmentWriter::emitYield(RewriterVar* v) {
 }
 
 void JitFragmentWriter::emitDelAttr(RewriterVar* target, BoxedString* attr) {
-    emitPPCall((void*)delattr, { target, imm(attr) }, 1, 512);
+    emitPPCall((void*)delattr, { target, imm(attr) }, 1, 512)->setType(RefType::OWNED);
 }
 
 void JitFragmentWriter::emitDelGlobal(BoxedString* name) {
     RewriterVar* globals = getInterp()->getAttr(ASTInterpreterJitInterface::getGlobalsOffset());
-    emitPPCall((void*)delGlobal, { globals, imm(name) }, 1, 512);
+    emitPPCall((void*)delGlobal, { globals, imm(name) }, 1, 512)->setType(RefType::OWNED);
 }
 
 void JitFragmentWriter::emitDelItem(RewriterVar* target, RewriterVar* slice) {
-    emitPPCall((void*)delitem, { target, slice }, 1, 512);
+    emitPPCall((void*)delitem, { target, slice }, 1, 512)->setType(RefType::OWNED);
 }
 
 void JitFragmentWriter::emitDelName(InternedString name) {
@@ -518,7 +518,7 @@ void JitFragmentWriter::emitReturn(RewriterVar* v) {
 }
 
 void JitFragmentWriter::emitSetAttr(AST_expr* node, RewriterVar* obj, BoxedString* s, RewriterVar* attr) {
-    emitPPCall((void*)setattr, { obj, imm(s), attr }, 2, 512, node);
+    emitPPCall((void*)setattr, { obj, imm(s), attr }, 2, 512, node)->setType(RefType::OWNED);
 }
 
 void JitFragmentWriter::emitSetBlockLocal(InternedString s, RewriterVar* v) {
@@ -539,11 +539,11 @@ void JitFragmentWriter::emitSetExcInfo(RewriterVar* type, RewriterVar* value, Re
 }
 
 void JitFragmentWriter::emitSetGlobal(Box* global, BoxedString* s, RewriterVar* v) {
-    emitPPCall((void*)setGlobal, { imm(global), imm(s), v }, 2, 512);
+    emitPPCall((void*)setGlobal, { imm(global), imm(s), v }, 2, 512)->setType(RefType::OWNED);
 }
 
 void JitFragmentWriter::emitSetItem(RewriterVar* target, RewriterVar* slice, RewriterVar* value) {
-    emitPPCall((void*)setitem, { target, slice, value }, 2, 512);
+    emitPPCall((void*)setitem, { target, slice, value }, 2, 512)->setType(RefType::OWNED);
 }
 
 void JitFragmentWriter::emitSetItemName(BoxedString* s, RewriterVar* v) {
@@ -563,6 +563,7 @@ void JitFragmentWriter::emitSetLocal(InternedString s, int vreg, bool set_closur
              v);
     } else {
         RewriterVar* prev = vregs_array->getAttr(8 * vreg)->setType(RefType::OWNED);
+        v->materializeVref();
         vregs_array->setAttr(8 * vreg, v);
         // XXX: this either needs to be an xdecref or we should check liveness analysis.
         prev->decvref();
@@ -990,7 +991,8 @@ void JitFragmentWriter::_emitSideExit(RewriterVar* var, RewriterVar* val_constan
     }
 
     {
-        assembler::ForwardJump jne(*assembler, assembler::COND_EQUAL);
+        // TODO: Figure out if we need a large/small forward based on the number of local syms we will have to decref?
+        assembler::LargeForwardJump jne(*assembler, assembler::COND_EQUAL);
 
         _decref(var);
 

src/codegen/baseline_jit.h

@@ -258,7 +258,7 @@ public:
     void emitRaise3(RewriterVar* arg0, RewriterVar* arg1, RewriterVar* arg2);
     void emitReturn(RewriterVar* v);
     void emitSetAttr(AST_expr* node, RewriterVar* obj, BoxedString* s, RewriterVar* attr);
-    void emitSetBlockLocal(InternedString s, RewriterVar* v);
+    void emitSetBlockLocal(InternedString s, STOLEN(RewriterVar*) v);
     void emitSetCurrentInst(AST_stmt* node);
     void emitSetExcInfo(RewriterVar* type, RewriterVar* value, RewriterVar* traceback);
     void emitSetGlobal(Box* global, BoxedString* s, RewriterVar* v);

src/runtime/objmodel.cpp

@@ -1282,7 +1282,6 @@ template <Rewritable rewritable> Box* typeLookup(BoxedClass* cls, BoxedString* a
             if (!val)
                 rewrite_args->setReturn(NULL, ReturnConvention::NO_RETURN);
             else {
-                // typeLookup is special since it doesn't even return a vref
                 rewrite_args->setReturn(
                     rewrite_args->rewriter->loadConst((int64_t)val)->setType(RefType::BORROWED),
                     ReturnConvention::HAS_RETURN);
@@ -1945,7 +1944,7 @@ Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rew
     // Look up the class attribute (called `descr` here because it might
     // be a descriptor).
     Box* descr = NULL;
-    RewriterVar* r_descr = NULL;
+    AutoDecvref r_descr = NULL;
     if (rewrite_args) {
         RewriterVar* r_obj_cls = rewrite_args->obj->getAttr(offsetof(Box, cls), Location::any());
         GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, r_obj_cls, rewrite_args->destination);
@@ -3575,9 +3574,9 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
                 rewrite_args->arg3->incvref();
             if (num_output_args >= 3) {
                 RELEASE_ASSERT(0, "not sure what to do here\n");
-                for (int i = 0; i < num_output_args - 3; i++) {
-                    rewrite_args->args->getAttr(i * sizeof(Box*))->incref();
-                }
+                //for (int i = 0; i < num_output_args - 3; i++) {
+                    //rewrite_args->args->getAttr(i * sizeof(Box*))->incref();
+                //}
             }
         }
         return;
@@ -4858,6 +4857,8 @@ extern "C" Box* binop(Box* lhs, Box* rhs, int op_type) {
         if (!rewrite_args.out_success) {
             rewriter.reset(NULL);
         } else {
+            rewriter->getArg(0)->decvref();
+            rewriter->getArg(1)->decvref();
             rewrite_args.out_rtn->materializeVref();
             rewriter->commitReturning(rewrite_args.out_rtn);
         }