Convert OSR to use vregs

The benefit here is indirect, of letting us get rid of some
of the compatibility shims for moving between vregs and names.

src/analysis/function_analysis.cpp

@@ -462,13 +462,21 @@ PhiAnalysis::PhiAnalysis(VRegMap<DefinednessAnalysis::DefinitionLevel> initial_m
                 const VRegSet& defined = definedness.getDefinedVregsAtEnd(pred);
                 for (int vreg : defined) {
                     if (!required[vreg] && liveness->isLiveAtEnd(vreg, pred)) {
-                        // printf("%d-%d %s\n", pred->idx, block->idx, s.c_str());
+                        // printf("%d-%d %s\n", pred->idx, block->idx, vreg_info.getName(vreg).c_str());
 
                         required.set(vreg);
                     }
                 }
             }
         }
+
+        if (VERBOSITY() >= 3) {
+            printf("Phis required at end of %d:", block->idx);
+            for (auto vreg : required) {
+                printf(" %s", vreg_info.getName(vreg).c_str());
+            }
+            printf("\n");
+        }
     }
 
     static StatCounter us_phis("us_compiling_analysis_phis");
@@ -573,22 +581,13 @@ std::unique_ptr<PhiAnalysis> computeRequiredPhis(const OSREntryDescriptor* entry
         initial_map[vreg] = DefinednessAnalysis::Undefined;
     }
 
-    llvm::StringSet<> potentially_undefined;
     for (const auto& p : entry_descriptor->args) {
-        if (!startswith(p.first.s(), "!is_defined_"))
+        if (!p.second)
             continue;
-        potentially_undefined.insert(p.first.s().substr(12));
-    }
 
-    for (const auto& p : entry_descriptor->args) {
-        if (p.first.s()[0] == '!')
-            continue;
-        if (p.first.s() == PASSED_CLOSURE_NAME || p.first.s() == FRAME_INFO_PTR_NAME
-            || p.first.s() == PASSED_GENERATOR_NAME || p.first.s() == CREATED_CLOSURE_NAME)
-            assert(0);
-        int vreg = cfg->getVRegInfo().getVReg(p.first);
-        ASSERT(initial_map[vreg] == DefinednessAnalysis::Undefined, "%s %d", p.first.c_str(), initial_map[vreg]);
-        if (potentially_undefined.count(p.first.s()))
+        int vreg = p.first;
+        ASSERT(initial_map[vreg] == DefinednessAnalysis::Undefined, "%d %d", vreg, initial_map[vreg]);
+        if (entry_descriptor->potentially_undefined[vreg])
             initial_map[vreg] = DefinednessAnalysis::PotentiallyDefined;
         else
             initial_map[vreg] = DefinednessAnalysis::Defined;

src/analysis/function_analysis.h

@@ -20,6 +20,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 
+#include "core/cfg.h"
 #include "core/stringpool.h"
 #include "core/types.h"
 
@@ -33,68 +34,6 @@ class CFGBlock;
 class ScopeInfo;
 class LivenessBBVisitor;
 
-template <typename T> class VRegMap {
-private:
-    // TODO: switch just to a T*
-    std::vector<T> v;
-
-public:
-    VRegMap(int num_vregs) : v(num_vregs) {}
-
-    T& operator[](int vreg) {
-        assert(vreg >= 0 && vreg < v.size());
-        return v[vreg];
-    }
-
-    const T& operator[](int vreg) const {
-        assert(vreg >= 0 && vreg < v.size());
-        return v[vreg];
-    }
-
-    void clear() {
-        int n = v.size();
-        for (int i = 0; i < n; i++) {
-            v[i] = T();
-        }
-    }
-
-    int numSet() {
-        int n = v.size();
-        int r = 0;
-        for (int i = 0; i < n; i++) {
-            if (v[i] != T())
-                r++;
-        }
-        return r;
-    }
-
-    class iterator {
-    public:
-        const VRegMap<T>& map;
-        int i;
-        iterator(const VRegMap<T>& map, int i) : map(map), i(i) {}
-
-        // TODO: make this skip unset values?
-        iterator& operator++() {
-            i++;
-            return *this;
-        }
-
-        bool operator==(const iterator& rhs) const { return i == rhs.i; }
-        bool operator!=(const iterator& rhs) const { return !(*this == rhs); }
-
-        std::pair<int, const T&> operator*() { return std::pair<int, const T&>(i, map[i]); }
-        int first() const { return i; }
-        const T& second() const { return map[i]; }
-    };
-
-    int numVregs() const { return v.size(); }
-
-    iterator begin() const { return iterator(*this, 0); }
-
-    iterator end() const { return iterator(*this, this->v.size()); }
-};
-
 class LivenessAnalysis {
 private:
     CFG* cfg;
@@ -117,62 +56,6 @@ public:
 
 class PhiAnalysis;
 
-class VRegSet {
-private:
-    // TODO: switch just to a bool*
-    std::vector<bool> v;
-
-public:
-    VRegSet(int num_vregs) : v(num_vregs, false) {}
-
-    // TODO: what is the referenc type here?
-    bool operator[](int vreg) {
-        assert(vreg >= 0 && vreg < v.size());
-        return v[vreg];
-    }
-    void set(int vreg) {
-        assert(vreg >= 0 && vreg < v.size());
-        v[vreg] = true;
-    }
-
-    int numSet() const {
-        int r = 0;
-        for (auto b : v)
-            if (b)
-                r++;
-        return r;
-    }
-
-    class iterator {
-    public:
-        const VRegSet& set;
-        int i;
-        iterator(const VRegSet& set, int i) : set(set), i(i) {}
-
-        iterator& operator++() {
-            do {
-                i++;
-            } while (i < set.v.size() && !set.v[i]);
-            return *this;
-        }
-
-        bool operator==(const iterator& rhs) const { return i == rhs.i; }
-        bool operator!=(const iterator& rhs) const { return !(*this == rhs); }
-
-        int operator*() { return i; }
-    };
-
-    iterator begin() const {
-        for (int i = 0; i < v.size(); i++) {
-            if (v[i])
-                return iterator(*this, i);
-        }
-        return iterator(*this, this->v.size());
-    }
-
-    iterator end() const { return iterator(*this, this->v.size()); }
-};
-
 class DefinednessAnalysis {
 public:
     enum DefinitionLevel {

src/analysis/type_analysis.cpp

@@ -158,6 +158,7 @@ private:
 
         if (VERBOSITY() >= 3) {
             printf("Type of ");
+            fflush(stdout);
             print_ast(node);
             printf(" is %s\n", rtn->debugName().c_str());
         }
@@ -790,8 +791,9 @@ public:
                 printf("before:\n");
                 TypeMap& starting = starting_types.find(block)->second;
                 for (const auto& p : starting) {
+                    if (!p.second)
+                        continue;
                     auto name = vreg_info.getName(p.first);
-                    ASSERT(p.second, "%s", name.c_str());
                     printf("%s: %s\n", name.c_str(), p.second->debugName().c_str());
                 }
             }
@@ -803,14 +805,16 @@ public:
                 printf("before (after):\n");
                 TypeMap& starting = starting_types.find(block)->second;
                 for (const auto& p : starting) {
+                    if (!p.second)
+                        continue;
                     auto name = vreg_info.getName(p.first);
-                    ASSERT(p.second, "%s", name.c_str());
                     printf("%s: %s\n", name.c_str(), p.second->debugName().c_str());
                 }
                 printf("after:\n");
                 for (const auto& p : ending) {
+                    if (!p.second)
+                        continue;
                     auto name = vreg_info.getName(p.first);
-                    ASSERT(p.second, "%s", name.c_str());
                     printf("%s: %s\n", name.c_str(), p.second->debugName().c_str());
                 }
             }
@@ -839,8 +843,9 @@ public:
 
                 const TypeMap& starting = p.second;
                 for (const auto& p : starting) {
+                    if (!p.second)
+                        continue;
                     auto name = vreg_info.getName(p.first);
-                    ASSERT(p.second, "%s", name.c_str());
                     printf("%s: %s\n", name.c_str(), p.second->debugName().c_str());
                 }
             }
@@ -901,12 +906,7 @@ TypeAnalysis* doTypeAnalysis(const OSREntryDescriptor* entry_descriptor, EffortL
     TypeMap initial_types(vreg_info.getTotalNumOfVRegs());
 
     for (auto&& p : entry_descriptor->args) {
-        if (p.first.s()[0] == '!')
-            continue;
-        if (p.first.s() == PASSED_CLOSURE_NAME || p.first.s() == FRAME_INFO_PTR_NAME
-            || p.first.s() == PASSED_GENERATOR_NAME || p.first.s() == CREATED_CLOSURE_NAME)
-            assert(0);
-        initial_types[vreg_info.getVReg(p.first)] = p.second;
+        initial_types[p.first] = p.second;
     }
 
     return PropagatingTypeAnalysis::doAnalysis(speculation, scope_info, std::move(initial_types),

src/codegen/ast_interpreter.cpp

@@ -739,9 +739,12 @@ Box* ASTInterpreter::doOSR(AST_Jump* node) {
         found_entry = p.first;
     }
 
-    std::map<InternedString, Box*> sorted_symbol_table;
+    int num_vregs = source_info->cfg->getVRegInfo().getTotalNumOfVRegs();
+    VRegMap<Box*> sorted_symbol_table(num_vregs);
+    VRegSet potentially_undefined(num_vregs);
 
     // TODO: maybe use a different placeholder (=NULL)?
+    // - new issue with that -- we can no longer distinguish NULL from unset-in-sorted_symbol_table
     // Currently we pass None because the LLVM jit will decref this value even though it may not be set.
     static Box* const VAL_UNDEFINED = (Box*)None;
 
@@ -751,16 +754,14 @@ Box* ASTInterpreter::doOSR(AST_Jump* node) {
         if (!liveness->isLiveAtEnd(vreg, current_block))
             continue;
 
-        InternedString name = source_info->cfg->getVRegInfo().getName(vreg);
         Box* val = vregs[vreg];
         if (phis->isPotentiallyUndefinedAfter(vreg, current_block)) {
+            potentially_undefined.set(vreg);
             bool is_defined = val != NULL;
-            // TODO only mangle once
-            sorted_symbol_table[getIsDefinedName(name, source_info->getInternedStrings())] = (Box*)is_defined;
-            sorted_symbol_table[name] = is_defined ? incref(val) : incref(VAL_UNDEFINED);
+            sorted_symbol_table[vreg] = is_defined ? incref(val) : incref(VAL_UNDEFINED);
         } else {
-            ASSERT(val != NULL, "%s", name.c_str());
-            sorted_symbol_table[name] = incref(val);
+            assert(val != NULL);
+            sorted_symbol_table[vreg] = incref(val);
         }
     }
 
@@ -769,7 +770,7 @@ Box* ASTInterpreter::doOSR(AST_Jump* node) {
 
     // LLVM has a limit on the number of operands a machine instruction can have (~255),
     // in order to not hit the limit with the patchpoints cancel OSR when we have a high number of symbols.
-    if (sorted_symbol_table.size() > 225) {
+    if (sorted_symbol_table.numSet() > 225) {
         static StatCounter times_osr_cancel("num_osr_cancel_too_many_syms");
         times_osr_cancel.log();
         return nullptr;
@@ -778,24 +779,28 @@ Box* ASTInterpreter::doOSR(AST_Jump* node) {
     if (found_entry == nullptr) {
         OSREntryDescriptor* entry = OSREntryDescriptor::create(getMD(), node, CXX);
 
-        for (auto& it : sorted_symbol_table) {
-            if (isIsDefinedName(it.first))
-                entry->args[it.first] = BOOL;
-            else {
-                assert(it.first.s()[0] != '!');
-                entry->args[it.first] = UNKNOWN;
-            }
+        // TODO can we just get rid of this?
+        for (auto&& p : sorted_symbol_table) {
+            if (p.second)
+                entry->args[p.first] = UNKNOWN;
         }
 
+        entry->potentially_undefined = potentially_undefined;
+
         found_entry = entry;
     }
 
     OSRExit exit(found_entry);
 
     std::vector<Box*> arg_array;
-    arg_array.reserve(sorted_symbol_table.size());
-    for (auto& it : sorted_symbol_table) {
-        arg_array.push_back(it.second);
+    arg_array.reserve(sorted_symbol_table.numSet() + potentially_undefined.numSet());
+    for (auto&& p : sorted_symbol_table) {
+        if (p.second)
+            arg_array.push_back(p.second);
+    }
+    for (int vreg : potentially_undefined) {
+        bool is_defined = sorted_symbol_table[vreg] != VAL_UNDEFINED;
+        arg_array.push_back((Box*)is_defined);
     }
 
     UNAVOIDABLE_STAT_TIMER(t0, "us_timer_in_jitted_code");

src/codegen/irgen/hooks.cpp

@@ -261,7 +261,9 @@ CompiledFunction* compileFunction(FunctionMetadata* f, FunctionSpecialization* s
 
         if (entry_descriptor && VERBOSITY("irgen") >= 2) {
             for (const auto& p : entry_descriptor->args) {
-                ss << p.first.s() << ": " << p.second->debugName() << '\n';
+                if (!p.second)
+                    continue;
+                ss << p.first << ": " << p.second->debugName() << '\n';
             }
         }
 

src/codegen/irgen/irgenerator.h

@@ -21,7 +21,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/IR/Instructions.h"
 
-#include "analysis/function_analysis.h"
+#include "core/cfg.h"
 #include "core/stringpool.h"
 #include "core/types.h"
 

src/codegen/osrentry.h

@@ -18,6 +18,7 @@
 #include <map>
 #include <vector>
 
+#include "core/cfg.h"
 #include "core/stringpool.h"
 
 namespace llvm {
@@ -32,7 +33,11 @@ struct StackMap;
 class OSREntryDescriptor {
 private:
     OSREntryDescriptor(FunctionMetadata* md, AST_Jump* backedge, ExceptionStyle exception_style)
-        : md(md), backedge(backedge), exception_style(exception_style) {
+        : md(md),
+          backedge(backedge),
+          exception_style(exception_style),
+          args(md->source->cfg->getVRegInfo().getTotalNumOfVRegs()),
+          potentially_undefined(md->source->cfg->getVRegInfo().getTotalNumOfVRegs()) {
         assert(md);
     }
 
@@ -40,8 +45,9 @@ public:
     FunctionMetadata* md;
     AST_Jump* const backedge;
     ExceptionStyle exception_style;
-    typedef std::map<InternedString, ConcreteCompilerType*> ArgMap;
+    typedef VRegMap<ConcreteCompilerType*> ArgMap;
     ArgMap args;
+    VRegSet potentially_undefined;
 
     static OSREntryDescriptor* create(FunctionMetadata* md, AST_Jump* backedge, ExceptionStyle exception_style) {
         return new OSREntryDescriptor(md, backedge, exception_style);

src/core/cfg.h

@@ -191,6 +191,124 @@ public:
     void print(llvm::raw_ostream& stream = llvm::outs());
 };
 
+class VRegSet {
+private:
+    // TODO: switch just to a bool*
+    std::vector<bool> v;
+
+public:
+    VRegSet(int num_vregs) : v(num_vregs, false) {}
+
+    // TODO: what is the referenc type here?
+    bool operator[](int vreg) const {
+        assert(vreg >= 0 && vreg < v.size());
+        return v[vreg];
+    }
+    void set(int vreg) {
+        assert(vreg >= 0 && vreg < v.size());
+        v[vreg] = true;
+    }
+
+    int numSet() const {
+        int r = 0;
+        for (auto b : v)
+            if (b)
+                r++;
+        return r;
+    }
+
+    class iterator {
+    public:
+        const VRegSet& set;
+        int i;
+        iterator(const VRegSet& set, int i) : set(set), i(i) {}
+
+        iterator& operator++() {
+            do {
+                i++;
+            } while (i < set.v.size() && !set.v[i]);
+            return *this;
+        }
+
+        bool operator==(const iterator& rhs) const { return i == rhs.i; }
+        bool operator!=(const iterator& rhs) const { return !(*this == rhs); }
+
+        int operator*() { return i; }
+    };
+
+    iterator begin() const {
+        for (int i = 0; i < v.size(); i++) {
+            if (v[i])
+                return iterator(*this, i);
+        }
+        return iterator(*this, this->v.size());
+    }
+
+    iterator end() const { return iterator(*this, this->v.size()); }
+};
+
+template <typename T> class VRegMap {
+private:
+    // TODO: switch just to a T*
+    std::vector<T> v;
+
+public:
+    VRegMap(int num_vregs) : v(num_vregs) {}
+
+    T& operator[](int vreg) {
+        assert(vreg >= 0 && vreg < v.size());
+        return v[vreg];
+    }
+
+    const T& operator[](int vreg) const {
+        assert(vreg >= 0 && vreg < v.size());
+        return v[vreg];
+    }
+
+    void clear() {
+        int n = v.size();
+        for (int i = 0; i < n; i++) {
+            v[i] = T();
+        }
+    }
+
+    int numSet() {
+        int n = v.size();
+        int r = 0;
+        for (int i = 0; i < n; i++) {
+            if (v[i] != T())
+                r++;
+        }
+        return r;
+    }
+
+    class iterator {
+    public:
+        const VRegMap<T>& map;
+        int i;
+        iterator(const VRegMap<T>& map, int i) : map(map), i(i) {}
+
+        // TODO: make this skip unset values?
+        iterator& operator++() {
+            i++;
+            return *this;
+        }
+
+        bool operator==(const iterator& rhs) const { return i == rhs.i; }
+        bool operator!=(const iterator& rhs) const { return !(*this == rhs); }
+
+        std::pair<int, const T&> operator*() { return std::pair<int, const T&>(i, map[i]); }
+        int first() const { return i; }
+        const T& second() const { return map[i]; }
+    };
+
+    int numVregs() const { return v.size(); }
+
+    iterator begin() const { return iterator(*this, 0); }
+
+    iterator end() const { return iterator(*this, this->v.size()); }
+};
+
 class SourceInfo;
 CFG* computeCFG(SourceInfo* source, std::vector<AST_stmt*> body, const ParamNames& param_names);
 void printCFG(CFG* cfg);

test/tests/undefined_passing.py

 # I think we have some other similar tests to this, but it's hard to find them.
 
+# Reopt:
 def f(x):
     if x:
         y = 1
     if '':
         pass
-    print y
+    y
 
 for i in xrange(10000):
     f(1)
@@ -15,3 +16,23 @@ try:
 except UnboundLocalError:
     pass
 
+
+# OSR:
+s = """
+def f(x):
+    if x:
+        y = 1
+    for i in xrange(10000):
+        pass
+    print y
+"""
+
+exec s
+f(1)
+
+exec s
+try:
+    f(0)
+    assert 0
+except UnboundLocalError:
+    pass