[PATCH 1/6] libgst: Do not flush the cache twice when adding a new method

install_method was flushing the cache twice. Once by calling
>>#at:put: on the MethodDictionary and once by install_method
itself. Only do that when the kernel is not initialized yet.

2013-12-22  Holger Hans Peter Freyther  <[hidden email]>

        * comp.c: Only flush the cache once when adding a new
        CompiledMethod.
---
 libgst/ChangeLog | 5 +++++
 libgst/comp.c    | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 03287b1..6e48372 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,10 @@
 2013-12-22  Holger Hans Peter Freyther  <[hidden email]>
 
+ * comp.c: Only flush the cache once when adding a new
+ CompiledMethod.
+
+2013-12-22  Holger Hans Peter Freyther  <[hidden email]>
+
  * interp.c: Do not flush the cache when creating a new
  process.
 
diff --git a/libgst/comp.c b/libgst/comp.c
index 0efcae1..040994c 100644
--- a/libgst/comp.c
+++ b/libgst/comp.c
@@ -2553,7 +2553,8 @@ install_method (OOP methodOOP, OOP classOOP)
 #ifdef VERIFY_COMPILED_METHODS
   _gst_verify_sent_method (methodOOP);
 #endif
-  _gst_invalidate_method_cache ();
+  if (!_gst_kernel_initialized)
+    _gst_invalidate_method_cache ();
 }
 
 OOP
--
1.8.5.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

This could be triggered with with the JIT and a simple test case
like this:

  p := [3 factorial] newProcess.
  p singleStep. p suspend.
  self assert: p isSuspended

2014-01-09  Holger Hans Peter Freyther  <[hidden email]>

        * interp.c: Check if the process is already suspended.
---
 libgst/ChangeLog | 4 ++++
 libgst/interp.c  | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 6e48372..954b58f 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,7 @@
+2014-01-09  Holger Hans Peter Freyther  <[hidden email]>
+
+ * interp.c: Check if the process is already suspended.
+
 2013-12-22  Holger Hans Peter Freyther  <[hidden email]>
 
  * comp.c: Only flush the cache once when adding a new
diff --git a/libgst/interp.c b/libgst/interp.c
index 0a01361..ab67daf 100644
--- a/libgst/interp.c
+++ b/libgst/interp.c
@@ -1794,7 +1794,8 @@ resume_process (OOP processOOP,
     {
       /* We're resuming a process with a *equal or higher* priority, so sleep
          the current one and activate the new one */
-      sleep_process (activeOOP);
+      if (active->myList != _gst_nil_oop)
+        sleep_process (activeOOP);
       activate_process (processOOP);
     }
   else
--
1.8.5.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

The primitive can fail and this is fixing the >>#testStepTooMuch
DebugTools testcase error.

2014-01-09  Holger Hans Peter Freyther  <[hidden email]>

        * prims.def: Mark VMpr_Process_singleStepWaitingOn as
        potentially failing.
---
 libgst/ChangeLog | 5 +++++
 libgst/prims.def | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 954b58f..4721c17 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,10 @@
 2014-01-09  Holger Hans Peter Freyther  <[hidden email]>
 
+ * prims.def: Mark VMpr_Process_singleStepWaitingOn as
+ potentially failing.
+
+2014-01-09  Holger Hans Peter Freyther  <[hidden email]>
+
  * interp.c: Check if the process is already suspended.
 
 2013-12-22  Holger Hans Peter Freyther  <[hidden email]>
diff --git a/libgst/prims.def b/libgst/prims.def
index 7e8eaaf..72add2b 100644
--- a/libgst/prims.def
+++ b/libgst/prims.def
@@ -2890,7 +2890,7 @@ primitive VMpr_Process_resume [succeed,fail,check_interrupt]
 }
 
 /* Process singleStepWaitingOn: */
-primitive VMpr_Process_singleStepWaitingOn [succeed]
+primitive VMpr_Process_singleStepWaitingOn [succeed,fail]
 {
   OOP oop1;
   OOP oop2;
--
1.8.5.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

This has been found with the DebugTools tests and the most simple
re-producer is:

  ([3+3] newProcess singleStep; suspend; suspendedContext) inspect.

The check for _gst_except_flag is triggerred in the method prologue
and the JIT returns to the interpreter loop. When the process is
switched an old _gst_ip was written to context->ipOffset. Modify the
emit_interrupt_check to take an optional ipOffset parameter that will
be stored when the interrupt check triggers. For primitives it is
not clear which ipOffset should be saved.

2013-12-23  Holger Hans Peter Freyther  <[hidden email]>

        * xlat.c: Add ipOffset parameter to emit_interrupt_check.
---
 libgst/ChangeLog |  4 ++++
 libgst/xlat.c    | 24 +++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 4721c17..113a0c3 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,7 @@
+2013-12-23  Holger Hans Peter Freyther  <[hidden email]>
+
+ * xlat.c: Add ipOffset parameter to emit_interrupt_check.
+
 2014-01-09  Holger Hans Peter Freyther  <[hidden email]>
 
  * prims.def: Mark VMpr_Process_singleStepWaitingOn as
diff --git a/libgst/xlat.c b/libgst/xlat.c
index 3f4a555..fbae8f7 100644
--- a/libgst/xlat.c
+++ b/libgst/xlat.c
@@ -332,7 +332,7 @@ static inline mst_Boolean emit_block_prolog (OOP blockOOP, gst_compiled_block bl
 static inline mst_Boolean emit_inlined_primitive (int primitive, int numArgs, int attr);
 static inline mst_Boolean emit_primitive (int primitive, int numArgs);
 
-static inline void emit_interrupt_check (int restartReg);
+static inline void emit_interrupt_check (int restartReg, int ipOffset);
 static inline void generate_run_time_code (void);
 static inline void translate_method (OOP methodOOP, OOP receiverClass, int size);
 static void emit_basic_size_in_r0 (OOP classOOP, mst_Boolean tagged, int objectReg);
@@ -2498,7 +2498,7 @@ emit_deferred_sends (deferred_send *ds)
 }
 
 void
-emit_interrupt_check (int restartReg)
+emit_interrupt_check (int restartReg, int ipOffset)
 {
   jit_insn *jmp, *begin;
 
@@ -2507,9 +2507,17 @@ emit_interrupt_check (int restartReg)
 
   jit_ldi_i (JIT_R2, &_gst_except_flag);
   jmp = jit_beqi_i (jit_forward (), JIT_R2, 0);
+
+  /* Save the global ip pointer */
+  if (ipOffset != -1)
+    {
+      jit_movi_ul (JIT_R2, ipOffset);
+      jit_sti_ul (&ip, JIT_R2);
+    }
+
+  /* Where to restart?*/
   if (restartReg == JIT_NOREG)
     jit_movi_p (JIT_RET, begin);
-
   else
     jit_movr_p (JIT_RET, restartReg);
 
@@ -2975,7 +2983,7 @@ emit_primitive (int primitive, int numArgs)
   if (attr & (PRIM_SUCCEED | PRIM_RELOAD_IP))
     {
       if (attr & PRIM_CHECK_INTERRUPT)
- emit_interrupt_check (JIT_V2);
+ emit_interrupt_check (JIT_V2, -1);
 
       jit_jmpr (JIT_V2);
     }
@@ -3218,7 +3226,7 @@ emit_method_prolog (OOP methodOOP,
   emit_context_setup (header.numArgs, header.numTemps);
 
   define_ip_map_entry (0);
-  emit_interrupt_check (JIT_NOREG);
+  emit_interrupt_check (JIT_NOREG, 0);
 
   /* For simplicity, we emit user-defined methods by creating a code_tree
      for the acrual send of #valueWithReceiver:withArguments: that they do.
@@ -3326,7 +3334,7 @@ emit_block_prolog (OOP blockOOP,
   emit_context_setup (header.numArgs, header.numTemps);
 
   define_ip_map_entry (0);
-  emit_interrupt_check (JIT_NOREG);
+  emit_interrupt_check (JIT_NOREG, 0);
 
   return (false);
 }
@@ -3675,9 +3683,7 @@ translate_method (OOP methodOOP, OOP receiverClass, int size)
   if (!lbl_define (*this_label))
     {
       define_ip_map_entry (bp - bc);
-      jit_movi_ul (JIT_V0, bp - bc);
-      jit_sti_ul (&ip, JIT_V0);
-      emit_interrupt_check (JIT_NOREG);
+      emit_interrupt_check (JIT_NOREG, bp - bc);
     }
  }
 
--
1.8.5.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

For single stepping through a method we should continue _after_ the
interrupt check or otherwise the interrupt check will _always_
trigger. This makes the DebugTools continue a bit.

2013-12-23  Holger Hans Peter Freyther  <[hidden email]>

        * xlat.c: Change emitted code in emit_interrupt_check.
---
 libgst/ChangeLog | 4 ++++
 libgst/xlat.c    | 7 ++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 113a0c3..05bc54b 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,9 @@
 2013-12-23  Holger Hans Peter Freyther  <[hidden email]>
 
+ * xlat.c: Change emitted code in emit_interrupt_check.
+
+2013-12-23  Holger Hans Peter Freyther  <[hidden email]>
+
  * xlat.c: Add ipOffset parameter to emit_interrupt_check.
 
 2014-01-09  Holger Hans Peter Freyther  <[hidden email]>
diff --git a/libgst/xlat.c b/libgst/xlat.c
index fbae8f7..9f45b33 100644
--- a/libgst/xlat.c
+++ b/libgst/xlat.c
@@ -2500,10 +2500,9 @@ emit_deferred_sends (deferred_send *ds)
 void
 emit_interrupt_check (int restartReg, int ipOffset)
 {
-  jit_insn *jmp, *begin;
+  jit_insn *jmp, *restart = NULL;
 
   jit_align (2);
-  begin = jit_get_label ();
 
   jit_ldi_i (JIT_R2, &_gst_except_flag);
   jmp = jit_beqi_i (jit_forward (), JIT_R2, 0);
@@ -2517,11 +2516,13 @@ emit_interrupt_check (int restartReg, int ipOffset)
 
   /* Where to restart?*/
   if (restartReg == JIT_NOREG)
-    jit_movi_p (JIT_RET, begin);
+    restart = jit_movi_p (JIT_RET, jit_forward());
   else
     jit_movr_p (JIT_RET, restartReg);
 
   jit_ret ();
+  if (restart)
+    jit_patch_movi (restart, jit_get_label());
   jit_patch (jmp);
 }
 
--
1.8.5.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

For the pointer we return it needs to look like a valid OOP. The
trick that was used to align the code to make that true. Otherwise
empty_context_stack will set a DUMMY_NATIVE_IP and we will resume
at the ipOffset (e.g. the beginning of a loop) and not after the
interrupt check.

This fixes something like this:

 a := nil. p := [a := 3] newProcess.
 [p isTerminated] whileFalse: [p singleStep; suspend]

2014-01-09  Holger Hans Peter Freyther  <[hidden email]>

        * xlat.c: Align the label correctly.
---
 libgst/ChangeLog | 4 ++++
 libgst/xlat.c    | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 05bc54b..528b6ae 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,7 @@
+2014-01-09  Holger Hans Peter Freyther  <[hidden email]>
+
+ * xlat.c: Align the label correctly.
+
 2013-12-23  Holger Hans Peter Freyther  <[hidden email]>
 
  * xlat.c: Change emitted code in emit_interrupt_check.
diff --git a/libgst/xlat.c b/libgst/xlat.c
index 9f45b33..260c7f6 100644
--- a/libgst/xlat.c
+++ b/libgst/xlat.c
@@ -2502,8 +2502,6 @@ emit_interrupt_check (int restartReg, int ipOffset)
 {
   jit_insn *jmp, *restart = NULL;
 
-  jit_align (2);
-
   jit_ldi_i (JIT_R2, &_gst_except_flag);
   jmp = jit_beqi_i (jit_forward (), JIT_R2, 0);
 
@@ -2514,6 +2512,8 @@ emit_interrupt_check (int restartReg, int ipOffset)
       jit_sti_ul (&ip, JIT_R2);
     }
 
+  jit_align (2);
+
   /* Where to restart?*/
   if (restartReg == JIT_NOREG)
     restart = jit_movi_p (JIT_RET, jit_forward());
--
1.8.5.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

Il 09/01/2014 19:09, Holger Hans Peter Freyther ha scritto:
Squash this in the previous patch, and we're good to go! :)

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Thu, Jan 09, 2014 at 07:11:36PM +0100, Paolo Bonzini wrote:

> Squash this in the previous patch, and we're good to go! :)

Yes, somehow I wanted to comment the knowledge I gained about
the requirements of the native_ip alignment. :}

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk