[m-rev.] for review: Implement unify/compare of tuples in Mercury.

[Julien Fischer]

On Mon, 7 Jul 2014, Peter Wang wrote:

> Branches: master
>
> The hand-written C unify and compare predicates for tuples did not preserve
> deep profiler invariants correctly across the recursive unify/compare of
> tuple arguments.  I tried to do so, and failed.  Instead, implement the
> predicates in Mercury so the compiler can perform the deep profiling
> transformation on them.  Bug #3.
>
> A micro-benchmark on my machine is about twice as fast in asm_fast.gc
> after this patch, and equally fast in hlc.gc.  The change to the
> high-level C backend is only to reduce code duplication.

I doubt there is much performance critical code about that uses tuples
anyway.

> I seem to have hit an unrelated bug in compare_representation so I did
> not implement the compare_representation predicate for tuples yet.
>
> library/builtin.m:
> 	Add `unify_tuple' and `compare_tuple' predicates.
>
> 	Add module initialisation predicate which sets
> 	`MR_special_pred_hooks' to point to those predicates.
>
> 	Delete unused predicates `call_rtti_generic_unify',
> 	`call_rtti_generic_compare'.
>
> runtime/mercury_ho_call.c:
> runtime/mercury_ho_call.h:
> 	Add a global variable `MR_special_pred_hooks' for the library to
> 	set up during initialisation.
>
> 	Add `tailcall' macros for use by `mercury_unify_compare_body.h'.
> 	Rename `tailcall_user_pred' to `tailcall_tci_pred'.
>
> 	Call the new unify/compare predicates in the high-level C
> 	backend via `MR_special_pred_hooks'.
>
> 	Delete `unify_tuples' and `compare_tuples' for the high-level C
> 	and call the Mercury predicates set in `MR_special_pred_hooks'.
>
> runtime/mercury_unify_compare_body.h:
> 	Delete the unify and compare code for tuples in the low-level C
> 	backend.  Jump to the Mercury predicates set in
> 	`MR_special_pred_hooks' instead.
>
> 	Add some comments.
>
> tests/hard_coded/Mmakefile:
> tests/hard_coded/tuple_test2.exp
> tests/hard_coded/tuple_test2.m:
> 	Add test case.
>
> NEWS:
> 	Announce the change.
> ---
> NEWS                                 |   3 +
> library/builtin.m                    | 120 ++++++++++++++++++++++++++++++---
> runtime/mercury_ho_call.c            | 125 +++++++++++++++--------------------
> runtime/mercury_ho_call.h            |  20 ++++++
> runtime/mercury_unify_compare_body.h |  66 ++++++++----------
> tests/hard_coded/Mmakefile           |   1 +
> tests/hard_coded/tuple_test2.exp     |  11 +++
> tests/hard_coded/tuple_test2.m       |  66 ++++++++++++++++++
> 8 files changed, 294 insertions(+), 118 deletions(-)
> create mode 100644 tests/hard_coded/tuple_test2.exp
> create mode 100644 tests/hard_coded/tuple_test2.m
>
> diff --git a/NEWS b/NEWS
> index 4c0a461..a6f5798 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -29,6 +29,9 @@ Changes to the Mercury standard library:
>
> Changes to the Mercury compiler:
>
> +* We have fixed a long-standing bug causing crashes in deep profiling
> +  grades, related to unify/compare of tuples.  (Bug #3)

s/of tuples/for tuples/



> diff --git a/runtime/mercury_ho_call.c b/runtime/mercury_ho_call.c
> index f528925..3598e8f 100644
> --- a/runtime/mercury_ho_call.c
> +++ b/runtime/mercury_ho_call.c

...

> @@ -163,7 +117,10 @@ mercury__builtin__unify_2_p_0(MR_Mercury_Type_Info ti, MR_Box x, MR_Box y)
>     */
>     type_ctor_rep = MR_type_ctor_rep(type_ctor_info);
>     if (type_ctor_rep == MR_TYPECTOR_REP_TUPLE) {
> -        return unify_tuples(ti, (MR_Tuple) x, (MR_Tuple) y);
> +        if (MR_special_pred_hooks.MR_unify_tuple_pred != NULL) {
> +            return MR_special_pred_hooks.MR_unify_tuple_pred(ti,
> +                (MR_Word) x, (MR_Word) y);
> +        }

So, if it is NULL we fall through and eventually call MR_fatal_error,
right?

...

> --- a/runtime/mercury_ho_call.h
> +++ b/runtime/mercury_ho_call.h
> @@ -194,4 +194,24 @@ MR_declare_entry(mercury__builtin__compare_representation_3_0);
>
> #endif	/* MR_HIGHLEVEL_CODE */
>
> +/*
> +** Special predicates implemented in the standard library
> +**
> +** The library sets the fields in this structure to the actual
> +** implementations of the predicates during initialization.
> +*/

Add a pointer to the relevant section of library/builtins.m that
explains why we do things this way.

> +
> +typedef struct MR_SpecialPredHooks_Struct {
> +  #ifdef MR_HIGHLEVEL_CODE
> +    MR_bool     (*MR_unify_tuple_pred)(MR_Word ti, MR_Word x, MR_Word y);
> +    MR_bool     (*MR_compare_tuple_pred)(MR_Word ti, MR_Word *res,
> +                    MR_Word x, MR_Word y);
> +  #else
> +    MR_ProcAddr MR_unify_tuple_pred;
> +    MR_ProcAddr MR_compare_tuple_pred;
> +  #endif
> +} MR_SpecialPredHooks;
> +
> +extern MR_SpecialPredHooks  MR_special_pred_hooks;
> +
> #endif	/* not MERCURY_HO_CALL_H */
> diff --git a/runtime/mercury_unify_compare_body.h b/runtime/mercury_unify_compare_body.h
> index 08c3941..dfbaedb 100644

The change looks fine to me (modulo my remarks elsewhere in this thread
about library initialisation).

Cheers,
Julien.