From d88a295b12c9a996bf863766cd2465e7b5141d15 Mon Sep 17 00:00:00 2001 From: Marius Wachtler Date: Fri, 23 Oct 2015 11:41:01 +0100 Subject: [PATCH] callattr runtimeCall: don't generate arg guards if we know that the classes can't change --- src/asm_writing/icinfo.cpp | 1 + src/asm_writing/icinfo.h | 4 ++++ src/asm_writing/rewriter.cpp | 4 ++-- src/asm_writing/rewriter.h | 2 ++ src/codegen/compvars.cpp | 9 +++++++-- src/codegen/patchpoints.cpp | 12 +++++++----- src/codegen/patchpoints.h | 10 +++++++--- src/runtime/objmodel.cpp | 14 ++++++-------- 8 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/asm_writing/icinfo.cpp b/src/asm_writing/icinfo.cpp index bd28e9ce2..c6541914c 100644 --- a/src/asm_writing/icinfo.cpp +++ b/src/asm_writing/icinfo.cpp @@ -236,6 +236,7 @@ ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, S retry_in(0), retry_backoff(1), times_rewritten(0), + has_const_arg_classes(false), start_addr(start_addr), slowpath_rtn_addr(slowpath_rtn_addr), continue_addr(continue_addr) { diff --git a/src/asm_writing/icinfo.h b/src/asm_writing/icinfo.h index 327a8efb4..4c8a4ee92 100644 --- a/src/asm_writing/icinfo.h +++ b/src/asm_writing/icinfo.h @@ -122,6 +122,7 @@ class ICInfo { TypeRecorder* const type_recorder; int retry_in, retry_backoff; int times_rewritten; + bool has_const_arg_classes; // for ICSlotRewrite: ICSlotInfo* pickEntryForRewrite(const char* debug_name); @@ -149,6 +150,9 @@ class ICInfo { int percentBackedoff() const { return retry_backoff; } int timesRewritten() const { return times_rewritten; } + void setHasConstArgClasses(bool b = true) { has_const_arg_classes = b; } + bool hasConstArgClasses() const { return has_const_arg_classes; } + friend class ICSlotRewrite; static void visitGCReferences(gc::GCVisitor* visitor); diff --git a/src/asm_writing/rewriter.cpp b/src/asm_writing/rewriter.cpp index c460afcdd..02dd7b6b0 100644 --- a/src/asm_writing/rewriter.cpp +++ b/src/asm_writing/rewriter.cpp @@ -1986,9 +1986,9 @@ Rewriter* Rewriter::createRewriter(void* rtn_addr, int num_args, const char* deb // Horrible non-robust optimization: addresses below this address are probably in the binary (ex the interpreter), // so don't do the more-expensive hash table lookup to find it. if (rtn_addr > (void*)0x1000000) { - ic = getICInfo(rtn_addr); + ic = pyston::getICInfo(rtn_addr); } else { - ASSERT(!getICInfo(rtn_addr), "%p", rtn_addr); + ASSERT(!pyston::getICInfo(rtn_addr), "%p", rtn_addr); } log_ic_attempts(debug_name); diff --git a/src/asm_writing/rewriter.h b/src/asm_writing/rewriter.h index ad5df4c50..a08c2f9a2 100644 --- a/src/asm_writing/rewriter.h +++ b/src/asm_writing/rewriter.h @@ -555,6 +555,8 @@ class Rewriter : public ICSlotRewrite::CommitHook, public gc::GCVisitable { TypeRecorder* getTypeRecorder(); + const ICInfo* getICInfo() { return rewrite->getICInfo(); } + const char* debugName() { return rewrite->debugName(); } // Register that this rewrite will embed a reference to a particular gc object. diff --git a/src/codegen/compvars.cpp b/src/codegen/compvars.cpp index 93ea3f245..cd495b4df 100644 --- a/src/codegen/compvars.cpp +++ b/src/codegen/compvars.cpp @@ -580,12 +580,16 @@ static ConcreteCompilerVariable* _call(IREmitter& emitter, const OpInfo& info, l bool pass_keyword_names = (keyword_names != nullptr); assert(pass_keyword_names == (argspec.num_keywords > 0)); + bool has_const_arg_classes = true; std::vector guaranteed_classes; std::vector converted_args; for (int i = 0; i < args.size(); i++) { assert(args[i]); converted_args.push_back(args[i]->makeConverted(emitter, args[i]->getBoxType())); - guaranteed_classes.push_back(converted_args.back()->guaranteedClass()); + BoxedClass* guaranteed_class = converted_args.back()->guaranteedClass(); + guaranteed_classes.push_back(guaranteed_class); + if (guaranteed_class == NULL) + has_const_arg_classes = false; } std::vector llvm_args; @@ -650,7 +654,8 @@ static ConcreteCompilerVariable* _call(IREmitter& emitter, const OpInfo& info, l if (do_patchpoint) { assert(func_addr); - ICSetupInfo* pp = createCallsiteIC(info.getTypeRecorder(), args.size(), info.getBJitICInfo()); + ICSetupInfo* pp + = createCallsiteIC(info.getTypeRecorder(), args.size(), info.getBJitICInfo(), has_const_arg_classes); llvm::Value* uncasted = emitter.createIC(pp, func_addr, llvm_args, info.unw_info, target_exception_style); diff --git a/src/codegen/patchpoints.cpp b/src/codegen/patchpoints.cpp index 6550c1bcd..c5fe87d4a 100644 --- a/src/codegen/patchpoints.cpp +++ b/src/codegen/patchpoints.cpp @@ -45,8 +45,9 @@ int ICSetupInfo::totalSize() const { static std::vector> new_patchpoints; ICSetupInfo* ICSetupInfo::initialize(bool has_return_value, int num_slots, int slot_size, ICType type, - TypeRecorder* type_recorder) { - ICSetupInfo* rtn = new ICSetupInfo(type, num_slots, slot_size, has_return_value, type_recorder); + TypeRecorder* type_recorder, bool has_const_arg_classes) { + ICSetupInfo* rtn + = new ICSetupInfo(type, num_slots, slot_size, has_return_value, type_recorder, has_const_arg_classes); // We use size == CALL_ONLY_SIZE to imply that the call isn't patchable assert(rtn->totalSize() > CALL_ONLY_SIZE); @@ -255,6 +256,7 @@ void processStackmap(CompiledFunction* cf, StackMap* stackmap) { start_addr, initialization_info.slowpath_start, initialization_info.continue_addr, initialization_info.slowpath_rtn_addr, ic, StackInfo(scratch_size, scratch_rsp_offset), std::move(initialization_info.live_outs)); + icinfo->setHasConstArgClasses(ic->has_const_arg_classes); assert(cf); // TODO: unsafe. hard to use a unique_ptr here though. @@ -330,9 +332,9 @@ ICSetupInfo* createDelattrIC(TypeRecorder* type_recorder) { return ICSetupInfo::initialize(false, 1, 144, ICSetupInfo::Delattr, type_recorder); } -ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info) { - return ICSetupInfo::initialize(true, numSlots(bjit_ic_info, 4), 640 + 48 * num_args, ICSetupInfo::Callsite, - type_recorder); +ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info, bool const_arg_classes) { + return ICSetupInfo::initialize(true, const_arg_classes ? 1 : numSlots(bjit_ic_info, 4), 640 + 48 * num_args, + ICSetupInfo::Callsite, type_recorder, const_arg_classes); } ICSetupInfo* createGetGlobalIC(TypeRecorder* type_recorder) { diff --git a/src/codegen/patchpoints.h b/src/codegen/patchpoints.h index 6934ba397..9d1787676 100644 --- a/src/codegen/patchpoints.h +++ b/src/codegen/patchpoints.h @@ -118,11 +118,13 @@ class ICSetupInfo { }; private: - ICSetupInfo(ICType type, int num_slots, int slot_size, bool has_return_value, TypeRecorder* type_recorder) + ICSetupInfo(ICType type, int num_slots, int slot_size, bool has_return_value, TypeRecorder* type_recorder, + bool has_const_arg_classes = false) : type(type), num_slots(num_slots), slot_size(slot_size), has_return_value(has_return_value), + has_const_arg_classes(has_const_arg_classes), type_recorder(type_recorder) {} public: @@ -130,6 +132,7 @@ class ICSetupInfo { const int num_slots, slot_size; const bool has_return_value; + const bool has_const_arg_classes; TypeRecorder* const type_recorder; int totalSize() const; @@ -149,12 +152,13 @@ class ICSetupInfo { } static ICSetupInfo* initialize(bool has_return_value, int num_slots, int slot_size, ICType type, - TypeRecorder* type_recorder); + TypeRecorder* type_recorder, bool has_const_arg_classes = false); }; class ICInfo; ICSetupInfo* createGenericIC(TypeRecorder* type_recorder, bool has_return_value, int size); -ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info); +ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info, + bool has_const_arg_classes); ICSetupInfo* createGetGlobalIC(TypeRecorder* type_recorder); ICSetupInfo* createGetattrIC(TypeRecorder* type_recorder, ICInfo* bjit_ic_info); ICSetupInfo* createSetattrIC(TypeRecorder* type_recorder, ICInfo* bjit_ic_info); diff --git a/src/runtime/objmodel.cpp b/src/runtime/objmodel.cpp index 3e9bfd764..a169e3d7f 100644 --- a/src/runtime/objmodel.cpp +++ b/src/runtime/objmodel.cpp @@ -3137,11 +3137,10 @@ Box* _callattrEntry(Box* obj, BoxedString* attr, CallattrFlags flags, Box* arg1, } if (rewriter.get()) { - // TODO feel weird about doing this; it either isn't necessary - // or this kind of thing is necessary in a lot more places - // rewriter->getArg(3).addGuard(npassed_args); - CallattrRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination()); + if (rewriter->getICInfo()->hasConstArgClasses()) + rewrite_args.args_guarded = true; + if (npassed_args >= 1) rewrite_args.arg1 = rewriter->getArg(3); if (npassed_args >= 2) @@ -4424,11 +4423,10 @@ static Box* runtimeCallEntry(Box* obj, ArgPassSpec argspec, Box* arg1, Box* arg2 #endif if (rewriter.get()) { - // TODO feel weird about doing this; it either isn't necessary - // or this kind of thing is necessary in a lot more places - // rewriter->getArg(1).addGuard(npassed_args); - CallRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination()); + if (rewriter->getICInfo()->hasConstArgClasses()) + rewrite_args.args_guarded = true; + if (npassed_args >= 1) rewrite_args.arg1 = rewriter->getArg(2); if (npassed_args >= 2)