Switch frame introspection to use vregs

src/codegen/ast_interpreter.cpp

@@ -172,7 +172,7 @@ public:
     Box** getVRegs() { return vregs; }
     const ScopeInfo* getScopeInfo() { return scope_info; }
 
-    void addSymbol(InternedString name, Box* value, bool allow_duplicates);
+    void addSymbol(int vreg, Box* value, bool allow_duplicates);
     void setGenerator(Box* gen);
     void setPassedClosure(Box* closure);
     void setCreatedClosure(Box* closure);
@@ -182,8 +182,8 @@ public:
     friend struct pyston::ASTInterpreterJitInterface;
 };
 
-void ASTInterpreter::addSymbol(InternedString name, Box* new_value, bool allow_duplicates) {
-    Box*& value = vregs[getVRegInfo().getVReg(name)];
+void ASTInterpreter::addSymbol(int vreg, Box* new_value, bool allow_duplicates) {
+    Box*& value = vregs[vreg];
     Box* old_value = value;
     value = incref(new_value);
     if (allow_duplicates)
@@ -2076,18 +2076,22 @@ extern "C" Box* astInterpretDeoptFromASM(FunctionMetadata* md, AST_expr* after_e
     ASTInterpreter interpreter(md, vregs, frame_state.frame_info);
 
     for (const auto& p : *frame_state.locals) {
-        assert(p.first->cls == str_cls);
-        auto name = static_cast<BoxedString*>(p.first)->s();
-        if (name == PASSED_GENERATOR_NAME) {
-            interpreter.setGenerator(p.second);
-        } else if (name == PASSED_CLOSURE_NAME) {
-            // this should have already got set because its stored in the frame info
-            assert(p.second == interpreter.getFrameInfo()->passed_closure);
-        } else if (name == CREATED_CLOSURE_NAME) {
-            interpreter.setCreatedClosure(p.second);
+        if (p.first->cls == int_cls) {
+            int vreg = static_cast<BoxedInt*>(p.first)->n;
+            interpreter.addSymbol(vreg, p.second, false);
         } else {
-            InternedString interned = md->source->getInternedStrings().get(name);
-            interpreter.addSymbol(interned, p.second, false);
+            assert(p.first->cls == str_cls);
+            auto name = static_cast<BoxedString*>(p.first)->s();
+            if (name == PASSED_GENERATOR_NAME) {
+                interpreter.setGenerator(p.second);
+            } else if (name == PASSED_CLOSURE_NAME) {
+                // this should have already got set because its stored in the frame info
+                assert(p.second == interpreter.getFrameInfo()->passed_closure);
+            } else if (name == CREATED_CLOSURE_NAME) {
+                interpreter.setCreatedClosure(p.second);
+            } else {
+                RELEASE_ASSERT(0, "");
+            }
         }
     }
 
@@ -2101,7 +2105,7 @@ extern "C" Box* astInterpretDeoptFromASM(FunctionMetadata* md, AST_expr* after_e
             assert(asgn->targets[0]->type == AST_TYPE::Name);
             auto name = ast_cast<AST_Name>(asgn->targets[0]);
             assert(name->id.s()[0] == '#');
-            interpreter.addSymbol(name->id, expr_val, true);
+            interpreter.addSymbol(name->vreg, expr_val, true);
             break;
         } else if (enclosing_stmt->type == AST_TYPE::Expr) {
             auto expr = ast_cast<AST_Expr>(enclosing_stmt);

src/codegen/codegen.cpp

@@ -76,9 +76,8 @@ BORROWED(BoxedCode*) FunctionMetadata::getCode() {
 void FunctionMetadata::addVersion(CompiledFunction* compiled) {
     assert(compiled);
     assert((compiled->spec != NULL) + (compiled->entry_descriptor != NULL) == 1);
-    assert(compiled->md == NULL);
+    assert(compiled->md);
     assert(compiled->code);
-    compiled->md = this;
 
     if (compiled->entry_descriptor == NULL) {
         bool could_have_speculations = (source.get() != NULL);

src/codegen/irgen.cpp

@@ -1100,7 +1100,7 @@ CompiledFunction* doCompile(FunctionMetadata* md, SourceInfo* source, ParamNames
         llvm_arg_types.push_back(g.llvm_value_type_ptr->getPointerTo());
     }
 
-    CompiledFunction* cf = new CompiledFunction(NULL, spec, NULL, effort, exception_style, entry_descriptor);
+    CompiledFunction* cf = new CompiledFunction(md, spec, NULL, effort, exception_style, entry_descriptor);
     setPointersInCodeStorage(&cf->pointers_in_code);
 
     // Make sure that the instruction memory keeps the module object alive.

src/codegen/irgen/hooks.cpp

@@ -682,10 +682,10 @@ void CompiledFunction::speculationFailed() {
 }
 
 std::unordered_set<CompiledFunction*> all_compiled_functions;
-CompiledFunction::CompiledFunction(llvm::Function* func, FunctionSpecialization* spec, void* code, EffortLevel effort,
+CompiledFunction::CompiledFunction(FunctionMetadata* md, FunctionSpecialization* spec, void* code, EffortLevel effort,
                                    ExceptionStyle exception_style, const OSREntryDescriptor* entry_descriptor)
-    : md(NULL),
-      func(func),
+    : md(md),
+      func(NULL),
       effort(effort),
       exception_style(exception_style),
       spec(spec),
@@ -829,6 +829,6 @@ void FunctionMetadata::addVersion(void* f, ConcreteCompilerType* rtn_type,
 #endif
 
     FunctionSpecialization* spec = new FunctionSpecialization(processType(rtn_type), arg_types);
-    addVersion(new CompiledFunction(NULL, spec, f, EffortLevel::MAXIMAL, exception_style, NULL));
+    addVersion(new CompiledFunction(this, spec, f, EffortLevel::MAXIMAL, exception_style, NULL));
 }
 }

src/codegen/irgen/irgenerator.cpp

@@ -62,7 +62,7 @@ IRGenState::IRGenState(FunctionMetadata* md, CompiledFunction* cf, SourceInfo* s
       stmt(NULL),
       scratch_size(0) {
     assert(cf->func);
-    assert(!cf->md); // in this case don't need to pass in sourceinfo
+    assert(cf->md->source.get() == source_info); // I guess this is duplicate now
 }
 
 IRGenState::~IRGenState() {
@@ -2757,43 +2757,45 @@ public:
 
         // For deopts we need to add the compiler created names to the stackmap
         if (ENABLE_FRAME_INTROSPECTION && pp->isDeopt()) {
-            // TODO: don't need to use a sorted symbol table if we're explicitly recording the names!
-            // nice for debugging though.
-            typedef std::pair<InternedString, CompilerVariable*> Entry;
-            std::vector<Entry> sorted_symbol_table;
-            for (auto&& p : symbol_table) {
-                if (p.second)
-                    sorted_symbol_table.push_back(Entry(vregs.getName(p.first), p.second));
-            }
-
-            // TODO: at some point it would be nice to pass these separately
             auto source = irstate->getSourceInfo();
             if (source->is_generator)
-                sorted_symbol_table.push_back(
-                    Entry(source->getInternedStrings().get(PASSED_GENERATOR_NAME),
-                          new ConcreteCompilerVariable(GENERATOR, irstate->getPassedGenerator())));
+                stackmap_args.push_back(irstate->getPassedGenerator());
 
             auto scoping = source->getScopeInfo();
             if (scoping->takesClosure())
-                sorted_symbol_table.push_back(
-                    Entry(source->getInternedStrings().get(PASSED_CLOSURE_NAME),
-                          new ConcreteCompilerVariable(CLOSURE, irstate->getPassedClosure())));
+                stackmap_args.push_back(irstate->getPassedClosure());
+
             if (scoping->createsClosure())
-                sorted_symbol_table.push_back(
-                    Entry(source->getInternedStrings().get(CREATED_CLOSURE_NAME),
-                          new ConcreteCompilerVariable(CLOSURE, irstate->getCreatedClosure())));
+                stackmap_args.push_back(irstate->getCreatedClosure());
+
+            typedef std::pair<InternedString, CompilerVariable*> Entry;
+            for (auto&& p : symbol_table) {
+                int vreg = p.first;
+                if (!p.second)
+                    continue;
 
-            std::sort(sorted_symbol_table.begin(), sorted_symbol_table.end(),
-                      [](const Entry& lhs, const Entry& rhs) { return lhs.first < rhs.first; });
-            for (const auto& p : sorted_symbol_table) {
                 // We never have to include non compiler generated vars because the user visible variables are stored
                 // inside the vregs array.
-                if (!p.first.isCompilerCreatedName())
+                if (vregs.isUserVisibleVReg(vreg)) {
+                    assert(!vregs.getName(p.first).isCompilerCreatedName());
                     continue;
+                }
 
                 CompilerVariable* v = p.second;
                 v->serializeToFrame(stackmap_args);
-                pp->addFrameVar(p.first.s(), v->getType());
+                pp->addFrameVar(p.first, v->getType());
+            }
+
+            for (auto&& p : definedness_vars) {
+                if (!p.second)
+                    continue;
+                if (vregs.isUserVisibleVReg(p.first))
+                    continue;
+
+                assert(symbol_table[p.first]);
+
+                stackmap_args.push_back(p.second);
+                pp->addPotentiallyUndefined(p.first);
             }
         }
 

src/codegen/patchpoints.cpp

@@ -30,10 +30,6 @@
 
 namespace pyston {
 
-void PatchpointInfo::addFrameVar(llvm::StringRef name, CompilerType* type) {
-    frame_vars.push_back(FrameVarInfo({.name = name, .type = type }));
-}
-
 int ICSetupInfo::totalSize() const {
     if (isDeopt())
         return DEOPT_CALL_ONLY_SIZE;
@@ -74,30 +70,46 @@ bool StackMap::Record::Location::operator==(const StackMap::Record::Location& rh
 }
 
 void PatchpointInfo::parseLocationMap(StackMap::Record* r, LocationMap* map) {
+    if (!this->isDeopt())
+        return;
+
     assert(r->locations.size() == totalStackmapArgs());
 
     int cur_arg = frameStackmapArgsStart();
 
     // printf("parsing pp %ld:\n", reinterpret_cast<int64_t>(this));
 
-    for (FrameVarInfo& frame_var : frame_vars) {
-        int num_args = frame_var.type->numFrameArgs();
+    auto parse_type = [&](CompilerType* type) {
+        int num_args = type->numFrameArgs();
 
         llvm::SmallVector<StackMap::Record::Location, 1> locations;
         locations.append(r->locations.data() + cur_arg, r->locations.data() + cur_arg + num_args);
-
-        // printf("%s %d %d\n", frame_var.name.c_str(), r->locations[cur_arg].type, r->locations[cur_arg].regnum);
-
-        map->names[frame_var.name].locations.push_back(
-            LocationMap::LocationTable::LocationEntry({._debug_pp_id = (uint64_t) this,
-                                                       .offset = r->offset,
-                                                       .length = patchpointSize(),
-                                                       .type = frame_var.type,
-                                                       .locations = std::move(locations) }));
-
         cur_arg += num_args;
+
+        return LocationMap::LocationTable::LocationEntry({._debug_pp_id = (uint64_t) this,
+                                                          .offset = r->offset,
+                                                          .length = patchpointSize(),
+                                                          .type = type,
+                                                          .locations = std::move(locations) });
+    };
+
+    auto&& source = parentFunction()->md->source;
+    if (source->is_generator)
+        map->generator.locations.push_back(parse_type(GENERATOR));
+    if (source->getScopeInfo()->takesClosure())
+        map->passed_closure.locations.push_back(parse_type(CLOSURE));
+    if (source->getScopeInfo()->createsClosure())
+        map->created_closure.locations.push_back(parse_type(CLOSURE));
+
+    for (FrameVarInfo& frame_var : frame_info_desc.vars) {
+        map->vars[frame_var.vreg].locations.push_back(parse_type(frame_var.type));
     }
-    assert(cur_arg - frameStackmapArgsStart() == numFrameStackmapArgs());
+    for (int vreg : frame_info_desc.potentially_undefined) {
+        map->definedness_vars[vreg].locations.push_back(parse_type(BOOL));
+    }
+
+    ASSERT(cur_arg - frameStackmapArgsStart() == numFrameStackmapArgs(), "%d %d %d", cur_arg, frameStackmapArgsStart(),
+           numFrameStackmapArgs());
 }
 
 static int extractScratchOffset(PatchpointInfo* pp, StackMap::Record* r) {

src/codegen/patchpoints.h

@@ -96,9 +96,13 @@ public:
 struct PatchpointInfo {
 public:
     struct FrameVarInfo {
-        llvm::StringRef name;
+        int vreg;
         CompilerType* type;
     };
+    struct FrameInfoDesc {
+        std::vector<FrameVarInfo> vars;
+        llvm::SmallVector<int, 2> potentially_undefined;
+    };
 
 private:
     CompiledFunction* const parent_cf;
@@ -106,10 +110,10 @@ private:
     int num_ic_stackmap_args;
     int num_frame_stackmap_args;
     bool is_frame_info_stackmap;
-
-    std::vector<FrameVarInfo> frame_vars;
     unsigned int id;
 
+    FrameInfoDesc frame_info_desc;
+
     PatchpointInfo(CompiledFunction* parent_cf, const ICSetupInfo* icinfo, int num_ic_stackmap_args)
         : parent_cf(parent_cf),
           icinfo(icinfo),
@@ -125,7 +129,7 @@ public:
     int patchpointSize();
     CompiledFunction* parentFunction() { return parent_cf; }
 
-    const std::vector<FrameVarInfo>& getFrameVars() { return frame_vars; }
+    FrameInfoDesc& getFrameDesc() { return frame_info_desc; }
 
     int scratchStackmapArg() { return 0; }
     int scratchSize() { return isDeopt() ? MAX_FRAME_SPILLS * sizeof(void*) : 96; }
@@ -133,7 +137,11 @@ public:
     bool isFrameInfoStackmap() const { return is_frame_info_stackmap; }
     int numFrameSpillsSupported() const { return isDeopt() ? MAX_FRAME_SPILLS : 0; }
 
-    void addFrameVar(llvm::StringRef name, CompilerType* type);
+    void addFrameVar(int vreg, CompilerType* type) {
+        frame_info_desc.vars.push_back(FrameVarInfo({.vreg = vreg, .type = type }));
+    }
+    void addPotentiallyUndefined(int vreg) { frame_info_desc.potentially_undefined.push_back(vreg); }
+
     void setNumFrameArgs(int num_frame_args) {
         assert(num_frame_stackmap_args == -1);
         num_frame_stackmap_args = num_frame_args;

src/codegen/stackmaps.h

@@ -19,6 +19,7 @@
 #include <unordered_map>
 #include <vector>
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 
@@ -101,7 +102,9 @@ public:
         }
     };
 
-    llvm::StringMap<LocationTable> names;
+    llvm::DenseMap<int, LocationTable> vars;
+    LocationTable generator, passed_closure, created_closure;
+    llvm::DenseMap<int, LocationTable> definedness_vars;
 };
 
 StackMap* parseStackMap();

src/codegen/unwinding.cpp

@@ -329,7 +329,9 @@ public:
         }
     }
 
-    llvm::ArrayRef<StackMap::Record::Location> findLocations(llvm::StringRef name) {
+#if 0
+    //llvm::ArrayRef<StackMap::Record::Location> findLocations(llvm::StringRef name) {
+    llvm::ArrayRef<StackMap::Record::Location> findLocations(const LocationMap::LocationTable& table) {
         assert(id.type == PythonFrameId::COMPILED);
 
         CompiledFunction* cf = getCF();
@@ -338,15 +340,14 @@ public:
         assert(ip > cf->code_start);
         unsigned offset = ip - cf->code_start;
 
-        assert(cf->location_map);
-        const LocationMap::LocationTable& table = cf->location_map->names[name];
-
+        //const LocationMap::LocationTable& table = cf->location_map->names[name];
         auto entry = table.findEntry(offset);
         if (!entry)
             return {};
         assert(entry->locations.size());
         return entry->locations;
     }
+#endif
 
     AST_stmt* getCurrentStatement() {
         assert(getFrameInfo()->stmt);
@@ -870,20 +871,37 @@ DeoptState getDeoptState() {
             // We have to detect + ignore any entries for variables that
             // could have been defined (so they have entries) but aren't (so the
             // entries point to uninitialized memory).
-            std::unordered_set<std::string> is_undefined;
+            std::unordered_set<int> is_undefined;
 
-            for (const auto& p : cf->location_map->names) {
-                if (!startswith(p.first(), "!is_defined_"))
-                    continue;
+            auto readEntry = [&](const LocationMap::LocationTable::LocationEntry* e) {
+                auto locs = e->locations;
+
+                llvm::SmallVector<uint64_t, 1> vals;
+                // printf("%s: %s\n", p.first().c_str(), e.type->debugName().c_str());
 
+                for (auto& loc : locs) {
+                    vals.push_back(frame_iter->readLocation(loc));
+                }
+
+                // this returns an owned reference so we don't incref it
+                Box* v = e->type->deserializeFromFrame(vals);
+                return v;
+            };
+
+            if (auto e = cf->location_map->generator.findEntry(offset))
+                d->d[boxString(PASSED_GENERATOR_NAME)] = readEntry(e);
+            if (auto e = cf->location_map->passed_closure.findEntry(offset))
+                d->d[boxString(PASSED_CLOSURE_NAME)] = readEntry(e);
+            if (auto e = cf->location_map->created_closure.findEntry(offset))
+                d->d[boxString(CREATED_CLOSURE_NAME)] = readEntry(e);
+
+            for (const auto& p : cf->location_map->definedness_vars) {
                 auto e = p.second.findEntry(offset);
                 if (e) {
-                    auto locs = e->locations;
-
-                    assert(locs.size() == 1);
-                    uint64_t v = frame_iter->readLocation(locs[0]);
-                    if ((v & 1) == 0)
-                        is_undefined.insert(p.first().substr(12));
+                    Box* b = readEntry(e);
+                    AUTO_DECREF(b);
+                    if (b == Py_False)
+                        is_undefined.insert(p.first);
                 }
             }
 
@@ -893,40 +911,23 @@ DeoptState getDeoptState() {
             // But deopts are so rare it's not really worth it.
             Box** vregs = frame_iter->getFrameInfo()->vregs;
             for (const auto& p : cf->md->source->cfg->getVRegInfo().getUserVisibleSymVRegMap()) {
-                if (is_undefined.count(p.first.s()))
-                    continue;
+                if (is_undefined.count(p.second))
+                    assert(0);
 
                 Box* v = vregs[p.second];
                 if (!v)
                     continue;
 
-                assert(!d->d.count(p.first.getBox()));
-                d->d[incref(p.first.getBox())] = incref(v);
+                d->d[boxInt(p.second)] = incref(v);
             }
 
-            for (const auto& p : cf->location_map->names) {
-                if (p.first()[0] == '!')
-                    continue;
-
-                if (is_undefined.count(p.first()))
+            for (const auto& p : cf->location_map->vars) {
+                if (is_undefined.count(p.first))
                     continue;
 
                 auto e = p.second.findEntry(offset);
-                if (e) {
-                    auto locs = e->locations;
-
-                    llvm::SmallVector<uint64_t, 1> vals;
-                    // printf("%s: %s\n", p.first().c_str(), e.type->debugName().c_str());
-
-                    for (auto& loc : locs) {
-                        vals.push_back(frame_iter->readLocation(loc));
-                    }
-
-                    // this returns an owned reference so we don't incref it
-                    Box* v = e->type->deserializeFromFrame(vals);
-                    // printf("%s: (pp id %ld) %p\n", p.first().c_str(), e._debug_pp_id, v);
-                    d->d[boxString(p.first())] = v;
-                }
+                if (e)
+                    d->d[boxInt(p.first)] = readEntry(e);
             }
         } else {
             abort();

src/core/types.h

@@ -379,7 +379,7 @@ public:
     // List of metadata objects for ICs inside this compilation
     std::vector<ICInfo*> ics;
 
-    CompiledFunction(llvm::Function* func, FunctionSpecialization* spec, void* code, EffortLevel effort,
+    CompiledFunction(FunctionMetadata* func, FunctionSpecialization* spec, void* code, EffortLevel effort,
                      ExceptionStyle exception_style, const OSREntryDescriptor* entry_descriptor);
 
     ConcreteCompilerType* getReturnType();

src/runtime/objmodel.cpp

@@ -137,6 +137,8 @@ extern "C" void xdecrefAndRethrow(void* cxa_ptr, int num, ...) {
 }
 
 extern "C" Box* deopt(AST_expr* expr, Box* value) {
+    ASSERT(ENABLE_FRAME_INTROSPECTION, "deopt will not work with frame introspection turned off");
+
     STAT_TIMER(t0, "us_timer_deopt", 10);
 
     static StatCounter num_deopt("num_deopt");