move constants from the module into CodeConstants (=code object)

src/codegen/irgen/irgenerator.cpp

@@ -69,6 +69,10 @@ IRGenState::IRGenState(BoxedCode* code, CompiledFunction* cf, llvm::Function* fu
 IRGenState::~IRGenState() {
 }
 
+const CodeConstants& IRGenState::getCodeConstants() {
+    return code->code_constants;
+}
+
 llvm::Value* IRGenState::getPassedClosure() {
     assert(getScopeInfo().takesClosure());
     assert(passed_closure);
@@ -721,9 +725,9 @@ public:
         return rtn;
     }
 
-    Box* getIntConstant(int64_t n) override { return irstate->getSourceInfo()->parent_module->getIntConstant(n); }
+    Box* getIntConstant(int64_t n) override { return irstate->getCodeConstants().getIntConstant(n); }
 
-    Box* getFloatConstant(double d) override { return irstate->getSourceInfo()->parent_module->getFloatConstant(d); }
+    Box* getFloatConstant(double d) override { return irstate->getCodeConstants().getFloatConstant(d); }
 
     void refConsumed(llvm::Value* v, llvm::Instruction* inst) override {
         irstate->getRefcounts()->refConsumed(v, inst);

src/codegen/irgen/irgenerator.h

@@ -92,6 +92,7 @@ public:
 
     CompiledFunction* getCurFunction() { return cf; }
     BoxedCode* getCode() { return code; }
+    const CodeConstants& getCodeConstants();
 
     ExceptionStyle getExceptionStyle() { return cf->exception_style; }
 

src/core/cfg.cpp

@@ -27,6 +27,8 @@
 #include "core/bst.h"
 #include "core/options.h"
 #include "core/types.h"
+#include "runtime/complex.h"
+#include "runtime/long.h"
 #include "runtime/objmodel.h"
 #include "runtime/types.h"
 
@@ -362,9 +364,19 @@ private:
     CFGBlock* curblock;
     std::vector<ContInfo> continuations;
     std::vector<ExcBlockInfo> exc_handlers;
+
+    // maps constants to their vreg number
     llvm::DenseMap<Box*, int> consts;
     CodeConstants code_constants;
 
+    llvm::StringMap<BoxedString*> str_constants;
+    llvm::StringMap<Box*> unicode_constants;
+    // I'm not sure how well it works to use doubles as hashtable keys; thankfully
+    // it's not a big deal if we get misses.
+    std::unordered_map<int64_t, Box*> imaginary_constants;
+    llvm::StringMap<Box*> long_constants;
+    llvm::DenseMap<InternedString, int> interned_string_constants;
+
     unsigned int next_var_index = 0;
 
     friend std::pair<CFG*, CodeConstants> computeCFG(llvm::ArrayRef<AST_stmt*> body, AST_TYPE::AST_TYPE ast_type,
@@ -518,16 +530,39 @@ private:
         return vreg;
     }
 
+    static int64_t getDoubleBits(double d) {
+        int64_t rtn;
+        static_assert(sizeof(rtn) == sizeof(d), "");
+        memcpy(&rtn, &d, sizeof(d));
+        return rtn;
+    }
+
+    TmpValue makeNum(int64_t n, int lineno) {
+        Box* o = code_constants.getIntConstant(n);
+        int vreg_const = addConst(o);
+        return TmpValue(vreg_const, lineno);
+    }
+
     TmpValue remapNum(AST_Num* num) {
         Box* o = NULL;
         if (num->num_type == AST_Num::INT) {
-            o = source->parent_module->getIntConstant(num->n_int);
+            o = code_constants.getIntConstant(num->n_int);
         } else if (num->num_type == AST_Num::FLOAT) {
-            o = source->parent_module->getFloatConstant(num->n_float);
+            o = code_constants.getFloatConstant(num->n_float);
         } else if (num->num_type == AST_Num::LONG) {
-            o = source->parent_module->getLongConstant(num->n_long);
+            Box*& r = long_constants[num->n_long];
+            if (!r) {
+                r = createLong(num->n_long);
+                code_constants.addOwnedRef(r);
+            }
+            o = r;
         } else if (num->num_type == AST_Num::COMPLEX) {
-            o = source->parent_module->getPureImaginaryConstant(num->n_float);
+            Box*& r = imaginary_constants[getDoubleBits(num->n_float)];
+            if (!r) {
+                r = createPureImaginary(num->n_float);
+                code_constants.addOwnedRef(r);
+            }
+            o = r;
         } else
             RELEASE_ASSERT(0, "not implemented");
 
@@ -535,23 +570,34 @@ private:
         return TmpValue(vreg_const, num->lineno);
     }
 
+    TmpValue makeStr(llvm::StringRef str, int lineno = 0) {
+        BoxedString*& o = str_constants[str];
+        // we always intern the string
+        if (!o) {
+            o = internStringMortal(str);
+            code_constants.addOwnedRef(o);
+        }
+        int vreg_const = addConst(o);
+        return TmpValue(vreg_const, lineno);
+    }
+
     TmpValue remapStr(AST_Str* str) {
         // TODO make this serializable
-        Box* o = NULL;
         if (str->str_type == AST_Str::STR) {
-            o = source->parent_module->getStringConstant(str->str_data, true);
+            return makeStr(str->str_data, str->lineno);
         } else if (str->str_type == AST_Str::UNICODE) {
-            o = source->parent_module->getUnicodeConstant(str->str_data);
-        } else {
-            RELEASE_ASSERT(0, "%d", str->str_type);
+            Box*& r = unicode_constants[str->str_data];
+            if (!r) {
+                r = decodeUTF8StringPtr(str->str_data);
+                code_constants.addOwnedRef(r);
+            }
+            return TmpValue(addConst(r), str->lineno);
         }
-
-        int vreg_const = addConst(o);
-        return TmpValue(vreg_const, str->lineno);
+        RELEASE_ASSERT(0, "%d", str->str_type);
     }
 
     TmpValue makeNum(int n, int lineno) {
-        Box* o = source->parent_module->getIntConstant(n);
+        Box* o = code_constants.getIntConstant(n);
         int vreg_const = addConst(o);
         return TmpValue(vreg_const, lineno);
     }
@@ -561,12 +607,6 @@ private:
         return TmpValue(vreg_const, lineno);
     }
 
-    TmpValue makeStr(llvm::StringRef str, int lineno = 0) {
-        Box* o = source->parent_module->getStringConstant(str, true);
-        int vreg_const = addConst(o);
-        return TmpValue(vreg_const, lineno);
-    }
-
     TmpValue applyComprehensionCall(AST_ListComp* node, TmpValue name) {
         TmpValue elt = remapExpr(node->elt);
         return makeCallAttr(name, internString("append"), true, { elt });
@@ -3334,7 +3374,7 @@ static std::pair<CFG*, CodeConstants> computeCFG(llvm::ArrayRef<AST_stmt*> body,
         rtn->print(visitor.code_constants, llvm::outs());
     }
 
-    return std::make_pair(rtn, visitor.code_constants);
+    return std::make_pair(rtn, std::move(visitor.code_constants));
 }
 
 

src/runtime/types.cpp

@@ -480,68 +480,6 @@ std::string BoxedModule::name() {
     }
 }
 
-BORROWED(BoxedString*) BoxedModule::getStringConstant(llvm::StringRef ast_str, bool intern) {
-    BoxedString*& r = str_constants[ast_str];
-    if (intern) {
-        // If we had previously created a box for this string, we have to create a new
-        // string (or at least, be prepared to return a different value that we had already
-        // interned).  This is fine, except we have to be careful because we promised
-        // that we would keep the previously-created string alive.
-        // So, make sure to put it onto the keep_alive list.
-        if (r && !PyString_CHECK_INTERNED(r)) {
-            RELEASE_ASSERT(0, "this codepath has been dead for a little while, make sure it still works");
-            keep_alive.push_back(r);
-            r = NULL;
-        }
-        if (!r)
-            r = internStringMortal(ast_str);
-    } else if (!r)
-        r = boxString(ast_str);
-    return r;
-}
-
-BORROWED(Box*) BoxedModule::getUnicodeConstant(llvm::StringRef ast_str) {
-    Box*& r = unicode_constants[ast_str];
-    if (!r)
-        r = decodeUTF8StringPtr(ast_str);
-    return r;
-}
-
-BORROWED(BoxedInt*) BoxedModule::getIntConstant(int64_t n) {
-    BoxedInt*& r = int_constants[n];
-    if (!r)
-        r = (BoxedInt*)boxInt(n);
-    return r;
-}
-
-static int64_t getDoubleBits(double d) {
-    int64_t rtn;
-    static_assert(sizeof(rtn) == sizeof(d), "");
-    memcpy(&rtn, &d, sizeof(d));
-    return rtn;
-}
-
-BORROWED(BoxedFloat*) BoxedModule::getFloatConstant(double d) {
-    BoxedFloat*& r = float_constants[getDoubleBits(d)];
-    if (!r)
-        r = static_cast<BoxedFloat*>(boxFloat(d));
-    return r;
-}
-
-BORROWED(Box*) BoxedModule::getPureImaginaryConstant(double d) {
-    Box*& r = imaginary_constants[getDoubleBits(d)];
-    if (!r)
-        r = createPureImaginary(d);
-    return r;
-}
-
-BORROWED(Box*) BoxedModule::getLongConstant(llvm::StringRef ast_str) {
-    Box*& r = long_constants[ast_str];
-    if (!r)
-        r = createLong(ast_str);
-    return r;
-}
-
 // This mustn't throw; our IR generator generates calls to it without "invoke" even when there are exception handlers /
 // finally-blocks in scope.
 extern "C" Box* createFunctionFromMetadata(BoxedCode* code, BoxedClosure* closure, Box* globals,
@@ -3817,21 +3755,12 @@ void BoxedModule::dealloc(Box* b) noexcept {
 
     BoxedModule::clear(self);
 
-    self->str_constants.~ContiguousMap();
-    self->unicode_constants.~ContiguousMap();
-    self->int_constants.~ContiguousMap();
-    self->float_constants.~ContiguousMap();
-    self->imaginary_constants.~ContiguousMap();
-    self->long_constants.~ContiguousMap();
-    assert(!self->keep_alive.size());
-
     b->cls->tp_free(self);
 }
 
 int BoxedModule::traverse(Box* _m, visitproc visit, void* arg) noexcept {
     BoxedModule* m = static_cast<BoxedModule*>(_m);
     Py_TRAVERSE(m->attrs);
-    assert(!m->keep_alive.size());
     return 0;
 }
 
@@ -3849,15 +3778,6 @@ extern "C" void _PyModule_Clear(PyObject* b) noexcept {
 
     HCAttrs* attrs = self->getHCAttrsPtr();
     attrs->moduleClear();
-
-    clearContiguousMap(self->str_constants);
-    clearContiguousMap(self->unicode_constants);
-    clearContiguousMap(self->int_constants);
-    clearContiguousMap(self->float_constants);
-    clearContiguousMap(self->imaginary_constants);
-    clearContiguousMap(self->long_constants);
-
-    assert(!self->keep_alive.size());
 }
 
 int BoxedModule::clear(Box* b) noexcept {
@@ -4102,6 +4022,33 @@ int BoxedClosure::clear(Box* _o) noexcept {
     return 0;
 }
 
+BORROWED(BoxedInt*) CodeConstants::getIntConstant(int64_t n) const {
+    BoxedInt*& r = int_constants[n];
+    if (!r) {
+        r = (BoxedInt*)boxInt(n);
+        addOwnedRef(r);
+    }
+    return r;
+}
+
+BORROWED(BoxedFloat*) CodeConstants::getFloatConstant(double d) const {
+    int64_t double_as_int64;
+    static_assert(sizeof(double_as_int64) == sizeof(d), "");
+    memcpy(&double_as_int64, &d, sizeof(d));
+
+    BoxedFloat*& r = float_constants[double_as_int64];
+    if (!r) {
+        r = (BoxedFloat*)boxFloat(d);
+        addOwnedRef(r);
+    }
+    return r;
+}
+
+void CodeConstants::dealloc() const {
+    decrefArray(owned_refs.data(), owned_refs.size());
+    owned_refs.clear();
+}
+
 #ifndef Py_REF_DEBUG
 #define PRINT_TOTAL_REFS()
 #else /* Py_REF_DEBUG */

src/runtime/types.h

@@ -1072,18 +1072,40 @@ static_assert(sizeof(BoxedDict) == sizeof(PyDictObject), "");
 
 class CodeConstants {
 private:
+    // stores all constants accessible by vregs in the corrext order
+    // constants[-(vreg + 1)] will allow one to retrieve the constant for a vreg
     std::vector<Box*> constants;
 
+    // all objects we need to decref when the code object dies
+    mutable std::vector<Box*> owned_refs;
+
+    // Note: DenseMap doesn't work here since we don't prevent the tombstone/empty
+    // keys from reaching it.
+    mutable std::unordered_map<int64_t, BoxedInt*> int_constants;
+    // I'm not sure how well it works to use doubles as hashtable keys; thankfully
+    // it's not a big deal if we get misses.
+    mutable std::unordered_map<int64_t, BoxedFloat*> float_constants;
+
 public:
     CodeConstants() {}
+    CodeConstants(CodeConstants&&) = default;
+    CodeConstants& operator=(CodeConstants&&) = default;
+    ~CodeConstants() { dealloc(); }
 
-    Box* getConstant(int vreg) const { return constants[-(vreg + 1)]; }
+    BORROWED(Box*) getConstant(int vreg) const { return constants[-(vreg + 1)]; }
 
     // returns the vreg num for the constant (which is a negative number)
     int createVRegEntryForConstant(Box* o) {
         constants.push_back(o);
         return -constants.size();
     }
+
+    void addOwnedRef(Box* o) const { owned_refs.emplace_back(o); }
+
+    BORROWED(BoxedInt*) getIntConstant(int64_t n) const;
+    BORROWED(BoxedFloat*) getFloatConstant(double d) const;
+
+    void dealloc() const;
 };