Merge pull request #706 from undingen/interp_init

Slightly speedup ASTInterpreter::initArguments

src/codegen/ast_interpreter.cpp

@@ -35,6 +35,7 @@
 #include "core/stats.h"
 #include "core/thread_utils.h"
 #include "core/util.h"
+#include "gc/roots.h"
 #include "runtime/generator.h"
 #include "runtime/import.h"
 #include "runtime/inline/boxing.h"
@@ -102,7 +103,7 @@ private:
     Box* createFunction(AST* node, AST_arguments* args, const std::vector<AST_stmt*>& body);
     Value doBinOp(Value left, Value right, int op, BinExpType exp_type);
     void doStore(AST_expr* node, Value value);
-    void doStore(InternedString name, Value value);
+    void doStore(AST_Name* name, Value value);
     Box* doOSR(AST_Jump* node);
     Value getNone();
 
@@ -299,24 +300,25 @@ void ASTInterpreter::initArguments(int nargs, BoxedClosure* _closure, BoxedGener
     if (scope_info->createsClosure())
         created_closure = createClosure(passed_closure, scope_info->getClosureSize());
 
-    std::vector<Box*, StlCompatAllocator<Box*>> argsArray{ arg1, arg2, arg3 };
-    for (int i = 3; i < nargs; ++i)
-        argsArray.push_back(args[i - 3]);
-
     const ParamNames& param_names = clfunc->param_names;
 
+    // make sure the AST_Name nodes are set
+    assert(param_names.args.size() == param_names.arg_names.size());
+    assert(param_names.vararg.empty() == (param_names.vararg_name == NULL));
+    assert(param_names.kwarg.empty() == (param_names.kwarg_name == NULL));
+
     int i = 0;
-    for (auto& name : param_names.args) {
-        doStore(source_info->getInternedStrings().get(name), Value(argsArray[i++], 0));
+    for (auto& name : param_names.arg_names) {
+        doStore(name, Value(getArg(i++, arg1, arg2, arg3, args), 0));
     }
 
-    if (!param_names.vararg.str().empty()) {
-        doStore(source_info->getInternedStrings().get(param_names.vararg), Value(argsArray[i++], 0));
-    }
+    if (param_names.vararg_name)
+        doStore(param_names.vararg_name, Value(getArg(i++, arg1, arg2, arg3, args), 0));
 
-    if (!param_names.kwarg.str().empty()) {
-        doStore(source_info->getInternedStrings().get(param_names.kwarg), Value(argsArray[i++], 0));
-    }
+    if (param_names.kwarg_name)
+        doStore(param_names.kwarg_name, Value(getArg(i++, arg1, arg2, arg3, args), 0));
+
+    assert(nargs == i);
 }
 
 RegisterHelper::RegisterHelper() : frame_addr(NULL), interpreter(NULL) {
@@ -506,8 +508,12 @@ Value ASTInterpreter::doBinOp(Value left, Value right, int op, BinExpType exp_ty
     return Value();
 }
 
-void ASTInterpreter::doStore(InternedString name, Value value) {
-    ScopeInfo::VarScopeType vst = scope_info->getScopeTypeOfName(name);
+void ASTInterpreter::doStore(AST_Name* node, Value value) {
+    if (node->lookup_type == ScopeInfo::VarScopeType::UNKNOWN)
+        node->lookup_type = scope_info->getScopeTypeOfName(node->id);
+
+    InternedString name = node->id;
+    ScopeInfo::VarScopeType vst = node->lookup_type;
     if (vst == ScopeInfo::VarScopeType::GLOBAL) {
         if (jit)
             jit->emitSetGlobal(globals, name.getBox(), value);
@@ -541,7 +547,7 @@ void ASTInterpreter::doStore(InternedString name, Value value) {
 void ASTInterpreter::doStore(AST_expr* node, Value value) {
     if (node->type == AST_TYPE::Name) {
         AST_Name* name = (AST_Name*)node;
-        doStore(name->id, value);
+        doStore(name, value);
     } else if (node->type == AST_TYPE::Attribute) {
         AST_Attribute* attr = (AST_Attribute*)node;
         Value o = visit_expr(attr->value);

src/codegen/irgen/hooks.cpp

@@ -45,7 +45,8 @@
 namespace pyston {
 
 // TODO terrible place for these!
-ParamNames::ParamNames(AST* ast, InternedStringPool& pool) : takes_param_names(true) {
+ParamNames::ParamNames(AST* ast, InternedStringPool& pool)
+    : takes_param_names(true), vararg_name(NULL), kwarg_name(NULL) {
     if (ast->type == AST_TYPE::Module || ast->type == AST_TYPE::ClassDef || ast->type == AST_TYPE::Expression
         || ast->type == AST_TYPE::Suite) {
         kwarg = "";
@@ -56,22 +57,30 @@ ParamNames::ParamNames(AST* ast, InternedStringPool& pool) : takes_param_names(t
         for (int i = 0; i < arguments->args.size(); i++) {
             AST_expr* arg = arguments->args[i];
             if (arg->type == AST_TYPE::Name) {
-                args.push_back(ast_cast<AST_Name>(arg)->id.s());
+                AST_Name* name = ast_cast<AST_Name>(arg);
+                arg_names.push_back(name);
+                args.push_back(name->id.s());
             } else {
                 InternedString dot_arg_name = pool.get("." + std::to_string(i));
+                arg_names.push_back(new AST_Name(dot_arg_name, AST_TYPE::Param, arg->lineno, arg->col_offset));
                 args.push_back(dot_arg_name.s());
             }
         }
 
         vararg = arguments->vararg.s();
+        if (vararg.size())
+            vararg_name = new AST_Name(pool.get(vararg), AST_TYPE::Param, arguments->lineno, arguments->col_offset);
+
         kwarg = arguments->kwarg.s();
+        if (kwarg.size())
+            kwarg_name = new AST_Name(pool.get(kwarg), AST_TYPE::Param, arguments->lineno, arguments->col_offset);
     } else {
         RELEASE_ASSERT(0, "%d", ast->type);
     }
 }
 
 ParamNames::ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg)
-    : takes_param_names(true) {
+    : takes_param_names(true), vararg_name(NULL), kwarg_name(NULL) {
     this->args = args;
     this->vararg = vararg;
     this->kwarg = kwarg;

src/core/types.h

@@ -95,6 +95,7 @@ class AST;
 class AST_FunctionDef;
 class AST_arguments;
 class AST_expr;
+class AST_Name;
 class AST_stmt;
 
 class PhiAnalysis;
@@ -147,6 +148,12 @@ struct ParamNames {
     std::vector<llvm::StringRef> args;
     llvm::StringRef vararg, kwarg;
 
+    // This members are only set if the InternedStringPool& constructor is used (aka. source is available)!
+    // They are used as an optimization while interpreting because the AST_Names nodes cache important stuff
+    // (InternedString, lookup_type) which would otherwise have to get recomputed all the time.
+    std::vector<AST_Name*> arg_names;
+    AST_Name* vararg_name, *kwarg_name;
+
     explicit ParamNames(AST* ast, InternedStringPool& pool);
     ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg);
     static ParamNames empty() { return ParamNames(); }
@@ -156,7 +163,7 @@ struct ParamNames {
     }
 
 private:
-    ParamNames() : takes_param_names(false) {}
+    ParamNames() : takes_param_names(false), vararg_name(NULL), kwarg_name(NULL) {}
 };
 
 // Probably overkill to copy this from ArgPassSpec