AST interpreter cleanup

- Move internal definitions from header to source file
- Fix root_stack_set bug carried over from llvm_interpreter
- Cut off access to the old llvm interpreter
- Slightly tune the tiering thresholds
 - They seem too high but our LLVM compilation is overly expensive right now
 - Our AST interpreter is also somewhat slow due to its string handling

docs/PROFILING

@@ -24,3 +24,14 @@ the JIT'd code.
 
 There's a gprof-based profile, but that doesn't support any JIT'd code.  It can be quite handy for profiling the Pyston
 codegen + LLVM cost.
+
+
+We also have a few internal timers that log important sections of code.  You can see their totals by passing '-s' to
+the executable (ARGS=-s to a Make recipe).
+Some common ones are:
+- us_compiling: total amount of time (in microseconds) spent in 'codegen', from
+        the time we decide we want to run some Python code to the point that we can
+        start executing it
+  - us_compiling_irgen: total time creating the LLVM IR.  includes some analysis time
+  - us_compiling_jitting: subset of us_compiling which is spent in LLVM
+        compilation (time from giving LLVM its IR to when we get machine code back)

src/analysis/scoping_analysis.cpp

@@ -70,8 +70,8 @@ public:
         // assert(name[0] != '#' && "should test this");
         return true;
     }
-    bool refersToClosure(const std::string name) override { return false; }
-    bool saveInClosure(const std::string name) override { return false; }
+    bool refersToClosure(const std::string& name) override { return false; }
+    bool saveInClosure(const std::string& name) override { return false; }
 
     const std::unordered_set<std::string>& getClassDefLocalNames() override { RELEASE_ASSERT(0, ""); }
 };
@@ -141,13 +141,13 @@ public:
             return true;
         return usage->written.count(name) == 0 && usage->got_from_closure.count(name) == 0;
     }
-    bool refersToClosure(const std::string name) override {
+    bool refersToClosure(const std::string& name) override {
         // HAX
         if (isCompilerCreatedName(name))
             return false;
         return usage->got_from_closure.count(name) != 0;
     }
-    bool saveInClosure(const std::string name) override {
+    bool saveInClosure(const std::string& name) override {
         // HAX
         if (isCompilerCreatedName(name))
             return false;

src/analysis/scoping_analysis.h

@@ -36,8 +36,8 @@ public:
     virtual void setTakesGenerator(bool b = true) { isGeneratorValue = b; }
 
     virtual bool refersToGlobal(const std::string& name) = 0;
-    virtual bool refersToClosure(const std::string name) = 0;
-    virtual bool saveInClosure(const std::string name) = 0;
+    virtual bool refersToClosure(const std::string& name) = 0;
+    virtual bool saveInClosure(const std::string& name) = 0;
 
     // Get the names set within a classdef that should be forwarded on to
     // the metaclass constructor.

src/codegen/ast_interpreter.h

@@ -15,12 +15,6 @@
 #ifndef PYSTON_CODEGEN_ASTINTERPRETER_H
 #define PYSTON_CODEGEN_ASTINTERPRETER_H
 
-#include <llvm/ADT/StringMap.h>
-
-#include "codegen/compvars.h"
-#include "core/ast.h"
-#include "runtime/objmodel.h"
-
 namespace pyston {
 
 namespace gc {
@@ -28,7 +22,9 @@ class GCVisitor;
 }
 
 class Box;
-class CLFunction;
+class BoxedDict;
+class BoxedModule;
+struct CompiledFunction;
 struct LineInfo;
 
 Box* astInterpretFunction(CompiledFunction* f, int nargs, Box* closure, Box* generator, Box* arg1, Box* arg2, Box* arg3,
@@ -37,90 +33,8 @@ Box* astInterpretFunction(CompiledFunction* f, int nargs, Box* closure, Box* gen
 const LineInfo* getLineInfoForInterpretedFrame(void* frame_ptr);
 BoxedModule* getModuleForInterpretedFrame(void* frame_ptr);
 
-union Value {
-    bool b;
-    int64_t n;
-    double d;
-    Box* o;
-
-    Value(bool b) : b(b) {}
-    Value(int64_t n = 0) : n(n) {}
-    Value(double d) : d(d) {}
-    Value(Box* o) : o(o) {}
-};
-
-class ASTInterpreter {
-public:
-    typedef llvm::StringMap<Box*> SymMap;
-
-    ASTInterpreter(CompiledFunction* compiled_function);
-    ~ASTInterpreter();
-
-    void initArguments(int nargs, BoxedClosure* closure, BoxedGenerator* generator, Box* arg1, Box* arg2, Box* arg3,
-                       Box** args);
-    Value execute(CFGBlock* block = 0);
-
-private:
-    Box* createFunction(AST* node, AST_arguments* args, const std::vector<AST_stmt*>& body);
-    Value doBinOp(Box* left, Box* right, int op, BinExpType exp_type);
-    void doStore(AST_expr* node, Value value);
-    void doStore(const std::string& name, Value value);
-    void eraseDeadSymbols();
-
-    Value visit_assert(AST_Assert* node);
-    Value visit_assign(AST_Assign* node);
-    Value visit_binop(AST_BinOp* node);
-    Value visit_call(AST_Call* node);
-    Value visit_classDef(AST_ClassDef* node);
-    Value visit_compare(AST_Compare* node);
-    Value visit_delete(AST_Delete* node);
-    Value visit_functionDef(AST_FunctionDef* node);
-    Value visit_global(AST_Global* node);
-    Value visit_module(AST_Module* node);
-    Value visit_print(AST_Print* node);
-    Value visit_raise(AST_Raise* node);
-    Value visit_return(AST_Return* node);
-    Value visit_stmt(AST_stmt* node);
-    Value visit_unaryop(AST_UnaryOp* node);
-
-    Value visit_attribute(AST_Attribute* node);
-    Value visit_dict(AST_Dict* node);
-    Value visit_expr(AST_expr* node);
-    Value visit_expr(AST_Expr* node);
-    Value visit_index(AST_Index* node);
-    Value visit_lambda(AST_Lambda* node);
-    Value visit_list(AST_List* node);
-    Value visit_name(AST_Name* node);
-    Value visit_num(AST_Num* node);
-    Value visit_repr(AST_Repr* node);
-    Value visit_set(AST_Set* node);
-    Value visit_slice(AST_Slice* node);
-    Value visit_str(AST_Str* node);
-    Value visit_subscript(AST_Subscript* node);
-    Value visit_tuple(AST_Tuple* node);
-    Value visit_yield(AST_Yield* node);
-
-
-    // pseudo
-    Value visit_augBinOp(AST_AugBinOp* node);
-    Value visit_branch(AST_Branch* node);
-    Value visit_clsAttribute(AST_ClsAttribute* node);
-    Value visit_invoke(AST_Invoke* node);
-    Value visit_jump(AST_Jump* node);
-    Value visit_langPrimitive(AST_LangPrimitive* node);
-
-public:
-    SourceInfo* source_info;
-    SymMap sym_table;
-    CFGBlock* next_block, *current_block;
-    AST* current_inst;
-    Box* last_exception;
-    BoxedClosure* passed_closure, *created_closure;
-    BoxedGenerator* generator;
-    ScopeInfo* scope_info;
-    CompiledFunction* compiled_func;
-    unsigned edgecount;
-};
+void gatherInterpreterRoots(gc::GCVisitor* visitor);
+BoxedDict* localsForInterpretedFrame(void* frame_ptr, bool only_user_visible);
 }
 
 #endif

src/codegen/irgen.cpp

@@ -279,7 +279,7 @@ computeBlockTraversalOrder(const BlockSet& full_blocks, const BlockSet& partial_
 }
 
 static ConcreteCompilerType* getTypeAtBlockStart(TypeAnalysis* types, const std::string& name, CFGBlock* block) {
-    if (startswith(name, "!is_defined"))
+    if (isIsDefinedName(name))
         return BOOL;
     else if (name == PASSED_GENERATOR_NAME)
         return GENERATOR;
@@ -410,7 +410,7 @@ static void emitBBs(IRGenState* irstate, const char* bb_type, GuardList& out_gua
 
                 assert(p.first[0] != '!');
 
-                std::string is_defined_name = "!is_defined_" + p.first;
+                std::string is_defined_name = getIsDefinedName(p.first);
                 llvm::BasicBlock* defined_join = nullptr, * defined_prev = nullptr, * defined_check = nullptr;
                 if (entry_descriptor->args.count(is_defined_name)) {
                     // relying on the fact that we are iterating over the names in order
@@ -658,7 +658,7 @@ static void emitBBs(IRGenState* irstate, const char* bb_type, GuardList& out_gua
             for (const auto& s : source->phis->getAllRequiredFor(block)) {
                 names.insert(s);
                 if (source->phis->isPotentiallyUndefinedAfter(s, block->predecessors[0])) {
-                    names.insert("!is_defined_" + s);
+                    names.insert(getIsDefinedName(s));
                 }
             }
 
@@ -840,7 +840,7 @@ static void emitBBs(IRGenState* irstate, const char* bb_type, GuardList& out_gua
                 llvm_phi->addIncoming(v->getValue(), osr_unbox_block);
             }
 
-            std::string is_defined_name = "!is_defined_" + it->first;
+            std::string is_defined_name = getIsDefinedName(it->first);
 
             for (int i = 0; i < block_guards.size(); i++) {
                 GuardList::BlockEntryGuard* guard = block_guards[i];

src/codegen/irgen.h

@@ -79,6 +79,13 @@ public:
                                   ExcInfo exc_info) = 0;
 };
 
+extern const std::string CREATED_CLOSURE_NAME;
+extern const std::string PASSED_CLOSURE_NAME;
+extern const std::string PASSED_GENERATOR_NAME;
+
+std::string getIsDefinedName(const std::string& name);
+bool isIsDefinedName(const std::string& name);
+
 CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_descriptor,
                             EffortLevel::EffortLevel effort, FunctionSpecialization* spec, std::string nameprefix);
 

src/codegen/irgen/irgenerator.cpp

@@ -256,6 +256,14 @@ const std::string CREATED_CLOSURE_NAME = "!created_closure";
 const std::string PASSED_CLOSURE_NAME = "!passed_closure";
 const std::string PASSED_GENERATOR_NAME = "!passed_generator";
 
+std::string getIsDefinedName(const std::string& name) {
+    return "!is_defined_" + name;
+}
+
+bool isIsDefinedName(const std::string& name) {
+    return startswith(name, "!is_defined_");
+}
+
 class IRGeneratorImpl : public IRGenerator {
 private:
     IRGenState* irstate;
@@ -411,8 +419,8 @@ private:
                     if (p.first[0] == '!' || p.first[0] == '#')
                         continue;
 
-                    ConcreteCompilerVariable* is_defined_var = static_cast<ConcreteCompilerVariable*>(
-                        _getFake(_getFakeName("is_defined", p.first.c_str()), true));
+                    ConcreteCompilerVariable* is_defined_var
+                        = static_cast<ConcreteCompilerVariable*>(_getFake(getIsDefinedName(p.first), true));
 
                     static const std::string setitem_str("__setitem__");
                     if (!is_defined_var) {
@@ -822,7 +830,7 @@ private:
                 return undefVariable();
             }
 
-            std::string defined_name = _getFakeName("is_defined", node->id.c_str());
+            std::string defined_name = getIsDefinedName(node->id);
             ConcreteCompilerVariable* is_defined_var
                 = static_cast<ConcreteCompilerVariable*>(_getFake(defined_name, true));
 
@@ -1272,12 +1280,6 @@ private:
         return rtn;
     }
 
-    static std::string _getFakeName(const char* prefix, const char* token) {
-        char buf[40];
-        snprintf(buf, 40, "!%s_%s", prefix, token);
-        return std::string(buf);
-    }
-
     void _setFake(std::string name, CompilerVariable* val) {
         assert(name[0] == '!');
         CompilerVariable*& cur = symbol_table[name];
@@ -1326,8 +1328,8 @@ private:
             val->incvref();
 
             // Clear out the is_defined name since it is now definitely defined:
-            assert(!startswith(name, "!is_defined"));
-            std::string defined_name = _getFakeName("is_defined", name.c_str());
+            assert(!isIsDefinedName(name));
+            std::string defined_name = getIsDefinedName(name);
             _popFake(defined_name, true);
 
             if (scope_info->saveInClosure(name)) {
@@ -1614,7 +1616,7 @@ private:
             return;
         }
 
-        std::string defined_name = _getFakeName("is_defined", target->id.c_str());
+        std::string defined_name = getIsDefinedName(target->id);
         ConcreteCompilerVariable* is_defined_var = static_cast<ConcreteCompilerVariable*>(_getFake(defined_name, true));
 
         if (is_defined_var) {
@@ -2141,7 +2143,7 @@ private:
     }
 
     bool allowableFakeEndingSymbol(const std::string& name) {
-        return startswith(name, "!is_defined") || name == PASSED_CLOSURE_NAME || name == CREATED_CLOSURE_NAME
+        return isIsDefinedName(name) || name == PASSED_CLOSURE_NAME || name == CREATED_CLOSURE_NAME
                || name == PASSED_GENERATOR_NAME;
     }
 
@@ -2159,7 +2161,7 @@ private:
                 continue;
             }
 
-            // ASSERT(it->first[0] != '!' || startswith(it->first, "!is_defined"), "left a fake variable in the real
+            // ASSERT(it->first[0] != '!' || isIsDefinedName(it->first), "left a fake variable in the real
             // symbol table? '%s'", it->first.c_str());
 
             if (!source->liveness->isLiveAtEnd(it->first, myblock)) {
@@ -2200,7 +2202,7 @@ private:
             assert(!scope_info->refersToGlobal(*it));
             CompilerVariable*& cur = symbol_table[*it];
 
-            std::string defined_name = _getFakeName("is_defined", it->c_str());
+            std::string defined_name = getIsDefinedName(*it);
 
             if (cur != NULL) {
                 // printf("defined on this path; ");
@@ -2291,7 +2293,7 @@ public:
                 assert(it->second->getVrefs() == 1);
                 // this conversion should have already happened... should refactor this.
                 ConcreteCompilerType* ending_type;
-                if (startswith(it->first, "!is_defined")) {
+                if (isIsDefinedName(it->first)) {
                     assert(it->second->getType() == BOOL);
                     ending_type = BOOL;
                 } else if (it->first == PASSED_CLOSURE_NAME) {

src/codegen/llvm_interpreter.h

@@ -29,12 +29,14 @@ class Box;
 class BoxedDict;
 struct LineInfo;
 
+#if 0
 Box* interpretFunction(llvm::Function* f, int nargs, Box* closure, Box* generator, Box* arg1, Box* arg2, Box* arg3,
                        Box** args);
 
 void gatherInterpreterRoots(gc::GCVisitor* visitor);
 const LineInfo* getLineInfoForInterpretedFrame(void* frame_ptr);
 BoxedDict* localsForInterpretedFrame(void* frame_ptr, bool only_user_visible);
+#endif
 }
 
 #endif

src/codegen/unwinding.cpp

@@ -308,7 +308,7 @@ public:
             int code = unw_get_proc_info(&this->cursor, &pip);
             RELEASE_ASSERT(code == 0, "%d", code);
 
-            if (pip.start_ip == (intptr_t)interpretFunction || pip.start_ip == (intptr_t)astInterpretFunction) {
+            if (pip.start_ip == (intptr_t)astInterpretFunction) {
                 unw_word_t bp;
                 unw_get_reg(&this->cursor, UNW_TDEP_BP, &bp);
 

src/gc/root_finder.cpp

@@ -21,8 +21,8 @@
 #include <setjmp.h>
 #include <vector>
 
+#include "codegen/ast_interpreter.h"
 #include "codegen/codegen.h"
-#include "codegen/llvm_interpreter.h"
 #include "core/common.h"
 #include "core/threading.h"
 #include "gc/collector.h"

test/tests/arg_unpacking.py

+# expected: fail
+
+def f((a,b)):
+    print a,b
+
+f(range(2))
+f((1, 2))
+

test/tests/generator_thread_sharing.py

+def gen():
+    while True:
+        for i in xrange(100):
+            range(100) * 1000 # allocate memory to force GC collections
+        b = yield 0
+        if b:
+            break
+
+done = []
+def thread_run(g):
+    for i in xrange(5):
+        g.next()
+    try:
+        g.send(1)
+        assert 0
+    except StopIteration:
+        pass
+    done.append(0)
+
+g = gen()
+for i in xrange(5):
+    g.next()
+
+from thread import start_new_thread
+t = start_new_thread(thread_run, (g,))
+g = ""
+
+def f():
+    pass
+
+f()
+f()
+
+import time
+while not done:
+    time.sleep(0.1)