Merge pull request #659 from tjhance/displacement

Use less large constants

src/asm_writing/assembler.cpp

@@ -670,7 +670,24 @@ void Assembler::decl(Indirect mem) {
     }
 }
 
+void Assembler::incl(Immediate imm) {
+    emitByte(0xff);
+    emitByte(0x04);
+    emitByte(0x25);
+    emitInt(imm.val, 4);
+}
+
+void Assembler::decl(Immediate imm) {
+    emitByte(0xff);
+    emitByte(0x0c);
+    emitByte(0x25);
+    emitInt(imm.val, 4);
+}
 
+void Assembler::call(Immediate imm) {
+    emitByte(0xe8);
+    emitInt(imm.val, 4);
+}
 
 void Assembler::callq(Register r) {
     assert(r == R11 && "untested");

src/asm_writing/assembler.h

@@ -149,9 +149,14 @@ public:
 
     void add(Immediate imm, Register reg);
     void sub(Immediate imm, Register reg);
+
     void incl(Indirect mem);
     void decl(Indirect mem);
 
+    void incl(Immediate mem);
+    void decl(Immediate mem);
+
+    void call(Immediate imm); // the value is the offset
     void callq(Register reg);
     void retq();
     void leave();
@@ -187,6 +192,7 @@ public:
     void fillWithNopsExcept(int bytes);
     void emitAnnotation(int num);
 
+    uint8_t* startAddr() const { return start_addr; }
     int bytesLeft() const { return end_addr - addr; }
     int bytesWritten() const { return addr - start_addr; }
     uint8_t* curInstPointer() { return addr; }

src/asm_writing/icinfo.cpp

@@ -75,6 +75,16 @@ void ICSlotRewrite::abort() {
     ic->retry_in = ic->retry_backoff;
 }
 
+ICSlotInfo* ICSlotRewrite::prepareEntry() {
+    this->ic_entry = ic->pickEntryForRewrite(debug_name);
+    return this->ic_entry;
+}
+
+uint8_t* ICSlotRewrite::getSlotStart() {
+    assert(ic_entry != NULL);
+    return (uint8_t*)ic->start_addr + ic_entry->idx * ic->getSlotSize();
+}
+
 void ICSlotRewrite::commit(CommitHook* hook) {
     bool still_valid = true;
     for (int i = 0; i < dependencies.size(); i++) {
@@ -91,14 +101,10 @@ void ICSlotRewrite::commit(CommitHook* hook) {
         return;
     }
 
-    ICSlotInfo* ic_entry = ic->pickEntryForRewrite(debug_name);
-    if (ic_entry == NULL)
-        return;
-
-    uint8_t* slot_start = (uint8_t*)ic->start_addr + ic_entry->idx * ic->getSlotSize();
+    uint8_t* slot_start = getSlotStart();
     uint8_t* continue_point = (uint8_t*)ic->continue_addr;
 
-    bool do_commit = hook->finishAssembly(ic_entry, continue_point - slot_start);
+    bool do_commit = hook->finishAssembly(continue_point - slot_start);
 
     if (!do_commit)
         return;
@@ -111,6 +117,8 @@ void ICSlotRewrite::commit(CommitHook* hook) {
         invalidator->addDependent(ic_entry);
     }
 
+    ic->next_slot_to_try++;
+
     // if (VERBOSITY()) printf("Commiting to %p-%p\n", start, start + ic->slot_size);
     memcpy(slot_start, buf, ic->getSlotSize());
 
@@ -166,10 +174,10 @@ ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) {
             continue;
 
         if (VERBOSITY() >= 4) {
-            printf("committing %s icentry to in-use slot %d at %p\n", debug_name, i, start_addr);
+            printf("picking %s icentry to in-use slot %d at %p\n", debug_name, i, start_addr);
         }
-        next_slot_to_try = i + 1;
 
+        next_slot_to_try = i;
         return &sinfo;
     }
     if (VERBOSITY() >= 4)
@@ -177,8 +185,6 @@ ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) {
     return NULL;
 }
 
-
-
 ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, StackInfo stack_info, int num_slots,
                int slot_size, llvm::CallingConv::ID calling_conv, const std::unordered_set<int>& live_outs,
                assembler::GenericRegister return_register, TypeRecorder* type_recorder)

src/asm_writing/icinfo.h

@@ -48,7 +48,7 @@ public:
     class CommitHook {
     public:
         virtual ~CommitHook() {}
-        virtual bool finishAssembly(ICSlotInfo* picked_slot, int fastpath_offset) = 0;
+        virtual bool finishAssembly(int fastpath_offset) = 0;
     };
 
 private:
@@ -60,6 +60,8 @@ private:
 
     std::vector<std::pair<ICInvalidator*, int64_t>> dependencies;
 
+    ICSlotInfo* ic_entry;
+
     ICSlotRewrite(ICInfo* ic, const char* debug_name);
 
 public:
@@ -69,11 +71,14 @@ public:
     int getSlotSize();
     int getScratchRspOffset();
     int getScratchSize();
+    uint8_t* getSlotStart();
 
     TypeRecorder* getTypeRecorder();
 
     assembler::GenericRegister returnRegister();
 
+    ICSlotInfo* prepareEntry();
+
     void addDependenceOn(ICInvalidator&);
     void commit(CommitHook* hook);
     void abort();

src/asm_writing/rewriter.cpp

@@ -813,17 +813,14 @@ void Rewriter::_call(RewriterVar* result, bool has_side_effects, void* func_addr
 
     if (has_side_effects) {
         if (!marked_inside_ic) {
-            // assembler->trap();
-
-            // TODO this is super hacky: we don't know the address that we want to inc/dec, since
-            // it depends on the slot that we end up picking, so just write out an arbitrary
-            // constant an we'll rewrite it later
-            // TODO if we can guarantee that the mark_addr will fit in 32 bits,
-            // we can use a more compact instruction encoding
-            mark_addr_addrs.push_back((void**)(assembler->curInstPointer() + 2));
-            assembler::Register reg = allocReg(Location::any());
-            assembler->mov(assembler::Immediate(0x1234567890abcdefL), reg);
+            uintptr_t counter_addr = (uintptr_t)(&picked_slot->num_inside);
+            if (isLargeConstant(counter_addr)) {
+                assembler::Register reg = allocReg(Location::any(), getReturnDestination());
+                assembler->mov(assembler::Immediate(counter_addr), reg);
                 assembler->incl(assembler::Indirect(reg, 0));
+            } else {
+                assembler->incl(assembler::Immediate(counter_addr));
+            }
 
             assertConsistent();
             marked_inside_ic = true;
@@ -953,8 +950,16 @@ void Rewriter::_call(RewriterVar* result, bool has_side_effects, void* func_addr
     }
 #endif
 
+    uint64_t asm_address = (uint64_t)assembler->curInstPointer() + 5;
+    uint64_t real_asm_address = asm_address + (uint64_t)rewrite->getSlotStart() - (uint64_t)assembler->startAddr();
+    int64_t offset = (int64_t)((uint64_t)func_addr - real_asm_address);
+    if (isLargeConstant(offset)) {
         const_loader.loadConstIntoReg((uint64_t)func_addr, r);
         assembler->callq(r);
+    } else {
+        assembler->call(assembler::Immediate(offset));
+        assert(asm_address == (uint64_t)assembler->curInstPointer());
+    }
 
     assert(vars_by_location.count(assembler::RAX) == 0);
     result->initializeInReg(assembler::RAX);
@@ -1071,6 +1076,12 @@ void Rewriter::commit() {
         on_done_guarding();
     }
 
+    picked_slot = rewrite->prepareEntry();
+    if (picked_slot == NULL) {
+        on_assemblyfail();
+        return;
+    }
+
     // Now, start emitting assembly; check if we're dong guarding after each.
     for (int i = 0; i < actions.size(); i++) {
         actions[i].action();
@@ -1090,15 +1101,14 @@ void Rewriter::commit() {
     if (marked_inside_ic) {
         assembler->comment("mark inside ic");
 
-        // TODO this is super hacky: we don't know the address that we want to inc/dec, since
-        // it depends on the slot that we end up picking, so just write out an arbitrary
-        // constant an we'll rewrite it later
-        // TODO if we can guarantee that the mark_addr will fit in 32 bits,
-        // we can use a more compact instruction encoding
-        mark_addr_addrs.push_back((void**)(assembler->curInstPointer() + 2));
+        uintptr_t counter_addr = (uintptr_t)(&picked_slot->num_inside);
+        if (isLargeConstant(counter_addr)) {
             assembler::Register reg = allocReg(Location::any(), getReturnDestination());
-        assembler->mov(assembler::Immediate(0x1234567890abcdefL), reg);
+            assembler->mov(assembler::Immediate(counter_addr), reg);
             assembler->decl(assembler::Indirect(reg, 0));
+        } else {
+            assembler->decl(assembler::Immediate(counter_addr));
+        }
     }
 
     assembler->comment("live outs");
@@ -1242,16 +1252,8 @@ void Rewriter::commit() {
     ic_rewrites_total_bytes.log(asm_size_bytes);
 }
 
-bool Rewriter::finishAssembly(ICSlotInfo* picked_slot, int continue_offset) {
-    if (marked_inside_ic) {
-        void* mark_addr = &picked_slot->num_inside;
-
-        // Go back and rewrite the faked constants to point to the correct address:
-        for (void** mark_addr_addr : mark_addr_addrs) {
-            assert(*mark_addr_addr == (void*)0x1234567890abcdefL);
-            *mark_addr_addr = mark_addr;
-        }
-    }
+bool Rewriter::finishAssembly(int continue_offset) {
+    assert(picked_slot);
 
     assembler->jmp(assembler::JumpDestination::fromStart(continue_offset));
 
@@ -1679,6 +1681,7 @@ TypeRecorder* Rewriter::getTypeRecorder() {
 Rewriter::Rewriter(std::unique_ptr<ICSlotRewrite> rewrite, int num_args, const std::vector<int>& live_outs)
     : rewrite(std::move(rewrite)),
       assembler(this->rewrite->getAssembler()),
+      picked_slot(NULL),
       const_loader(this),
       return_location(this->rewrite->returnRegister()),
       failed(false),

src/asm_writing/rewriter.h

@@ -317,6 +317,8 @@ protected:
 
     std::unique_ptr<ICSlotRewrite> rewrite;
     assembler::Assembler* assembler;
+    ICSlotInfo* picked_slot;
+
     ConstLoader const_loader;
     std::vector<RewriterVar*> vars;
 
@@ -370,7 +372,6 @@ protected:
     }
     bool added_changing_action;
     bool marked_inside_ic;
-    std::vector<void**> mark_addr_addrs;
 
     int last_guard_action;
 
@@ -408,7 +409,7 @@ protected:
     // Do the bookkeeping to say that var is no longer in location l
     void removeLocationFromVar(RewriterVar* var, Location l);
 
-    bool finishAssembly(ICSlotInfo* picked_slot, int continue_offset) override;
+    bool finishAssembly(int continue_offset) override;
 
     void _trap();
     void _loadConst(RewriterVar* result, int64_t val);