More fixes. Need to support nullable in more places

src/codegen/compvars.cpp

@@ -1409,8 +1409,10 @@ public:
             // Will this ever hit the cache?
             boxed = embedRelocatablePtr(emitter.getFloatConstant(llvm_val->getValueAPF().convertToDouble()),
                                         g.llvm_value_type_ptr);
+            emitter.setType(boxed, RefType::BORROWED);
         } else {
             boxed = emitter.getBuilder()->CreateCall(g.funcs.boxFloat, unboxed);
+            emitter.setType(boxed, RefType::OWNED);
         }
         return new ConcreteCompilerVariable(other_type, boxed);
     }

src/codegen/irgen.h

@@ -195,7 +195,8 @@ class IRGenState;
 class RefcountTracker {
 private:
     struct RefcountState {
-        RefType reftype;
+        RefType reftype = RefType::UNKNOWN;
+        bool nullable = false;
 
         //llvm::SmallVector<llvm::Instruction*, 2> ref_consumers;
     };
@@ -204,6 +205,7 @@ private:
 public:
 
     llvm::Value* setType(llvm::Value* v, RefType reftype);
+    llvm::Value* setNullable(llvm::Value* v, bool nullable = true);
     void refConsumed(llvm::Value* v, llvm::Instruction*);
     static void addRefcounts(IRGenState* state);
 };

src/codegen/irgen/irgenerator.cpp

@@ -526,6 +526,11 @@ public:
         return v;
     }
 
+    llvm::Value* setNullable(llvm::Value* v, bool nullable) {
+        irstate->getRefcounts()->setNullable(v, nullable);
+        return v;
+    }
+
     ConcreteCompilerVariable* getNone() {
         llvm::Constant* none = embedRelocatablePtr(None, g.llvm_value_type_ptr, "cNone");
         setType(none, RefType::BORROWED);
@@ -740,9 +745,11 @@ private:
                     // For example, the cfg code will conservatively assume that any name-access can
                     // trigger an exception, but the irgenerator will know that definitely-defined
                     // local symbols will not throw.
+                    emitter.getBuilder()->CreateUnreachable();
                     exc_type = undefVariable();
                     exc_value = undefVariable();
                     exc_tb = undefVariable();
+                    endBlock(DEAD);
                 }
 
                 // clear this out to signal that we consumed them:
@@ -2668,23 +2675,31 @@ public:
         }
 
         if (param_names.kwarg.size()) {
+            llvm::Value* passed_dict = python_parameters[i];
+            emitter.setNullable(passed_dict, true);
+
             llvm::BasicBlock* starting_block = emitter.currentBasicBlock();
             llvm::BasicBlock* isnull_bb = emitter.createBasicBlock("isnull");
             llvm::BasicBlock* continue_bb = emitter.createBasicBlock("kwargs_join");
 
             llvm::Value* kwargs_null
-                = emitter.getBuilder()->CreateICmpEQ(python_parameters[i], getNullPtr(g.llvm_value_type_ptr));
+                = emitter.getBuilder()->CreateICmpEQ(passed_dict, getNullPtr(g.llvm_value_type_ptr));
             llvm::BranchInst* null_check = emitter.getBuilder()->CreateCondBr(kwargs_null, isnull_bb, continue_bb);
 
             emitter.setCurrentBasicBlock(isnull_bb);
             llvm::Value* created_dict = emitter.getBuilder()->CreateCall(g.funcs.createDict);
-            emitter.getBuilder()->CreateBr(continue_bb);
+            emitter.setType(created_dict, RefType::OWNED);
+            auto isnull_terminator = emitter.getBuilder()->CreateBr(continue_bb);
 
             emitter.setCurrentBasicBlock(continue_bb);
             llvm::PHINode* phi = emitter.getBuilder()->CreatePHI(g.llvm_value_type_ptr, 2);
-            phi->addIncoming(python_parameters[i], starting_block);
+            phi->addIncoming(passed_dict, starting_block);
             phi->addIncoming(created_dict, isnull_bb);
 
+            emitter.setType(phi, RefType::OWNED);
+            emitter.refConsumed(passed_dict, null_check);
+            emitter.refConsumed(created_dict, isnull_terminator);
+
             loadArgument(internString(param_names.kwarg), arg_types[i], phi, UnwindInfo::cantUnwind());
             i++;
         }

src/codegen/irgen/refcounts.cpp

@@ -52,6 +52,16 @@ llvm::Value* RefcountTracker::setType(llvm::Value* v, RefType reftype) {
     return v;
 }
 
+llvm::Value* RefcountTracker::setNullable(llvm::Value* v, bool nullable) {
+    assert(!llvm::isa<llvm::UndefValue>(v));
+
+    auto& var = this->vars[v];
+
+    assert(var.nullable == nullable || var.nullable == false);
+    var.nullable = nullable;
+    return v;
+}
+
 void RefcountTracker::refConsumed(llvm::Value* v, llvm::Instruction* inst) {
     assert(this->vars[v].reftype != RefType::UNKNOWN);
 
@@ -63,21 +73,23 @@ llvm::Instruction* findIncrefPt(llvm::BasicBlock* BB) {
     ASSERT(pred_begin(BB) == pred_end(BB) || pred_end(BB) == ++pred_begin(BB),
            "We shouldn't be inserting anything at the beginning of blocks with multiple predecessors");
 
-    llvm::Instruction* incref_pt;// = BB->getFirstInsertionPt();
     if (llvm::isa<llvm::LandingPadInst>(*BB->begin())) {
         // Don't split up the landingpad+extract+cxa_begin_catch
         auto it = BB->begin();
         ++it;
         ++it;
         ++it;
-        incref_pt = &*it;
+        return &*it;
     } else {
-        incref_pt = BB->getFirstInsertionPt();
+        for (llvm::Instruction& I : *BB) {
+            if (!llvm::isa<llvm::PHINode>(I) && !llvm::isa<llvm::AllocaInst>(I))
+                return &I;
+        }
+        abort();
     }
-    return incref_pt;
 }
 
-void addIncrefs(llvm::Value* v, int num_refs, llvm::Instruction* incref_pt) {
+void addIncrefs(llvm::Value* v, bool nullable, int num_refs, llvm::Instruction* incref_pt) {
     if (num_refs > 1) {
         // Not bad but I don't think this should happen:
         //printf("Whoa more than one incref??\n");
@@ -85,29 +97,55 @@ void addIncrefs(llvm::Value* v, int num_refs, llvm::Instruction* incref_pt) {
     }
 
     assert(num_refs > 0);
+
+    llvm::BasicBlock* cur_block;
+    llvm::BasicBlock* continue_block;
+    llvm::BasicBlock* incref_block;
+
+    llvm::IRBuilder<true> builder(incref_pt);
+
+    if (nullable) {
+        cur_block = incref_pt->getParent();
+        continue_block = cur_block->splitBasicBlock(incref_pt);
+        incref_block
+            = llvm::BasicBlock::Create(g.context, "incref", incref_pt->getParent()->getParent(), continue_block);
+
+        assert(llvm::isa<llvm::BranchInst>(cur_block->getTerminator()));
+        cur_block->getTerminator()->eraseFromParent();
+
+        builder.SetInsertPoint(cur_block);
+        auto isnull = builder.CreateICmpEQ(v, getNullPtr(g.llvm_value_type_ptr));
+        builder.CreateCondBr(isnull, continue_block, incref_block);
+
+        builder.SetInsertPoint(incref_block);
+    }
+
 #ifdef Py_REF_DEBUG
     auto reftotal_gv = g.cur_module->getOrInsertGlobal("_Py_RefTotal", g.i64);
-    auto reftotal = new llvm::LoadInst(reftotal_gv, "", incref_pt);
-    auto new_reftotal = llvm::BinaryOperator::Create(llvm::BinaryOperator::BinaryOps::Add, reftotal,
-                                                     getConstantInt(num_refs, g.i64), "", incref_pt);
-    new llvm::StoreInst(new_reftotal, reftotal_gv, incref_pt);
+    auto reftotal = builder.CreateLoad(reftotal_gv);
+    auto new_reftotal = builder.CreateAdd(reftotal, getConstantInt(num_refs, g.i64));
+    builder.CreateStore(new_reftotal, reftotal_gv);
 #endif
 
     llvm::ArrayRef<llvm::Value*> idxs({ getConstantInt(0, g.i32), getConstantInt(0, g.i32) });
-    auto refcount_ptr = llvm::GetElementPtrInst::CreateInBounds(v, idxs, "", incref_pt);
-    auto refcount = new llvm::LoadInst(refcount_ptr, "", incref_pt);
-    auto new_refcount = llvm::BinaryOperator::Create(llvm::BinaryOperator::BinaryOps::Add, refcount,
-                                                     getConstantInt(num_refs, g.i64), "", incref_pt);
-    new llvm::StoreInst(new_refcount, refcount_ptr, incref_pt);
+    auto refcount_ptr = builder.CreateConstInBoundsGEP2_32(v, 0, 0);
+    auto refcount = builder.CreateLoad(refcount_ptr);
+    auto new_refcount = builder.CreateAdd(refcount, getConstantInt(num_refs, g.i64));
+    builder.CreateStore(new_refcount, refcount_ptr);
+
+    if (nullable)
+        builder.CreateBr(continue_block);
 }
 
-void addDecrefs(llvm::Value* v, int num_refs, llvm::Instruction* decref_pt) {
+void addDecrefs(llvm::Value* v, bool nullable, int num_refs, llvm::Instruction* decref_pt) {
     if (num_refs > 1) {
         // Not bad but I don't think this should happen:
         printf("Whoa more than one decref??\n");
         raise(SIGTRAP);
     }
 
+    assert(!nullable);
+
     assert(num_refs > 0);
     llvm::IRBuilder<true> builder(decref_pt);
 #ifdef Py_REF_DEBUG
@@ -392,6 +430,7 @@ void RefcountTracker::addRefcounts(IRGenState* irstate) {
 
     struct RefOp {
         llvm::Value* operand;
+        bool nullable;
         int num_refs;
         llvm::Instruction* insertion_pt;
     };
@@ -473,10 +512,10 @@ void RefcountTracker::addRefcounts(IRGenState* irstate) {
                         this_refs = it->second;
                     }
                     if (this_refs > min_refs) {
-                        state.increfs.push_back(RefOp({v, this_refs - min_refs, findIncrefPt(SBB)}));
+                        state.increfs.push_back(RefOp({v, refstate.nullable, this_refs - min_refs, findIncrefPt(SBB)}));
                     } else if (this_refs < min_refs) {
                         assert(refstate.reftype == RefType::OWNED);
-                        state.decrefs.push_back(RefOp({v, min_refs - this_refs, findIncrefPt(SBB)}));
+                        state.decrefs.push_back(RefOp({v, refstate.nullable, min_refs - this_refs, findIncrefPt(SBB)}));
                     }
                 }
 
@@ -530,15 +569,15 @@ void RefcountTracker::addRefcounts(IRGenState* irstate) {
                             llvm::outs() << "Last use of " << *op << " is at " << I << "; adding a decref after\n";
 
                             if (llvm::InvokeInst* invoke = llvm::dyn_cast<llvm::InvokeInst>(&I)) {
-                                state.decrefs.push_back(RefOp({op, 1, findIncrefPt(invoke->getNormalDest())}));
-                                state.decrefs.push_back(RefOp({op, 1, findIncrefPt(invoke->getUnwindDest())}));
+                                state.decrefs.push_back(RefOp({op, rt->vars[op].nullable, 1, findIncrefPt(invoke->getNormalDest())}));
+                                state.decrefs.push_back(RefOp({op, rt->vars[op].nullable, 1, findIncrefPt(invoke->getUnwindDest())}));
                             } else {
                                 assert(&I != I.getParent()->getTerminator());
                                 auto next = I.getNextNode();
                                 //while (llvm::isa<llvm::PHINode>(next))
                                     //next = next->getNextNode();
                                 ASSERT(!llvm::isa<llvm::UnreachableInst>(next), "Can't add decrefs after this function...");
-                                state.decrefs.push_back(RefOp({op, 1, next}));
+                                state.decrefs.push_back(RefOp({op, rt->vars[op].nullable, 1, next}));
                             }
                             state.ending_refs[op] = 1;
                         }
@@ -567,9 +606,11 @@ void RefcountTracker::addRefcounts(IRGenState* irstate) {
 
                     if (state.ending_refs[inst] < starting_refs) {
                         assert(p.second.reftype == RefType::OWNED);
-                        state.decrefs.push_back(RefOp({inst, starting_refs - state.ending_refs[inst], insertion_pt}));
+                        state.decrefs.push_back(
+                            RefOp({ inst, p.second.nullable, starting_refs - state.ending_refs[inst], insertion_pt }));
                     } else {
-                        state.increfs.push_back(RefOp({inst, state.ending_refs[inst] - starting_refs, insertion_pt}));
+                        state.increfs.push_back(
+                            RefOp({ inst, p.second.nullable, state.ending_refs[inst] - starting_refs, insertion_pt }));
                     }
                 }
                 state.ending_refs.erase(inst);
@@ -601,7 +642,7 @@ void RefcountTracker::addRefcounts(IRGenState* irstate) {
 #endif
                 assert(rt->vars[p.first].reftype == RefType::BORROWED);
 
-                state.increfs.push_back(RefOp({p.first, p.second, findIncrefPt(&BB)}));
+                state.increfs.push_back(RefOp({p.first, rt->vars[p.first].nullable, p.second, findIncrefPt(&BB)}));
             }
             state.ending_refs.clear();
         }
@@ -622,9 +663,9 @@ void RefcountTracker::addRefcounts(IRGenState* irstate) {
     for (auto&& p : states) {
         auto&& state = p.second;
         for (auto& op : state.increfs)
-            addIncrefs(op.operand, op.num_refs, op.insertion_pt);
+            addIncrefs(op.operand, op.nullable, op.num_refs, op.insertion_pt);
         for (auto& op : state.decrefs)
-            addDecrefs(op.operand, op.num_refs, op.insertion_pt);
+            addDecrefs(op.operand, op.nullable, op.num_refs, op.insertion_pt);
     }
 
     if (VERBOSITY()) {

src/runtime/objmodel.cpp

@@ -3782,21 +3782,24 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
 
     auto propagate_args = [&]() {
         if (num_output_args >= 1)
-            Py_INCREF(oarg1);
+            Py_XINCREF(oarg1);
         if (num_output_args >= 2)
-            Py_INCREF(oarg2);
+            Py_XINCREF(oarg2);
         if (num_output_args >= 3)
-            Py_INCREF(oarg3);
+            Py_XINCREF(oarg3);
         if (num_output_args >= 3) {
             memcpy(oargs, args, sizeof(Box*) * (num_output_args - 3));
             for (int i = 0; i < num_output_args - 3; i++) {
-                Py_INCREF(oargs[i]);
+                Py_XINCREF(oargs[i]);
             }
         }
         if (rewrite_args) {
             if (num_output_args >= 3) {
                 for (int i = 0; i < num_output_args - 3; i++) {
-                    rewrite_args->args->getAttr(i * sizeof(Box*))->setType(RefType::BORROWED)->refConsumed();
+                    rewrite_args->args->getAttr(i * sizeof(Box*))
+                        ->setType(RefType::BORROWED)
+                        ->setNullable(true)
+                        ->refConsumed();
                 }
             }
         }
@@ -3918,7 +3921,9 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
         getArg(i, oarg1, oarg2, oarg3, oargs) = incref(getArg(i, arg1, arg2, arg3, args));
     }
     if (rewrite_args) {
-        assert(positional_to_positional < 3 && "figure this out");
+        for (int i = 3; i < positional_to_positional; i++) {
+            rewrite_args->args->getAttr((i - 3) * sizeof(Box*))->setType(RefType::BORROWED)->refConsumed();
+        }
     }
 
     int varargs_to_positional = std::min((int)varargs_size, paramspec.num_args - positional_to_positional);
@@ -3939,7 +3944,6 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
 
     RewriterVar::SmallVector unused_positional_rvars;
     for (int i = positional_to_positional; i < argspec.num_args; i++) {
-        assert(!rewrite_args && "check refcounting");
         unused_positional.push_back(getArg(i, arg1, arg2, arg3, args));
         if (rewrite_args) {
             if (i == 0)
@@ -3949,7 +3953,8 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
             if (i == 2)
                 unused_positional_rvars.push_back(rewrite_args->arg3);
             if (i >= 3)
-                unused_positional_rvars.push_back(rewrite_args->args->getAttr((i - 3) * sizeof(Box*)));
+                unused_positional_rvars.push_back(
+                    rewrite_args->args->getAttr((i - 3) * sizeof(Box*))->setType(RefType::BORROWED));
         }
     }
     for (int i = varargs_to_positional; i < varargs_size; i++) {
@@ -3960,7 +3965,6 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
     if (paramspec.takes_varargs) {
         int varargs_idx = paramspec.num_args;
         if (rewrite_args) {
-            assert(0 && "check refcounting");
             assert(!varargs_size);
             assert(!argspec.has_starargs);
 
@@ -3968,8 +3972,10 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
             int varargs_size = unused_positional_rvars.size();
 
             if (varargs_size == 0) {
-                varargs_val = rewrite_args->rewriter->loadConst(
-                    (intptr_t)EmptyTuple, varargs_idx < 3 ? Location::forArg(varargs_idx) : Location::any());
+                varargs_val
+                    = rewrite_args->rewriter->loadConst((intptr_t)EmptyTuple,
+                                                        varargs_idx < 3 ? Location::forArg(varargs_idx)
+                                                                        : Location::any())->setType(RefType::BORROWED);
             } else {
                 assert(varargs_size <= 6);
                 void* create_ptrs[] = {
@@ -3981,7 +3987,8 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
                     (void*)BoxedTuple::create5, //
                     (void*)BoxedTuple::create6, //
                 };
-                varargs_val = rewrite_args->rewriter->call(false, create_ptrs[varargs_size], unused_positional_rvars);
+                varargs_val = rewrite_args->rewriter->call(false, create_ptrs[varargs_size], unused_positional_rvars)
+                                  ->setType(RefType::OWNED);
             }
 
             if (varargs_val) {
@@ -3991,8 +3998,10 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
                     rewrite_args->arg2 = varargs_val;
                 if (varargs_idx == 2)
                     rewrite_args->arg3 = varargs_val;
-                if (varargs_idx >= 3)
+                if (varargs_idx >= 3) {
                     rewrite_args->args->setAttr((varargs_idx - 3) * sizeof(Box*), varargs_val);
+                    varargs_val->refConsumed();
+                }
             }
         }
 
@@ -4025,7 +4034,6 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
     // If you need to access the dict, you should call get_okwargs()
     BoxedDict** _okwargs = NULL;
     if (paramspec.takes_kwargs) {
-        assert(!rewrite_args && "check refcounting");
         int kwargs_idx = paramspec.num_args + (paramspec.takes_varargs ? 1 : 0);
         if (rewrite_args) {
             RewriterVar* r_kwargs = rewrite_args->rewriter->loadConst(0);
@@ -6714,7 +6722,6 @@ extern "C" Box* getGlobal(Box* globals, BoxedString* name) {
                 auto r_rtn = rewrite_args.getReturn(ReturnConvention::HAS_RETURN);
                 rewriter->commitReturning(r_rtn);
             } else {
-                assert(0 && "check refcounting");
                 rewrite_args.getReturn(); // just to make the asserts happy
                 rewriter.reset(NULL);
             }

src/runtime/types.cpp

@@ -1113,7 +1113,7 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
 
         class InitHelper {
         public:
-            static Box* call(Box* made, BoxedClass* cls, Box* args, Box* kwargs) noexcept(S == CAPI) {
+            static Box* call(STOLEN(Box*) made, BoxedClass* cls, Box* args, Box* kwargs) noexcept(S == CAPI) {
                 if (!isSubclass(made->cls, cls))
                     return made;
 
@@ -1132,8 +1132,10 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
         assert(!arg3 || arg3->cls == dict_cls);
 
         if (rewrite_args) {
-            rewrite_args->out_rtn = rewrite_args->rewriter->call(true, (void*)InitHelper::call, r_made, r_ccls,
-                                                                 rewrite_args->arg2, rewrite_args->arg3);
+            rewrite_args->out_rtn
+                = rewrite_args->rewriter->call(true, (void*)InitHelper::call, r_made, r_ccls, rewrite_args->arg2,
+                                               rewrite_args->arg3)->setType(RefType::OWNED);
+            r_made->refConsumed();
             rewrite_args->out_success = true;
         }
         return InitHelper::call(made, cls, arg2, arg3);