callattr runtimeCall: don't generate arg guards if we know that the classes can't change

Author: undingen

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) {

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);
 

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);

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.

src/codegen/compvars.cpp

@@ -584,12 +584,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<BoxedClass*> guaranteed_classes;
     std::vector<ConcreteCompilerVariable*> 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::Value*> llvm_args;
@@ -654,7 +658,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);
 

src/codegen/patchpoints.cpp

@@ -48,8 +48,9 @@ int ICSetupInfo::totalSize() const {
 static std::vector<std::pair<PatchpointInfo*, void* /* addr of func to call */>> 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);
@@ -258,6 +259,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.
@@ -333,9 +335,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) {

src/codegen/patchpoints.h

@@ -59,18 +59,21 @@ 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)
         : 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:
     const ICType type;
 
     const int num_slots, slot_size;
     const bool has_return_value;
+    const bool has_const_arg_classes;
     TypeRecorder* const type_recorder;
 
     int totalSize() const;
@@ -91,7 +94,7 @@ 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);
 };
 
 struct PatchpointInfo {
@@ -163,7 +166,8 @@ struct PatchpointInfo {
 
 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);

src/runtime/objmodel.cpp

@@ -3127,11 +3127,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)
@@ -4413,11 +4412,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)