[m-rev.] for post commit review: catch attempts to mix paralellism and minimal model tabling

[Julien Fischer]

Hi Zoltan,

On Sun, 13 Jun 2021, Zoltan Somogyi wrote:

> For review by anyone.

> Generate diagnostics for .par.mm grades.
> 
> compiler/handle_options.m:
> runtime/mercury_grade.h:
> scripts/final_grade_options.sh-subr:
>     Generate an error message for grades that mix parallel execution
>     and minimal model tabling.
> 
> compiler/compute_grade.m:
>     Improve two comments.


> diff --git a/compiler/handle_options.m b/compiler/handle_options.m
> index 0b1f6ce61..a92227f17 100644
> --- a/compiler/handle_options.m
> +++ b/compiler/handle_options.m
> @@ -1641,8 +1641,6 @@ handle_minimal_model_options(!Globals, AllowHijacksMMSC, !Specs) :-
>          UseMinimalModelStackCopy),
>      globals.lookup_bool_option(!.Globals, use_minimal_model_own_stacks,
>          UseMinimalModelOwnStacks),
> -    bool.or(UseMinimalModelStackCopy, UseMinimalModelOwnStacks,
> -        UseMinimalModel),
>      % Minimal model tabling is not compatible with high level code
>      % or with trailing; see the comments in runtime/mercury_grade.h.

Does this comment also need to be updated?

>      ( if
> @@ -1653,25 +1651,46 @@ handle_minimal_model_options(!Globals, AllowHijacksMMSC, !Specs) :-
>              [words("You cannot use both forms of minimal model tabling"),
>              words("at once."), nl],

...

> diff --git a/runtime/mercury_grade.h b/runtime/mercury_grade.h
> index 4d874db4c..d328c4244 100644
> --- a/runtime/mercury_grade.h
> +++ b/runtime/mercury_grade.h
> @@ -290,6 +290,28 @@
>      #error "high level code and minimal model tabling are not compatible"
>    #endif
> 
> +  // Tabling is incompatible with multiple threads of execution for several
> +  // reasons. The main one is that when we find that the entry for a call's
> +  // input arguments in the called procedure's call table is already active
> +  // before the call, all forms of tabling treat it as a sign of infinite
> +  // recursion, as in e.g. f(42, ...) calling ... calling f(42, ...).
> +  // However, in a grade that allows more than thread of execution to be active

more than one

> +  // at the same time, the two f(42, ...) calls need not be related as

Delete the second "the" on that line.

> +  // ancestor and descendant; they could be independent calls in different
> +  // threads. This invalidates the basic assumption on top of which
> +  // tabling is built.
> +  // 
> +  // There are other reasons as well. The data structures used by tabling
> +  // are not protected by critical sections, so simultaneous access by more
> +  // than one thread at the same time can cause data corruption, and
> +  // in the process of suspending one call in one thread, the stack copy
> +  // implementation of minimal model tabling can actively overwrite parts
> +  // of the stack that are still being used by other threads.
> +
> +  #if defined(MR_THREAD_SAFE)
> +    #error "parallel execution and minimal model tabling are not compatible"
> +  #endif

That looks fine otherwise.

Julien.