[m-rev.] For review: Loop control runtime support.

Zoltan Somogyi zs at csse.unimelb.edu.au
Mon Sep 12 18:08:49 AEST 2011


On 12-Sep-2011, Paul Bone <pbone at csse.unimelb.edu.au> wrote:
> On Fri, Sep 09, 2011 at 04:04:46PM +1000, Paul Bone wrote:
> > 
> > For review by Zoltan.
> > 
> > Note that there are many XXXs marking opportunities for optimization, I expect
> > to revisit these and perhaps even the design (we discussed a different idea for
> > what to do when no worker is available some weeks ago).
> > 
> 
> I've committed this so that Zoltan can review it more eaisly.
> 

Post-commit review of Paul's change introducing the loop control primitives.
It also updates some documentation Paul's update did not touch.

library/par_buildin.m:
runtime/mercury_atomic_ops.h:
runtime/mercury_context.h:
	Fix formatting and grammar.

runtime/mercury_par_builtin.[ch]:
	Use a variable length array in the loop control struct to store
	the loop control slots. This setup needs one load to access a slot,
	compared to two with the previous arrangement.

	Fix formatting and grammar.

	Add XXXs where relevant.

Zoltan.

cvs diff: Diffing .
cvs diff: Diffing analysis
cvs diff: Diffing bindist
cvs diff: Diffing boehm_gc
cvs diff: Diffing boehm_gc/Mac_files
cvs diff: Diffing boehm_gc/cord
cvs diff: Diffing boehm_gc/cord/private
cvs diff: Diffing boehm_gc/doc
cvs diff: Diffing boehm_gc/extra
cvs diff: Diffing boehm_gc/include
cvs diff: Diffing boehm_gc/include/extra
cvs diff: Diffing boehm_gc/include/private
cvs diff: Diffing boehm_gc/libatomic_ops
cvs diff: Diffing boehm_gc/libatomic_ops/doc
cvs diff: Diffing boehm_gc/libatomic_ops/src
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/armcc
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/gcc
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/hpc
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/ibmc
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/icc
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/msftc
cvs diff: Diffing boehm_gc/libatomic_ops/src/atomic_ops/sysdeps/sunc
cvs diff: Diffing boehm_gc/libatomic_ops/tests
cvs diff: Diffing boehm_gc/libatomic_ops-1.2
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/doc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/tests
cvs diff: Diffing boehm_gc/m4
cvs diff: Diffing boehm_gc/tests
cvs diff: Diffing browser
cvs diff: Diffing bytecode
cvs diff: Diffing compiler
cvs diff: Diffing compiler/notes
cvs diff: Diffing deep_profiler
cvs diff: Diffing deep_profiler/notes
cvs diff: Diffing doc
cvs diff: Diffing extras
cvs diff: Diffing extras/base64
cvs diff: Diffing extras/cgi
cvs diff: Diffing extras/complex_numbers
cvs diff: Diffing extras/complex_numbers/samples
cvs diff: Diffing extras/complex_numbers/tests
cvs diff: Diffing extras/curs
cvs diff: Diffing extras/curs/samples
cvs diff: Diffing extras/curses
cvs diff: Diffing extras/curses/sample
cvs diff: Diffing extras/dynamic_linking
cvs diff: Diffing extras/error
cvs diff: Diffing extras/fixed
cvs diff: Diffing extras/gator
cvs diff: Diffing extras/gator/generations
cvs diff: Diffing extras/gator/generations/1
cvs diff: Diffing extras/graphics
cvs diff: Diffing extras/graphics/easyx
cvs diff: Diffing extras/graphics/easyx/samples
cvs diff: Diffing extras/graphics/mercury_allegro
cvs diff: Diffing extras/graphics/mercury_allegro/examples
cvs diff: Diffing extras/graphics/mercury_allegro/samples
cvs diff: Diffing extras/graphics/mercury_allegro/samples/demo
cvs diff: Diffing extras/graphics/mercury_allegro/samples/mandel
cvs diff: Diffing extras/graphics/mercury_allegro/samples/pendulum2
cvs diff: Diffing extras/graphics/mercury_allegro/samples/speed
cvs diff: Diffing extras/graphics/mercury_cairo
cvs diff: Diffing extras/graphics/mercury_cairo/samples
cvs diff: Diffing extras/graphics/mercury_cairo/samples/data
cvs diff: Diffing extras/graphics/mercury_cairo/tutorial
cvs diff: Diffing extras/graphics/mercury_glut
cvs diff: Diffing extras/graphics/mercury_opengl
cvs diff: Diffing extras/graphics/mercury_tcltk
cvs diff: Diffing extras/graphics/samples
cvs diff: Diffing extras/graphics/samples/calc
cvs diff: Diffing extras/graphics/samples/gears
cvs diff: Diffing extras/graphics/samples/maze
cvs diff: Diffing extras/graphics/samples/pent
cvs diff: Diffing extras/lazy_evaluation
cvs diff: Diffing extras/lex
cvs diff: Diffing extras/lex/samples
cvs diff: Diffing extras/lex/tests
cvs diff: Diffing extras/log4m
cvs diff: Diffing extras/logged_output
cvs diff: Diffing extras/monte
cvs diff: Diffing extras/moose
cvs diff: Diffing extras/moose/samples
cvs diff: Diffing extras/moose/tests
cvs diff: Diffing extras/mopenssl
cvs diff: Diffing extras/morphine
cvs diff: Diffing extras/morphine/non-regression-tests
cvs diff: Diffing extras/morphine/scripts
cvs diff: Diffing extras/morphine/source
cvs diff: Diffing extras/net
cvs diff: Diffing extras/odbc
cvs diff: Diffing extras/posix
cvs diff: Diffing extras/posix/samples
cvs diff: Diffing extras/quickcheck
cvs diff: Diffing extras/quickcheck/tutes
cvs diff: Diffing extras/references
cvs diff: Diffing extras/references/samples
cvs diff: Diffing extras/references/tests
cvs diff: Diffing extras/solver_types
cvs diff: Diffing extras/solver_types/library
cvs diff: Diffing extras/trailed_update
cvs diff: Diffing extras/trailed_update/samples
cvs diff: Diffing extras/trailed_update/tests
cvs diff: Diffing extras/windows_installer_generator
cvs diff: Diffing extras/windows_installer_generator/sample
cvs diff: Diffing extras/windows_installer_generator/sample/images
cvs diff: Diffing extras/xml
cvs diff: Diffing extras/xml/samples
cvs diff: Diffing extras/xml_stylesheets
cvs diff: Diffing java
cvs diff: Diffing java/runtime
cvs diff: Diffing library
Index: library/par_builtin.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/library/par_builtin.m,v
retrieving revision 1.27
diff -u -b -r1.27 par_builtin.m
--- library/par_builtin.m	12 Sep 2011 04:51:16 -0000	1.27
+++ library/par_builtin.m	12 Sep 2011 06:00:15 -0000
@@ -77,22 +77,24 @@
 
 :- type loop_control_slot.
 
-    % Create a loop control structure, see MR_lc_create in mercury_par_builtin.h
+    % Create a loop control structure.
+    % For documentation, see MR_lc_create in mercury_par_builtin.h.
     %
 :- pred lc_create(int::in, loop_control::out) is det.
 
-    % Finish a loop and finalize a loop control structure, see MR_lc_finish in
-    % mercury_par_builtin.h
-
+    % Finish a loop and finalize a loop control structure.
+    % For documentation, see MR_lc_finish in mercury_par_builtin.h.
+    %
 :- impure pred lc_finish(loop_control::in) is det.
 
     % Allocate a free slot from the loop control structure and return it.
-    % See MR_lc_try_get_free_slot in mercury_par_builtin.h
+    % For documentation, see MR_lc_try_get_free_slot in mercury_par_builtin.h
     %
-:- impure pred lc_free_slot(loop_control::in, loop_control_slot::out) is semidet.
+:- impure pred lc_free_slot(loop_control::in, loop_control_slot::out)
+    is semidet.
 
     % Finish one iteration of the loop.  This call does not return.
-    % See MR_lc_join_and_terminate in mercury_par_builtin.h
+    % For documentation, see MR_lc_join_and_terminate in mercury_par_builtin.h.
     %
 :- impure pred lc_join_and_terminate(loop_control::in, loop_control_slot::in)
     is det.
@@ -285,7 +287,6 @@
 :- pragma foreign_type("Java", loop_control_slot, "java.lang.Object").
 
 :- pragma foreign_proc("C",
-
     lc_create(NumWorkers::in, LC::out),
     [will_not_call_mercury, will_not_throw_exception, thread_safe,
      promise_pure],
@@ -297,14 +298,12 @@
 #endif
 ").
 
-
-%
 % IMPORTANT: any changes or additions to external predicates should be
-% reflected in the definition of pred_is_external in
-% mdbcomp/program_representation.m.  The debugger needs to know what predicates
+% reflected in the definition of pred_is_external in mdbcomp/
+% program_representation.m.  The debugger needs to know what predicates
 % are defined externally, so that it knows not to expect events for those
 % predicates.
-%
+
 :- external(lc_finish/1).
 
 :- pragma foreign_code("C",
@@ -342,7 +341,9 @@
 
 #if defined(MR_THREAD_SAFE) && defined(MR_LL_PARALLEL_CONJ)
     {
-        MR_LoopControl* LC = (MR_LoopControl*)MR_r1;
+        MR_LoopControl  *LC;
+        
+        LC = (MR_LoopControl *) MR_r1;
         MR_lc_finish_part1(LC, par_builtin__lc_finish_1_0_i1);
     }
 #else
@@ -354,7 +355,9 @@
 
 #if defined(MR_THREAD_SAFE) && defined(MR_LL_PARALLEL_CONJ)
     {
-        MR_LoopControl* LC = (MR_LoopControl*)MR_sv(1);
+        MR_LoopControl  *LC;
+        
+        LC = (MR_LoopControl *) MR_sv(1);
         MR_lc_finish_part2(LC);
     }
 #endif
cvs diff: Diffing mdbcomp
cvs diff: Diffing profiler
cvs diff: Diffing robdd
cvs diff: Diffing runtime
Index: runtime/mercury_atomic_ops.h
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_atomic_ops.h,v
retrieving revision 1.22
diff -u -b -r1.22 mercury_atomic_ops.h
--- runtime/mercury_atomic_ops.h	26 Aug 2011 14:10:04 -0000	1.22
+++ runtime/mercury_atomic_ops.h	12 Sep 2011 06:58:32 -0000
@@ -575,8 +575,8 @@
     !defined(MR_AVOID_HANDWRITTEN_ASSEMBLER)
 
     /*
-    ** Guarantees that any stores executed before this fence are globally
-    ** visible before those after this fence.
+    ** Guarantees that any stores executed before this fence are
+    ** globally visible before those after this fence.
     */
     #define MR_CPU_SFENCE                                                   \
         do {                                                                \
@@ -584,8 +584,8 @@
         } while(0)
 
     /*
-    ** Guarantees that any loads executed before this fence are complete before
-    ** any loads after this fence.
+    ** Guarantees that any loads executed before this fence are complete
+    ** before any loads after this fence.
     */
     #define MR_CPU_LFENCE                                                   \
         do {                                                                \
@@ -615,8 +615,8 @@
 
 #else
 
-    #pragma error "Please implement memory fence operations for this " \
-        "compiler/architecture"
+    #pragma error "Please implement memory fence operations "               \
+        "for this compiler/architecture"
 
 #endif
 
@@ -630,8 +630,8 @@
 ** Roll our own cheap user-space mutual exclusion locks.  Blocking without
 ** spinning is not supported.  Storage for these locks should be volatile.
 **
-** I expect these to be faster than pthread mutexes when threads are pinned and
-** critical sections are short.
+** I expect these to be faster than pthread mutexes when threads are pinned
+** and critical sections are short.
 */
 typedef MR_Unsigned MR_Us_Lock;
 
Index: runtime/mercury_context.c
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_context.c,v
retrieving revision 1.98
diff -u -b -r1.98 mercury_context.c
--- runtime/mercury_context.c	12 Sep 2011 04:51:17 -0000	1.98
+++ runtime/mercury_context.c	12 Sep 2011 06:17:05 -0000
@@ -217,10 +217,10 @@
 
 #ifdef MR_LL_PARALLEL_CONJ
 /*
-** Try to wake up a sleeping message and tell it to do action.  The engine is
-** only woken if the engine is in one of the states in the bitfield states.  If
-** the engine is woekn the result of this function is MR_TRUE, otherwise it's
-** MR_FALSE.
+** Try to wake up a sleeping message and tell it to do action. The engine
+** is only woken if the engine is in one of the states in the bitfield states.
+** If the engine is woken, this function returns MR_TRUE, otherwise it
+** returns MR_FALSE.
 */
 static MR_bool
 try_wake_engine(MR_EngineId engine_id, int action,
@@ -284,15 +284,15 @@
         } else {
             MR_num_threads = result;
             /*
-            ** On systems that don't support sched_setaffinity we don't try to
-            ** automatically enable thread pinning. This prevents a runtime
+            ** On systems that don't support sched_setaffinity, we don't try
+            ** to automatically enable thread pinning. This prevents a runtime
             ** warning that could unnecessarily confuse the user.
             **/
           #if defined(MR_LL_PARALLEL_CONJ) && \
               defined(MR_HAVE_SCHED_SETAFFINITY)
             /*
-            ** Comment this back in to enable thread pinning by default if we
-            ** autodetected the correct number of CPUs.
+            ** Comment this back in to enable thread pinning by default
+            ** if we autodetected the correct number of CPUs.
             */
             /* MR_thread_pinning = MR_TRUE; */
           #endif
@@ -316,7 +316,7 @@
         sem_init(&(engine_sleep_sync_data[i].d.es_wake_semaphore), 0, 1);
         /*
         ** All engines are initially working (because telling them to wake up
-        ** before they're started would be useless.
+        ** before they are started would be useless).
         */
         engine_sleep_sync_data[i].d.es_state = ENGINE_STATE_WORKING;
         engine_sleep_sync_data[i].d.es_action = MR_ENGINE_ACTION_NONE;
@@ -326,8 +326,8 @@
 }
 
 /*
-** Pin the primordial thread first to the CPU it is currently using (where
-** support is available).
+** Pin the primordial thread first to the CPU it is currently using
+** (if support is available for thread pinning).
 */
 #if defined(MR_THREAD_SAFE) && defined(MR_LL_PARALLEL_CONJ)
 unsigned
@@ -376,8 +376,9 @@
     unsigned cpu;
 
     /*
-    ** We go through the motions of thread pinning even when thread pinning is
-    ** not supported as the allocation of CPUs to threads may be used later.
+    ** We go through the motions of thread pinning even when thread pinning
+    ** is not supported, as the allocation of CPUs to threads may be
+    ** used later.
     */
     MR_LOCK(&MR_next_cpu_lock, "MR_pin_thread");
     if (MR_next_cpu == MR_primordial_thread_cpu) {
@@ -411,8 +412,8 @@
         if (sched_setaffinity(0, sizeof(cpu_set_t), &cpus) == -1) {
             perror("Warning: Couldn't set CPU affinity: ");
             /*
-            ** If this failed once, it will probably fail again, so we
-            ** disable it.
+            ** If this failed once, it will probably fail again,
+            ** so we disable it.
             */
             MR_thread_pinning = MR_FALSE;
         }
@@ -451,8 +452,8 @@
 **
 ** This writes out a flat text file which may be parsed by a machine or easily
 ** read by a human. There is no advantage in using a binary format since we
-** do this once at the end of execution and it's a small amount of data.
-** Therefore a text file is used since it has the advantage of being human
+** do this once at the end of execution and it is a small amount of data.
+** Therefore we use a text file, since it has the advantage of being human
 ** readable.
 */
 
@@ -642,11 +643,13 @@
 #endif
     if (c->MR_ctxt_nondetstack_zone == NULL) {
         if (gen != NULL) {
-            c->MR_ctxt_nondetstack_zone = MR_create_or_reuse_zone("gen_nondetstack",
+            c->MR_ctxt_nondetstack_zone =
+                MR_create_or_reuse_zone("gen_nondetstack",
                     MR_gen_nondetstack_size, MR_next_offset(),
                     MR_gen_nondetstack_zone_size, MR_default_handler);
         } else {
-            c->MR_ctxt_nondetstack_zone = MR_create_or_reuse_zone(nondetstack_name,
+            c->MR_ctxt_nondetstack_zone =
+                MR_create_or_reuse_zone(nondetstack_name,
                     nondetstack_size, MR_next_offset(),
                     MR_nondetstack_zone_size, MR_default_handler);
         }
@@ -845,7 +848,7 @@
 #endif
 
     /*
-    ** Save the context first, even though we're not saving a computation
+    ** Save the context first, even though we are not saving a computation
     ** that's in progress we are saving some bookkeeping information.
     */
     /*
@@ -937,14 +940,16 @@
     while (cur != NULL) {
 #ifdef MR_DEBUG_THREADS
         if (MR_debug_threads) {
-            fprintf(stderr, "%ld Eng: %d, c_depth: %d, Considering context %p\n",
+            fprintf(stderr,
+                "%ld Eng: %d, c_depth: %d, Considering context %p\n",
                 MR_SELF_THREAD_ID, engine_id, depth, cur);
         }
 #endif
         if (cur->MR_ctxt_resume_engine_required == MR_TRUE) {
 #ifdef MR_DEBUG_THREADS
             if (MR_debug_threads) {
-                fprintf(stderr, "%ld Context requires engine %d and c_depth %d\n",
+                fprintf(stderr,
+                    "%ld Context requires engine %d and c_depth %d\n",
                     MR_SELF_THREAD_ID, cur->MR_ctxt_resume_owner_engine,
                     cur->MR_ctxt_resume_c_depth);
             }
@@ -1101,10 +1106,10 @@
 }
 
 /*
-** Check to see if any contexts that blocked on IO have become
-** runnable. Return the number of contexts that are still blocked.
-** The parameter specifies whether or not the call to select should
-** block or not.
+** Check to see if any contexts that blocked on IO have become runnable.
+** Return the number of contexts that are still blocked.
+** The parameter specifies whether or not the call to select should block
+** or not.
 */
 
 static int
@@ -1239,13 +1244,13 @@
             &wake_action_data, ENGINE_STATE_IDLE | ENGINE_STATE_SLEEPING))
         {
             /*
-            ** We've successfully given the context to the correct engine.
+            ** We have successfully given the context to the correct engine.
             */
             return;
         }
     } else {
         /*
-        ** If there is some idle engine try to wake it up, starting with the
+        ** If there is some idle engine, try to wake it up, starting with the
         ** preferred engine.
         */
         if (MR_num_idle_engines > 0) {
@@ -1253,7 +1258,7 @@
                 &wake_action_data, NULL))
             {
                 /*
-                ** The context has been given to a engine.
+                ** The context has been given to an engine.
                 */
                 return;
             }
@@ -1275,7 +1280,7 @@
 
 #ifdef MR_LL_PARALLEL_CONJ
 /*
-** Try to wake an engine, starting at the preferred engine
+** Try to wake an engine, starting at the preferred engine.
 */
 MR_bool
 MR_try_wake_an_engine(MR_EngineId preferred_engine, int action,
@@ -1294,7 +1299,7 @@
         current_engine = (i + preferred_engine) % MR_num_threads;
         if (current_engine == MR_ENGINE(MR_eng_id)) {
             /*
-            ** Don't post superfluous events to ourself
+            ** Don't post superfluous events to ourself.
             */
             continue;
         }
@@ -1331,8 +1336,8 @@
         /*
         ** We now KNOW that the engine is in one of the correct states.
         **
-        ** We tell the engine what to do, and tell others that we've woken
-        ** it before actually waking it.
+        ** We tell the engine what to do, and tell others that we have woken it
+        ** before actually waking it.
         */
         esync->d.es_action = action;
         if (action_data) {
@@ -1340,7 +1345,8 @@
         }
         esync->d.es_state = ENGINE_STATE_WOKEN;
         MR_CPU_SFENCE;
-        MR_SEM_POST(&(esync->d.es_sleep_semaphore), "try_wake_engine sleep_sem");
+        MR_SEM_POST(&(esync->d.es_sleep_semaphore),
+            "try_wake_engine sleep_sem");
         success = MR_TRUE;
     }
     MR_SEM_POST(&(esync->d.es_wake_semaphore), "try_wake_engine wake_sem");
@@ -1380,13 +1386,13 @@
 */
 
 /*
-** The run queue used to include timing code, it's been removed and may be
+** The run queue used to include timing code. It has been removed and may be
 ** added in the future.
 */
 
 /*
 ** If the call returns a non-null code pointer then jump to that address,
-** otherwise fall-through
+** otherwise fall-through.
 */
 #define MR_MAYBE_TRAMPOLINE_AND_ACTION(call, action)                        \
     do {                                                                    \
@@ -1436,8 +1442,8 @@
 prepare_engine_for_context(MR_Context *context);
 
 /*
-** Advertise that the engine is looking for work after being in the working state.
-** (Do not use this call when waking from sleep).
+** Advertise that the engine is looking for work after being in the
+** working state. (Do not use this call when waking from sleep).
 */
 static void
 advertise_engine_state_idle(void);
@@ -1471,7 +1477,7 @@
   #else /* !MR_THREAD_SAFE */
 {
     /*
-    ** When an engine becomes idle in a non parallel grade it simply picks up
+    ** When an engine becomes idle in a non parallel grade, it simply picks up
     ** another context.
     */
     if (MR_runqueue_head == NULL && MR_pending_contexts == NULL) {
@@ -1509,7 +1515,7 @@
     MR_MAYBE_TRAMPOLINE(do_work_steal(NULL));
 
     /*
-     * XXX: I'm not convinced that the idle state is useful.
+    ** XXX: I'm not convinced that the idle state is useful.
      */
     advertise_engine_state_idle();
 
@@ -1536,12 +1542,13 @@
         MR_MAYBE_TRAMPOLINE(do_get_context());
 
         /*
-        ** If execution reaches this point then we lost a race for a free context.
-        ** Tell threadscope about it,
+        ** If execution reaches this point, then we lost a race for
+        ** a free context. Tell threadscope about it,
         ** XXX: Consider making this a first-class event in the future.
         */
 #ifdef MR_THREADSCOPE
-        MR_threadscope_post_log_msg("Runtime: Lost a race for a runnable context.");
+        MR_threadscope_post_log_msg(
+            "Runtime: Lost a race for a runnable context.");
 #endif
         join_label = NULL;
     }
@@ -1553,8 +1560,8 @@
     MR_MAYBE_TRAMPOLINE(do_work_steal(join_label));
     if (join_label != NULL) {
         /*
-        ** Save the dirty context, we can't take it to sleep and it won't be used
-        ** if do_get_context() succeeds.
+        ** Save the dirty context. We cannot take it to sleep and
+        ** it won't be used if do_get_context() succeeds.
         */
         save_dirty_context(join_label);
         MR_ENGINE(MR_eng_this_context) = NULL;
@@ -1600,7 +1607,8 @@
             action = engine_sleep_sync_data[engine_id].d.es_action;
 #ifdef MR_DEBUG_THREADS
             if (MR_debug_threads) {
-                fprintf(stderr, "%ld Engine %d is awake and will do action %d\n",
+                fprintf(stderr,
+                    "%ld Engine %d is awake and will do action %d\n",
                     MR_SELF_THREAD_ID, engine_id, action);
             }
 #endif
@@ -1614,7 +1622,8 @@
                     assert(engine_id != 0);
                     MR_atomic_dec_int(&MR_num_idle_engines);
                     MR_destroy_thread(MR_cur_engine());
-                    MR_SEM_POST(&shutdown_semaphore, "MR_do_sleep shutdown_sem");
+                    MR_SEM_POST(&shutdown_semaphore,
+                        "MR_do_sleep shutdown_sem");
                     pthread_exit(0);
                     break;
 
@@ -1655,7 +1664,7 @@
             }
         } else {
             /*
-            ** sem_wait reported an error
+            ** Sem_wait reported an error.
             */
             switch (errno) {
                 case EINTR:
@@ -1715,7 +1724,7 @@
 static void
 prepare_engine_for_context(MR_Context *context) {
     /*
-    ** Discard whatever unused context we may have and switch to the new one.
+    ** Discard whatever unused context we may have, and switch to the new one.
     */
     if (MR_ENGINE(MR_eng_this_context) != NULL) {
     #ifdef MR_DEBUG_STACK_SEGMENTS
@@ -1785,8 +1794,8 @@
 #endif
 
     /*
-    ** At this point we have a context, either a dirty context that's
-    ** compatbile or a clean one.
+    ** At this point we have a context, either a dirty context that is
+    ** compatible, or a clean one.
     */
     MR_parent_sp = spark->MR_spark_sync_term->MR_st_parent_sp;
     MR_SET_THREAD_LOCAL_MUTABLES(spark->MR_spark_thread_local_mutables);
@@ -1856,8 +1865,8 @@
     this_context->MR_ctxt_resume_owner_engine = MR_ENGINE(MR_eng_id);
     MR_save_context(this_context);
     /*
-    ** Make sure the context gets saved before we set the join
-    ** label, use a memory barrier.
+    ** Make sure the context gets saved before we set the join label,
+    ** use a memory barrier.
     */
     MR_CPU_SFENCE;
     this_context->MR_ctxt_resume = join_label;
@@ -1898,7 +1907,7 @@
 
     /*
     ** Atomically decrement and fetch the number of conjuncts yet to complete.
-    ** If we're the last conjunct to complete (the parallel conjunction is
+    ** If we are the last conjunct to complete (the parallel conjunction is
     ** finished) then jnc_last will be true.
     */
     /*
@@ -1915,7 +1924,7 @@
         */
         if (jnc_last) {
             /*
-            ** All the conjuncts have finished so jump to the join label.
+            ** All the conjuncts have finished, so jump to the join label.
             */
             return join_label;
         } else {
@@ -1936,7 +1945,7 @@
 #endif
             /*
             ** This context didn't originate this parallel conjunction and
-            ** we're the last branch to finish. The originating context should
+            ** we are the last branch to finish. The originating context should
             ** be suspended waiting for us to finish, we should run it using
             ** the current engine.
             **
@@ -1963,8 +1972,8 @@
 #ifdef MR_LL_PARALLEL_CONJ
 
 /*
- * Debugging functions for runtime granularity control.
- */
+** Debugging functions for runtime granularity control.
+*/
 
 #ifdef MR_DEBUG_RUNTIME_GRANULARITY_CONTROL
 
Index: runtime/mercury_par_builtin.c
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_par_builtin.c,v
retrieving revision 1.2
diff -u -b -r1.2 mercury_par_builtin.c
--- runtime/mercury_par_builtin.c	12 Sep 2011 04:51:17 -0000	1.2
+++ runtime/mercury_par_builtin.c	12 Sep 2011 07:44:27 -0000
@@ -40,15 +40,15 @@
 
 #if defined(MR_THREAD_SAFE) && defined(MR_LL_PARALLEL_CONJ)
 
-MR_LoopControl* MR_lc_create(unsigned num_workers)
+MR_LoopControl *
+MR_lc_create(unsigned num_workers)
 {
     MR_LoopControl* lc;
     unsigned        i;
 
-    lc = MR_GC_malloc(sizeof(MR_LoopControl));
-    lc->MR_lc_slots = MR_GC_malloc(sizeof(MR_LoopControlSlot) * num_workers);
-    for (i = 0; i < num_workers; i++)
-    {
+    lc = MR_GC_malloc(sizeof(MR_LoopControl) +
+        (num_workers-1) * sizeof(MR_LoopControlSlot));
+    for (i = 0; i < num_workers; i++) {
         /*
         ** We allocate contexts as necessary, so that we never allocate a
         ** context we don't use.  Also by delaying the allocation of contexts
@@ -65,7 +65,8 @@
     return lc;
 }
 
-MR_LoopControlSlot* MR_lc_try_get_free_slot(MR_LoopControl* lc)
+MR_LoopControlSlot *
+MR_lc_try_get_free_slot(MR_LoopControl* lc)
 {
     if (lc->MR_lc_outstanding_workers == lc->MR_lc_num_slots) {
         return NULL;
@@ -76,7 +77,7 @@
         ** Optimize this by using a hint to start the search at.
         */
         for (i = 0; i<lc->MR_lc_num_slots; i++) {
-            if (lc->MR_lc_slots[i].MR_lcs_is_free == MR_TRUE) {
+            if (lc->MR_lc_slots[i].MR_lcs_is_free) {
                 lc->MR_lc_slots[i].MR_lcs_is_free = MR_FALSE;
                 MR_atomic_inc_int(&(lc->MR_lc_outstanding_workers));
                 return &(lc->MR_lc_slots[i]);
@@ -87,7 +88,8 @@
     }
 }
 
-void MR_lc_spawn_off_(MR_LoopControlSlot* lcs, MR_Code* code_ptr)
+void
+MR_lc_spawn_off_func(MR_LoopControlSlot* lcs, MR_Code* code_ptr)
 {
     if (lcs->MR_lcs_context == NULL) {
         /*
@@ -95,7 +97,8 @@
         */
         lcs->MR_lcs_context = MR_create_context("Loop control",
             MR_CONTEXT_SIZE_FOR_LOOP_CONTROL_WORKER, NULL);
-        lcs->MR_lcs_context->MR_ctxt_thread_local_mutables = MR_THREAD_LOCAL_MUTABLES;
+        lcs->MR_lcs_context->MR_ctxt_thread_local_mutables =
+            MR_THREAD_LOCAL_MUTABLES;
     }
 
     lcs->MR_lcs_context->MR_ctxt_resume = code_ptr;
@@ -103,20 +106,23 @@
     MR_schedule_context(lcs->MR_lcs_context);
 }
 
-void MR_lc_join(MR_LoopControl* lc, MR_LoopControlSlot* lcs)
+void
+MR_lc_join(MR_LoopControl* lc, MR_LoopControlSlot* lcs)
 {
     MR_bool     last_worker;
     MR_Context  *wakeup_context;
 
     lcs->MR_lcs_is_free = MR_TRUE;
-    last_worker = MR_atomic_dec_and_is_zero_int(&(lc->MR_lc_outstanding_workers));
+    last_worker =
+        MR_atomic_dec_and_is_zero_int(&(lc->MR_lc_outstanding_workers));
+
     /*
-    ** This barrier ensures we update MR_lc_outstanding_contexts before we read
-    ** MR_lc_finished, it works with another barrier in MR_lc_finish().
-    ** Together these barriers prevent a race where by the original thread is
-    ** not resumed because MR_lc_finished looked false in the condition below
-    ** but last_worker was true and the original thread is about to go to
-    ** sleep.
+    ** This barrier ensures we update MR_lc_outstanding_contexts before
+    ** we read MR_lc_finished. It works together with another barrier
+    ** in MR_lc_finish(). Together these barriers prevent a race whereby
+    ** the original thread is not resumed because MR_lc_finished looked false
+    ** in the condition below but last_worker was true, and the original
+    ** thread is about to go to sleep.
     **
     ** We go through these checks to avoid taking the lock in the then branch
     ** below in cases when MR_lc_outstanding_workers is zero but the original
@@ -126,20 +132,21 @@
     if (last_worker && lc->MR_lc_finished) {
         /*
         ** Wake up the first thread if it is sleeping.
-        ** XXX: a spinlock would do here, or maybe a CAS, we never hold the lock for long.
+        ** XXX: a spinlock would do here, or maybe a CAS;
+        ** we never hold the lock for long.
         */
         MR_LOCK(&(lc->MR_lc_lock), "MC_lc_join_and_terminate");
         wakeup_context = lc->MR_lc_waiting_context;
         /*
-        ** We don't need to clear the context field at this point, only one
-        ** worker can ever be the last worker and therefore there's no danger
+        ** We don't need to clear the context field at this point: only one
+        ** worker can ever be the last worker, and therefore there is no danger
         ** in adding this context to the run queue twice.
         */
         MR_UNLOCK(&(lc->MR_lc_lock), "MR_lc_join_and_terminate");
         if (wakeup_context != NULL) {
             /*
-            ** XXX: it's faster to switch to this context ourselves since we're
-            ** going to unload our own context.
+            ** XXX: it is faster to switch to this context ourselves
+            ** since we are going to unload our own context.
             */
             MR_schedule_context(wakeup_context);
         }
Index: runtime/mercury_par_builtin.h
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_par_builtin.h,v
retrieving revision 1.7
diff -u -b -r1.7 mercury_par_builtin.h
--- runtime/mercury_par_builtin.h	12 Sep 2011 04:51:17 -0000	1.7
+++ runtime/mercury_par_builtin.h	12 Sep 2011 07:44:03 -0000
@@ -52,7 +52,6 @@
 
 #endif /* !MR_THREAD_SAFE */
 
-
 /*
 ** The mutex needs to be destroyed when the future is garbage collected.
 ** For efficiency we might want to ignore this altogether, e.g. on Linux
@@ -115,10 +114,10 @@
   #endif /* ! MR_THREADSCOPE */
 
     /*
-    ** If MR_fut_signalled is true then we guarantee that reading MR_fut_value
-    ** is safe, even without a lock, see the corresponding code in
-    ** MR_par_builtin_signal_future();
-    ** If MR_fut_signalled is false then we do take a lock and re-read the
+    ** If MR_fut_signalled is true, then we guarantee that reading MR_fut_value
+    ** is safe, even without a lock; see the corresponding code in
+    ** MR_par_builtin_signal_future().
+    ** If MR_fut_signalled is false, then we do take a lock and re-read
     ** this value (to ensure there was not a race).
     */
 
@@ -188,7 +187,7 @@
                                                                             \
             /*                                                              \
             ** Post the threadscope signal future message before waking any \
-            ** threads (and posting those messages.                         \
+            ** threads (and posting those messages).                        \
             */                                                              \
             MR_maybe_post_signal_future(Future);                            \
             MR_LOCK(&(Future->MR_fut_lock), "future.signal");               \
@@ -293,30 +292,29 @@
 #if defined(MR_THREAD_SAFE) && defined(MR_LL_PARALLEL_CONJ)
 
 typedef struct MR_LoopControl_Struct        MR_LoopControl;
-
 typedef struct MR_LoopControlSlot_Struct    MR_LoopControlSlot;
 
+struct MR_LoopControlSlot_Struct
+{
+    MR_Context                              *MR_lcs_context;
+    MR_bool                                 MR_lcs_is_free;
+};
+
 struct MR_LoopControl_Struct
 {
-    MR_LoopControlSlot*                 MR_lc_slots;
     unsigned                            MR_lc_num_slots;
     MR_THREADSAFE_VOLATILE MR_Integer   MR_lc_outstanding_workers;
-    MR_Context*                         MR_lc_waiting_context;
+    MR_Context                              *MR_lc_waiting_context;
     MR_THREADSAFE_VOLATILE MR_bool      MR_lc_finished;
     MercuryLock                         MR_lc_lock;
-};
-
-struct MR_LoopControlSlot_Struct
-{
-    MR_Context*         MR_lcs_context;
-    MR_bool             MR_lcs_is_free;
+    MR_LoopControlSlot                      MR_lc_slots[1];
 };
 
 #else
 
 /*
-** We have to define these types so that par_builtin.m can use them as foreign
-** types, even in grades that don't support them.
+** We have to define these types so that par_builtin.m can use them
+** as foreign types, even in grades that do not support them.
 */
 
 typedef MR_Word MR_LoopControl;
@@ -327,41 +325,42 @@
 #if defined(MR_THREAD_SAFE) && defined(MR_LL_PARALLEL_CONJ)
 
 /*
-** XXX: Make these functions macros, they're functions now to make debugging
+** XXX: Make these functions macros, they are now functions to make debugging
 ** and testing easier.
 */
 
 /*
 ** Create and initialize a loop control structure.
 */
-extern MR_LoopControl* MR_lc_create(unsigned num_workers);
+extern MR_LoopControl   *MR_lc_create(unsigned num_workers);
 
 /*
-** Wait for all workers and then finalize and free the loop control structure,
+** Wait for all workers, and then finalize and free the loop control structure.
 ** The caller must pass a module-unique (unquoted) string for resume_point_name
 ** that will be used in the name for a C label.
 */
-#define MR_lc_finish_part1(lc, label)                                       \
+#define MR_lc_finish_part1(lc, part2_label)                                 \
     do {                                                                    \
         (lc)->MR_lc_finished = MR_TRUE;                                     \
         /*                                                                  \
-        ** This barrier ensures that MR_lc_finished before we read          \
-        ** MR_lc_outstanding_contexts, it works with another barrier in     \
+        ** This barrier ensures that MR_lc_finished has been set to MR_TRUE \
+        ** before we read MR_lc_outstanding_contexts.
+        ** it works with another barrier in     \
         ** MR_lc_join_and_terminate().  See MR_lc_join_and_terminate().     \
         */                                                                  \
         MR_CPU_MFENCE;                                                      \
         if ((lc)->MR_lc_outstanding_workers > 0) {                          \
             /*                                                              \
             ** This context must wait until the workers are finished.       \
-            ** This must be implemented as a macro, we cannot move the C    \
-            ** stack pointer without extra work.                            \
+            ** This must be implemented as a macro, since we cannot move    \
+            ** the C stack pointer without extra work.                      \
             */                                                              \
             MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =                \
-                MR_LABEL(MR_add_prefix(label));                             \
+                MR_LABEL(MR_add_prefix(part2_label));                       \
             MR_LOCK(&((lc)->MR_lc_lock), "MR_lc_finish_part1");             \
             if ((lc)->MR_lc_outstanding_workers == 0) {                     \
                 MR_UNLOCK(&((lc)->MR_lc_lock), "MR_lc_finish_part1");       \
-                MR_GOTO_LOCAL(MR_add_prefix(label));                        \
+                MR_GOTO_LOCAL(MR_add_prefix(part2_label));                  \
             }                                                               \
             MR_save_context(MR_ENGINE(MR_eng_this_context));                \
             (lc)->MR_lc_waiting_context = MR_ENGINE(MR_eng_this_context);   \
@@ -376,7 +375,7 @@
         unsigned i;                                                         \
                                                                             \
         /*                                                                  \
-        ** All the jobs have finished,                                      \
+        ** All the jobs have finished.                                      \
         */                                                                  \
         for (i = 0; i < (lc)->MR_lc_num_slots; i++) {                       \
             if ((lc)->MR_lc_slots[i].MR_lcs_context != NULL) {              \
@@ -395,12 +394,12 @@
 ** Try to spawn off this code using the free slot.
 */
 #define MR_lc_spawn_off(lcs, label) \
-    MR_lc_spawn_off_((lcs), MR_LABEL(MR_add_prefix(label)))
+    MR_lc_spawn_off_func((lcs), MR_LABEL(MR_add_prefix(label)))
 
-extern void MR_lc_spawn_off_(MR_LoopControlSlot* lcs, MR_Code* code_ptr);
+extern void MR_lc_spawn_off_func(MR_LoopControlSlot* lcs, MR_Code* code_ptr);
 
 /*
-** Join and termiante a worker.
+** Join and terminate a worker.
 */
 #define MR_lc_join_and_terminate(lc, lcs)                                   \
     do {                                                                    \
@@ -413,6 +412,7 @@
         ** computation, but we do so that we save bookkeeping information.  \
         ** A similar mistake was the cause of a hard-to-diagnose bug in     \
         ** parallel stack segments grades.                                  \
+        ** XXX give a pointer to a description of that bug.                 \
         */                                                                  \
         MR_save_context(MR_ENGINE(MR_eng_this_context));                    \
         MR_ENGINE(MR_eng_this_context) = NULL;                              \
cvs diff: Diffing runtime/GETOPT
cvs diff: Diffing runtime/machdeps
cvs diff: Diffing samples
cvs diff: Diffing samples/appengine
cvs diff: Diffing samples/appengine/war
cvs diff: Diffing samples/appengine/war/WEB-INF
cvs diff: Diffing samples/c_interface
cvs diff: Diffing samples/c_interface/c_calls_mercury
cvs diff: Diffing samples/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/mercury_calls_c
cvs diff: Diffing samples/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/standalone_c
cvs diff: Diffing samples/concurrency
cvs diff: Diffing samples/concurrency/dining_philosophers
cvs diff: Diffing samples/concurrency/midimon
cvs diff: Diffing samples/diff
cvs diff: Diffing samples/java_interface
cvs diff: Diffing samples/java_interface/java_calls_mercury
cvs diff: Diffing samples/java_interface/mercury_calls_java
cvs diff: Diffing samples/lazy_list
cvs diff: Diffing samples/muz
cvs diff: Diffing samples/rot13
cvs diff: Diffing samples/solutions
cvs diff: Diffing samples/solver_types
cvs diff: Diffing samples/tests
cvs diff: Diffing samples/tests/c_interface
cvs diff: Diffing samples/tests/c_interface/c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/tests/c_interface/mercury_calls_c
cvs diff: Diffing samples/tests/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/tests/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/tests/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/tests/diff
cvs diff: Diffing samples/tests/muz
cvs diff: Diffing samples/tests/rot13
cvs diff: Diffing samples/tests/solutions
cvs diff: Diffing samples/tests/toplevel
cvs diff: Diffing scripts
cvs diff: Diffing slice
cvs diff: Diffing ssdb
cvs diff: Diffing tests
cvs diff: Diffing tests/analysis
cvs diff: Diffing tests/analysis/ctgc
cvs diff: Diffing tests/analysis/excp
cvs diff: Diffing tests/analysis/ext
cvs diff: Diffing tests/analysis/sharing
cvs diff: Diffing tests/analysis/table
cvs diff: Diffing tests/analysis/trail
cvs diff: Diffing tests/analysis/unused_args
cvs diff: Diffing tests/benchmarks
cvs diff: Diffing tests/debugger
cvs diff: Diffing tests/debugger/declarative
cvs diff: Diffing tests/dppd
cvs diff: Diffing tests/general
cvs diff: Diffing tests/general/accumulator
cvs diff: Diffing tests/general/string_format
cvs diff: Diffing tests/general/structure_reuse
cvs diff: Diffing tests/grade_subdirs
cvs diff: Diffing tests/hard_coded
cvs diff: Diffing tests/hard_coded/exceptions
cvs diff: Diffing tests/hard_coded/purity
cvs diff: Diffing tests/hard_coded/sub-modules
cvs diff: Diffing tests/hard_coded/typeclasses
cvs diff: Diffing tests/invalid
cvs diff: Diffing tests/invalid/purity
cvs diff: Diffing tests/misc_tests
cvs diff: Diffing tests/mmc_make
cvs diff: Diffing tests/mmc_make/lib
cvs diff: Diffing tests/par_conj
cvs diff: Diffing tests/recompilation
cvs diff: Diffing tests/stm
cvs diff: Diffing tests/stm/orig
cvs diff: Diffing tests/stm/orig/stm-compiler
cvs diff: Diffing tests/stm/orig/stm-compiler/test1
cvs diff: Diffing tests/stm/orig/stm-compiler/test10
cvs diff: Diffing tests/stm/orig/stm-compiler/test2
cvs diff: Diffing tests/stm/orig/stm-compiler/test3
cvs diff: Diffing tests/stm/orig/stm-compiler/test4
cvs diff: Diffing tests/stm/orig/stm-compiler/test5
cvs diff: Diffing tests/stm/orig/stm-compiler/test6
cvs diff: Diffing tests/stm/orig/stm-compiler/test7
cvs diff: Diffing tests/stm/orig/stm-compiler/test8
cvs diff: Diffing tests/stm/orig/stm-compiler/test9
cvs diff: Diffing tests/stm/orig/stm-compiler-par
cvs diff: Diffing tests/stm/orig/stm-compiler-par/bm1
cvs diff: Diffing tests/stm/orig/stm-compiler-par/bm2
cvs diff: Diffing tests/stm/orig/stm-compiler-par/stmqueue
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test1
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test10
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test11
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test2
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test3
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test4
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test5
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test6
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test7
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test8
cvs diff: Diffing tests/stm/orig/stm-compiler-par/test9
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test1
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test2
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test3
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test4
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test5
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test6
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test7
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test8
cvs diff: Diffing tests/stm/orig/stm-compiler-par-asm_fast/test9
cvs diff: Diffing tests/tabling
cvs diff: Diffing tests/term
cvs diff: Diffing tests/trailing
cvs diff: Diffing tests/valid
cvs diff: Diffing tests/warnings
cvs diff: Diffing tools
cvs diff: Diffing trace
cvs diff: Diffing util
cvs diff: Diffing vim
cvs diff: Diffing vim/after
cvs diff: Diffing vim/ftplugin
cvs diff: Diffing vim/syntax
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------



More information about the reviews mailing list