Only rewrite IC slots that are not currently being executed

Before, we tried to do it based on whether the rewrite we were
adding was "compatible" with whatever was already in there.  But we
weren't really doing this and there were a lot of limitations with this
method anyway.

src/asm_writing/icinfo.cpp

@@ -71,7 +71,7 @@ void ICSlotRewrite::abort() {
     ic->failed = true;
 }
 
-void ICSlotRewrite::commit(uint64_t decision_path, CommitHook* hook) {
+void ICSlotRewrite::commit(CommitHook* hook) {
     bool still_valid = true;
     for (int i = 0; i < dependencies.size(); i++) {
         int orig_version = dependencies[i].second;
@@ -87,7 +87,7 @@ void ICSlotRewrite::commit(uint64_t decision_path, CommitHook* hook) {
         return;
     }
 
-    ICSlotInfo* ic_entry = ic->pickEntryForRewrite(decision_path, debug_name);
+    ICSlotInfo* ic_entry = ic->pickEntryForRewrite(debug_name);
     if (ic_entry == NULL)
         return;
 
@@ -146,40 +146,25 @@ ICSlotRewrite* ICInfo::startRewrite(const char* debug_name) {
     return new ICSlotRewrite(this, debug_name);
 }
 
-ICSlotInfo* ICInfo::pickEntryForRewrite(uint64_t decision_path, const char* debug_name) {
-    for (int i = 0; i < getNumSlots(); i++) {
-        SlotInfo& sinfo = slots[i];
-        if (!sinfo.is_patched) {
-            if (VERBOSITY()) {
-                printf("committing %s icentry to unused slot %d at %p\n", debug_name, i, start_addr);
-            }
-
-            sinfo.is_patched = true;
-            sinfo.decision_path = decision_path;
-            return &sinfo.entry;
-        }
-    }
-
+ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) {
     int num_slots = getNumSlots();
     for (int _i = 0; _i < num_slots; _i++) {
         int i = (_i + next_slot_to_try) % num_slots;
 
-        SlotInfo& sinfo = slots[i];
-        if (sinfo.is_patched && sinfo.decision_path != decision_path) {
+        ICSlotInfo& sinfo = slots[i];
+        assert(sinfo.num_inside >= 0);
+        if (sinfo.num_inside)
             continue;
-        }
 
         if (VERBOSITY()) {
             printf("committing %s icentry to in-use slot %d at %p\n", debug_name, i, start_addr);
         }
-        next_slot_to_try++;
+        next_slot_to_try = i + 1;
 
-        sinfo.is_patched = true;
-        sinfo.decision_path = decision_path;
-        return &sinfo.entry;
+        return &sinfo;
     }
     if (VERBOSITY())
-        printf("not committing %s icentry since it is not compatible (%lx)\n", debug_name, decision_path);
+        printf("not committing %s icentry since there are no available slots\n", debug_name);
     return NULL;
 }
 
@@ -193,7 +178,7 @@ ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, S
       type_recorder(type_recorder), failed(false), start_addr(start_addr), slowpath_rtn_addr(slowpath_rtn_addr),
       continue_addr(continue_addr) {
     for (int i = 0; i < num_slots; i++) {
-        slots.push_back(SlotInfo(this, i));
+        slots.push_back(ICSlotInfo(this, i));
     }
 }
 

src/asm_writing/icinfo.h

@@ -74,7 +74,7 @@ public:
     assembler::GenericRegister returnRegister();
 
     void addDependenceOn(ICInvalidator&);
-    void commit(uint64_t decision_path, CommitHook* hook);
+    void commit(CommitHook* hook);
     void abort();
 
     friend class ICInfo;
@@ -82,14 +82,7 @@ public:
 
 class ICInfo {
 private:
-    struct SlotInfo {
-        bool is_patched;
-        uint64_t decision_path;
-        ICSlotInfo entry;
-
-        SlotInfo(ICInfo* ic, int idx) : is_patched(false), decision_path(0), entry(ic, idx) {}
-    };
-    std::vector<SlotInfo> slots;
+    std::vector<ICSlotInfo> slots;
     // For now, just use a round-robin eviction policy.
     // This is probably a bunch worse than LRU, but it's also
     // probably a bunch better than the "always evict slot #0" policy
@@ -106,7 +99,7 @@ private:
     bool failed;
 
     // for ICSlotRewrite:
-    ICSlotInfo* pickEntryForRewrite(uint64_t decision_path, const char* debug_name);
+    ICSlotInfo* pickEntryForRewrite(const char* debug_name);
 
 public:
     ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, StackInfo stack_info, int num_slots,

src/asm_writing/rewriter.cpp

@@ -936,7 +936,7 @@ void Rewriter::commit() {
     finished = true;
 
     // TODO: have to check that we have enough room to write the final jmp
-    rewrite->commit(decision_path, this);
+    rewrite->commit(this);
 
     assert(!assembler->hasFailed());
 }
@@ -966,12 +966,6 @@ void Rewriter::commitReturning(RewriterVar* var) {
     commit();
 }
 
-void Rewriter::addDecision(int way) {
-    assert(ndecisions < 60);
-    ndecisions++;
-    decision_path = (decision_path << 1) | way;
-}
-
 void Rewriter::addDependenceOn(ICInvalidator& invalidator) {
     rewrite->addDependenceOn(invalidator);
 }
@@ -1351,8 +1345,7 @@ TypeRecorder* Rewriter::getTypeRecorder() {
 
 Rewriter::Rewriter(ICSlotRewrite* rewrite, int num_args, const std::vector<int>& live_outs)
     : rewrite(rewrite), assembler(rewrite->getAssembler()), return_location(rewrite->returnRegister()),
-      added_changing_action(false), marked_inside_ic(false), last_guard_action(-1), done_guarding(false), ndecisions(0),
-      decision_path(1) {
+      added_changing_action(false), marked_inside_ic(false), last_guard_action(-1), done_guarding(false) {
     initPhaseCollecting();
 
 #ifndef NDEBUG

src/asm_writing/rewriter.h

@@ -379,9 +379,6 @@ private:
 
     void finishAssembly(ICSlotInfo* picked_slot, int continue_offset) override;
 
-    int ndecisions;
-    uint64_t decision_path;
-
     void _trap();
     void _loadConst(RewriterVar* result, int64_t val, Location loc);
     void _call(RewriterVar* result, bool can_call_into_python, void* func_addr, const std::vector<RewriterVar*>& args,
@@ -464,8 +461,6 @@ public:
 
     static Rewriter* createRewriter(void* rtn_addr, int num_args, const char* debug_name);
 
-    void addDecision(int way);
-
     friend class RewriterVar;
 };
 

src/runtime/objmodel.cpp

@@ -2731,9 +2731,6 @@ Box* runtimeCallInternal(Box* obj, CallRewriteArgs* rewrite_args, ArgPassSpec ar
             }
             rewrite_args->args_guarded = true;
         }
-
-        rewrite_args->rewriter->addDecision(obj->cls == function_cls || obj->cls == builtin_function_or_method_cls ? 1
-                                                                                                                   : 0);
     }
 
     if (obj->cls == function_cls || obj->cls == builtin_function_or_method_cls) {