From 109abdb061263c28ae97424729e979cd0ff8289a Mon Sep 17 00:00:00 2001
From: Erik Verbruggen <erik.verbruggen@digia.com>
Date: Mon, 11 Aug 2014 13:15:34 +0200
Subject: [PATCH] V4 JIT: fix LookupCall on ARM

To generate a LookupCall, the register r8 was used on ARM instead of the
ReturnValue register. The reason is that the ReturnValue register is
also the register for the first argument. However, now that we use
callee-saved registers (r8 among them), this would clobber any value
stored in r8. The fix is to actually use r0 to calculate the value,
because the first argument holds the lookup table, and the call is
relative to that. This leaves r8 free to be used by the register
allocator.

Change-Id: I5095bf69d27e16111ad32d9e5d5691c7bce14516
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>
---
 src/qml/jit/qv4assembler_p.h | 42 ++++++++++++++++++++++++++++++++++++++----
 src/qml/jit/qv4isel_masm_p.h | 18 +++++++-----------
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/src/qml/jit/qv4assembler_p.h b/src/qml/jit/qv4assembler_p.h
index 4c5d2ae..36611b9 100644
--- a/src/qml/jit/qv4assembler_p.h
+++ b/src/qml/jit/qv4assembler_p.h
@@ -93,6 +93,15 @@ struct RelativeCall {
     {}
 };
 
+struct LookupCall {
+    JSC::MacroAssembler::Address addr;
+    uint getterSetterOffset;
+
+    LookupCall(const JSC::MacroAssembler::Address &addr, uint getterSetterOffset)
+        : addr(addr)
+        , getterSetterOffset(getterSetterOffset)
+    {}
+};
 
 template <typename T>
 struct ExceptionCheck {
@@ -344,6 +353,11 @@ public:
         call(relativeCall.addr);
     }
 
+    void callAbsolute(const char* /*functionName*/, const LookupCall &lookupCall)
+    {
+        call(lookupCall.addr);
+    }
+
     void registerBlock(IR::BasicBlock*, IR::BasicBlock *nextBlock);
     IR::BasicBlock *nextBlock() const { return _nextBlock; }
     void jumpToBlock(IR::BasicBlock* current, IR::BasicBlock *target);
@@ -866,8 +880,8 @@ public:
         loadArgumentOnStackOrRegister<2>(arg3);
         loadArgumentOnStackOrRegister<1>(arg2);
 
-        prepareRelativeCall(function, this);
-        loadArgumentOnStackOrRegister<0>(arg1);
+        if (prepareCall(function, this))
+            loadArgumentOnStackOrRegister<0>(arg1);
 
 #ifdef RESTORE_EBX_ON_CALL
         load32(ebxAddressOnStack(), JSC::X86Registers::ebx); // restore the GOT ptr
@@ -1198,11 +1212,31 @@ void Assembler::copyValue(Result result, IR::Expr* source)
 
 
 
-template <typename T> inline void prepareRelativeCall(const T &, Assembler *){}
-template <> inline void prepareRelativeCall(const RelativeCall &relativeCall, Assembler *as)
+template <typename T> inline bool prepareCall(T &, Assembler *)
+{ return true; }
+
+template <> inline bool prepareCall(RelativeCall &relativeCall, Assembler *as)
 {
     as->loadPtr(Assembler::Address(Assembler::ContextRegister, qOffsetOf(QV4::ExecutionContext::Data, lookups)),
                 relativeCall.addr.base);
+    return true;
+}
+
+template <> inline bool prepareCall(LookupCall &lookupCall, Assembler *as)
+{
+    // IMPORTANT! See generateLookupCall in qv4isel_masm_p.h for details!
+
+    // same as prepareCall(RelativeCall ....) : load the table from the context
+    as->loadPtr(Assembler::Address(Assembler::ContextRegister, qOffsetOf(QV4::ExecutionContext::Data, lookups)),
+                lookupCall.addr.base);
+    // pre-calculate the indirect address for the lookupCall table:
+    if (lookupCall.addr.offset)
+        as->addPtr(Assembler::TrustedImm32(lookupCall.addr.offset), lookupCall.addr.base);
+    // store it as the first argument
+    as->loadArgumentOnStackOrRegister<0>(lookupCall.addr.base);
+    // set the destination addresses offset to the getterSetterOffset. The base is the lookupCall table's address
+    lookupCall.addr.offset = lookupCall.getterSetterOffset;
+    return false;
 }
 
 } // end of namespace JIT
diff --git a/src/qml/jit/qv4isel_masm_p.h b/src/qml/jit/qv4isel_masm_p.h
index 9ed8be8..1f1e71a 100644
--- a/src/qml/jit/qv4isel_masm_p.h
+++ b/src/qml/jit/qv4isel_masm_p.h
@@ -229,19 +229,15 @@ private:
     template <typename Retval, typename Arg1, typename Arg2, typename Arg3>
     void generateLookupCall(Retval retval, uint index, uint getterSetterOffset, Arg1 arg1, Arg2 arg2, Arg3 arg3)
     {
-        Assembler::RegisterID lookupRegister;
-#if CPU(ARM)
-        lookupRegister = JSC::ARMRegisters::r8;
-#else
-        lookupRegister = Assembler::ReturnValueRegister;
-#endif
-        Assembler::Pointer lookupAddr(lookupRegister, index * sizeof(QV4::Lookup));
-
-        Assembler::Address getterSetter = lookupAddr;
-        getterSetter.offset += getterSetterOffset;
+        // Note: using the return value register is intentional: for ABIs where the first parameter
+        // goes into the same register as the return value (currently only ARM), the prepareCall
+        // will combine loading the looupAddr into the register and calculating the indirect call
+        // address.
+        Assembler::Pointer lookupAddr(Assembler::ReturnValueRegister, index * sizeof(QV4::Lookup));
 
          _as->generateFunctionCallImp(retval, "lookup getter/setter",
-                                      RelativeCall(getterSetter), lookupAddr, arg1, arg2, arg3);
+                                      LookupCall(lookupAddr, getterSetterOffset), lookupAddr,
+                                      arg1, arg2, arg3);
     }
 
     template <typename Retval, typename Arg1, typename Arg2>
-- 
2.7.4