Fix various accumulator-saving problems

We need to keep the accumulator alive across function calls. This
requires:

1, Saving the accumulator on the stack if the function might allocate, to
   protect it from the garbage collector. However, we don't need to do
   that if the result of the function is to be saved in the accumulator
   and the function itself doesn't use the accumulator as argument. In
   this case the previous value becomes unaccessible and we might as
   well GC it.
2, In the JIT, restoring the accumulator from the stack after the
   function call if we want to ignore the return value.
3, Therefore, also saving the accumulator on the stack before calling in
   case of 2.

We assume that we don't need to keep the accumulator alive across the
jump to the exception handler. Saving the accumulator more often than
necessary is detrimental for performance. To make sure the assumption
holds, explicitly load the accumulator with undefined _before_ jumping
to any exception handler.

Change-Id: I78cbc42847b8885a0659b23f3b81655b7f1a0bc4
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

5 files changed, 27 insertions, 5 deletions

diff --git a/src/qml/jit/qv4assemblercommon_p.h b/src/qml/jit/qv4assemblercommon_p.h
index 729d0fc53d..dcf39ab636 100644
--- a/src/qml/jit/qv4assemblercommon_p.h
+++ b/src/qml/jit/qv4assemblercommon_p.h
@@ -621,9 +621,9 @@ public:
 
         loadPtr(exceptionHandlerAddress(), ScratchRegister);
         Jump exitFunction = branchPtr(Equal, ScratchRegister, TrustedImmPtr(0));
+        loadUndefined();
         jump(ScratchRegister);
         exitFunction.link(this);
-        loadUndefined();
 
         if (functionExit.isSet())
             jump(functionExit);
diff --git a/src/qml/jit/qv4baselineassembler.cpp b/src/qml/jit/qv4baselineassembler.cpp
index b13f646360..73e396890e 100644
--- a/src/qml/jit/qv4baselineassembler.cpp
+++ b/src/qml/jit/qv4baselineassembler.cpp
@@ -1469,6 +1469,12 @@ void BaselineAssembler::saveAccumulatorInFrame()
                                                         offsetof(CallData, accumulator)));
 }
 
+void BaselineAssembler::loadAccumulatorFromFrame()
+{
+    pasm()->loadAccumulator(PlatformAssembler::Address(PlatformAssembler::JSStackFrameRegister,
+                                                       offsetof(CallData, accumulator)));
+}
+
 static ReturnedValue TheJitIs__Tail_Calling__ToTheRuntimeSoTheJitFrameIsMissing(CppStackFrame *frame, ExecutionEngine *engine)
 {
     return Runtime::method_tailCall(frame, engine);
@@ -1594,7 +1600,6 @@ void BaselineAssembler::deadTemporalZoneCheck(int offsetForSavedIP, int variable
 {
     auto valueIsAliveJump = pasm()->jumpNotEmpty();
     storeInstructionPointer(offsetForSavedIP);
-    saveAccumulatorInFrame();
     prepareCallWithArgCount(2);
     passInt32AsArg(variableName, 1);
     passEngineAsArg(0);
diff --git a/src/qml/jit/qv4baselineassembler_p.h b/src/qml/jit/qv4baselineassembler_p.h
index 0aa508ae71..f5ae826c55 100644
--- a/src/qml/jit/qv4baselineassembler_p.h
+++ b/src/qml/jit/qv4baselineassembler_p.h
@@ -152,6 +152,7 @@ public:
     void passInt32AsArg(int value, int arg);
     void callRuntime(const char *functionName, const void *funcPtr, CallResultDestination dest);
     void saveAccumulatorInFrame();
+    void loadAccumulatorFromFrame();
     void jsTailCall(int func, int thisObject, int argc, int argv);
 
     // exception/context stuff
diff --git a/src/qml/jit/qv4baselinejit.cpp b/src/qml/jit/qv4baselinejit.cpp
index 7bd51ba37e..51cd15099d 100644
--- a/src/qml/jit/qv4baselinejit.cpp
+++ b/src/qml/jit/qv4baselinejit.cpp
@@ -75,6 +75,7 @@ void BaselineJIT::generate()
 
 #define STORE_IP() as->storeInstructionPointer(nextInstructionOffset())
 #define STORE_ACC() as->saveAccumulatorInFrame()
+#define LOAD_ACC() as->loadAccumulatorFromFrame()
 #define BASELINEJIT_GENERATE_RUNTIME_CALL(function, destination) \
     as->GENERATE_RUNTIME_CALL(function, destination)
 #define BASELINEJIT_GENERATE_TAIL_CALL(function) \
@@ -233,6 +234,7 @@ void BaselineJIT::generate_StoreNameSloppy(int name)
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Runtime::method_storeNameSloppy, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_StoreNameStrict(int name)
@@ -245,6 +247,7 @@ void BaselineJIT::generate_StoreNameStrict(int name)
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Runtime::method_storeNameStrict, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_LoadElement(int base)
@@ -270,6 +273,7 @@ void BaselineJIT::generate_StoreElement(int base, int index)
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Runtime::method_storeElement, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_LoadProperty(int name)
@@ -308,6 +312,7 @@ void BaselineJIT::generate_StoreProperty(int name, int base)
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Runtime::method_storeProperty, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_SetLookup(int index, int base)
@@ -327,7 +332,6 @@ void BaselineJIT::generate_SetLookup(int index, int base)
 void BaselineJIT::generate_LoadSuperProperty(int property)
 {
     STORE_IP();
-    STORE_ACC();
     as->prepareCallWithArgCount(2);
     as->passJSSlotAsArg(property, 1);
     as->passEngineAsArg(0);
@@ -345,6 +349,7 @@ void BaselineJIT::generate_StoreSuperProperty(int property)
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Runtime::method_storeSuperProperty, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_Yield()
@@ -589,6 +594,7 @@ void BaselineJIT::generate_PushBlockContext(int index)
     as->passInt32AsArg(index, 1);
     as->passJSSlotAsArg(0, 0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Helpers::pushBlockContext, CallResultDestination::Ignore);
+    as->loadAccumulatorFromFrame();
 }
 
 void BaselineJIT::generate_CloneBlockContext()
@@ -597,6 +603,7 @@ void BaselineJIT::generate_CloneBlockContext()
     as->prepareCallWithArgCount(1);
     as->passJSSlotAsArg(CallData::Context, 0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Helpers::cloneBlockContext, CallResultDestination::Ignore);
+    as->loadAccumulatorFromFrame();
 }
 
 void BaselineJIT::generate_PushScriptContext(int index)
@@ -607,6 +614,7 @@ void BaselineJIT::generate_PushScriptContext(int index)
     as->passEngineAsArg(1);
     as->passJSSlotAsArg(0, 0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Helpers::pushScriptContext, CallResultDestination::Ignore);
+    as->loadAccumulatorFromFrame();
 }
 
 void BaselineJIT::generate_PopScriptContext()
@@ -616,6 +624,7 @@ void BaselineJIT::generate_PopScriptContext()
     as->passEngineAsArg(1);
     as->passJSSlotAsArg(0, 0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Helpers::popScriptContext, CallResultDestination::Ignore);
+    as->loadAccumulatorFromFrame();
 }
 
 void BaselineJIT::generate_PopContext() { as->popContext(); }
@@ -716,11 +725,13 @@ void BaselineJIT::generate_TypeofValue()
 
 void BaselineJIT::generate_DeclareVar(int varName, int isDeletable)
 {
+    STORE_ACC();
     as->prepareCallWithArgCount(3);
     as->passInt32AsArg(varName, 2);
     as->passInt32AsArg(isDeletable, 1);
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Runtime::method_declareVar, CallResultDestination::Ignore);
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_DefineArray(int argc, int args)
@@ -778,11 +789,13 @@ void BaselineJIT::generate_CreateRestParameter(int argIndex)
 
 void BaselineJIT::generate_ConvertThisToObject()
 {
+    STORE_ACC();
     as->prepareCallWithArgCount(2);
     as->passJSSlotAsArg(CallData::This, 1);
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Helpers::convertThisToObject, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_LoadSuperConstructor()
@@ -909,6 +922,7 @@ void BaselineJIT::generate_ThrowOnNullOrUndefined()
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(Helpers::throwOnNullOrUndefined, CallResultDestination::Ignore);
     as->checkException();
+    LOAD_ACC();
 }
 
 void BaselineJIT::generate_GetTemplateObject(int index)
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index c69bb67061..47ad22f4f7 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -640,7 +640,6 @@ QV4::ReturnedValue VME::interpret(CppStackFrame *frame, ExecutionEngine *engine,
 
     MOTH_BEGIN_INSTR(LoadSuperProperty)
         STORE_IP();
-        STORE_ACC();
         acc = Runtime::method_loadSuperProperty(engine, STACK_VALUE(property));
         CHECK_EXCEPTION;
     MOTH_END_INSTR(LoadSuperProperty)
@@ -787,12 +786,14 @@ QV4::ReturnedValue VME::interpret(CppStackFrame *frame, ExecutionEngine *engine,
 
     MOTH_BEGIN_INSTR(Construct)
         STORE_IP();
+        STORE_ACC();
         acc = Runtime::method_construct(engine, STACK_VALUE(func), ACC, stack + argv, argc);
         CHECK_EXCEPTION;
     MOTH_END_INSTR(Construct)
 
     MOTH_BEGIN_INSTR(ConstructWithSpread)
         STORE_IP();
+        STORE_ACC();
         acc = Runtime::method_constructWithSpread(engine, STACK_VALUE(func), ACC, stack + argv, argc);
         CHECK_EXCEPTION;
     MOTH_END_INSTR(ConstructWithSpread)
@@ -820,7 +821,6 @@ QV4::ReturnedValue VME::interpret(CppStackFrame *frame, ExecutionEngine *engine,
     MOTH_BEGIN_INSTR(DeadTemporalZoneCheck)
         if (ACC.isEmpty()) {
             STORE_IP();
-            STORE_ACC();
             Runtime::method_throwReferenceError(engine, name);
             goto handleUnwind;
         }
@@ -989,6 +989,7 @@ QV4::ReturnedValue VME::interpret(CppStackFrame *frame, ExecutionEngine *engine,
             if (t->isNullOrUndefined()) {
                 *t = engine->globalObject->asReturnedValue();
             } else {
+                STORE_ACC();
                 *t = t->toObject(engine)->asReturnedValue();
                 CHECK_EXCEPTION;
             }
@@ -1001,6 +1002,7 @@ QV4::ReturnedValue VME::interpret(CppStackFrame *frame, ExecutionEngine *engine,
     MOTH_END_INSTR(LoadSuperConstructor)
 
     MOTH_BEGIN_INSTR(ToObject)
+        STORE_ACC();
         acc = ACC.toObject(engine)->asReturnedValue();
         CHECK_EXCEPTION;
     MOTH_END_INSTR(ToObject)