[m-rev.] for review: grade compability tests in .h files

[Julien Fischer]

On Mon, 11 Apr 2016, Zoltan Somogyi wrote:

> For review by anyone, but I think everyone should read the rest
> of this email.
> 
> There is a ZZZ in the diff. The accurate gc that Fergus wrote works only in
> hlc grades. However, the runtime does not currently enforce this constraint,
> because in the past, it was useful to invoke test the effects of specifying
> accurate gc on the compiler's liveness calculations.

IIRC, the low-level C version of the accurate collector advanced a bit
further than that.  (According to the mailing list archives it was
possible to build the stage 2 compiler.)

> The test cases tests/valid/agc_*.m currently fail in llds grades if I
> enable the constraint marked with ZZZ in runtime/mercury_grade.h. The
> .c file is generated ok, but the C compiler generates the error
> message called-for by the constraint.
> 
> I am pretty sure that we should enable the constraint, but what should we
> do with the tests? The right thing to do would be to have them run in hlc.agc
> grades, but I don't test those grades myself. Does anyone else?

No, last I checked (quite a while ago) hlc.agc wasn't working.  We
certainly don't regularly test it.

> Alternatively, we could arrange the test setup so that for those
> tests, we only test the generation of the .c file, not the .o file.

I think that is the preferable solution for now.

> After I modify the grade library in accordance with our discussions last week,
> I plan to start integrating it into the compiler, starting with steps that
> won't be visible to users.
> 
> The first step is simply giving the compiler access to the library.
> This can be done in one of two ways:
> 
> - Building a library named e.g. libmer_grade.{a,so} in the grade_lib
>   directory, and linking it into the compiler the same way we link
>   e.g. the mdbcomp library into it.
> 
> - Copying each .m file of the library to the compiler directory, the same
>   way we copy the modules of the mdbcomp library to the deep_profiling
>   and slice directories.
> 
> I prefer the second option, because it allows the standalone programs
> in the grade_lib directory (some of which will eventually be user-visible)
> to be compiled in a different grade from the compiler itself. This means that
> e.g. if I want to rebuild one of those small programs in a debug grade,
> I don't have to wait for the rest of the system (including the Mercury
> standard library) to be rebuilt in that grade. (This is the reason
> why we use this technique for the deep profiling and slice directories.)
> The downside is small: it means using the installed standard library,
> so the grade library's code cannot build on a new standard library predicate
> in the workspace until that workspace has been installed.
> 
> Does anyone object to this choice?

I don't.

> The second step is to change how the main body of the compiler makes
> decisions that depend on the settings of grade components. At the moment,
> it looks up most grade components in the options table, although a few
> grade components are stored next to the options table in the globals
> structure in purpose-specific enums (e.g. target). I propose to
> 
> - change the internal names of the relevant options and globals fields
>   (leaving the public option names unchanged),
> 
> - trivially update the code in handle_options.m and compute_grade.m
>   (but NOT the rest of the compiler) to use the new names,
> 
> - set the values of the grade_vars (in grade_lib/grade_vars.m) based on
>   the values of these options computed by the current code in handle_options.m
>   and compute_grade.m, and add the grade_vars structure as a new field
>   of the globals structures, and
> 
> - update all the REST of the compiler to use these grade_vars.
> 
> The last step should allow some code to be simplified, by replacing code
> that consults two or more boolean options with code that looks up a grade_var
> that is an enum with three or more values.
> 
> Does anyone object to this plan, or have a better one?
> 
> Zoltan.

...

> Move grade compatibility tests to mercury_grade.h.
> 
> Fix some comments (mostly in layout) in all the touched runtime headers.
> 
> runtime/mercury.h:
>     Simplify the error message on the one grade compatibility test
>     that is best kept out of mercury_grade.h, because it is next to
>     the cause of the potential incompatibility.
> 
> runtime/mercury_grade.h:
>     Add a comment about the grade compatibility test still in mercury.h.
>
>     Move all the grade compatibility tests from mercury_conf_param.h here.
>     Reword their error messages to talk directly about the user-visible
>     grade components (or the Mercury compiler options enabling them,
>     if there is no single grade component name for them) instead of the
>     names of the C macros representing them.
>
>     Group some related existing tests together.
> 
> runtime/mercury_conf_param.h:
>     Move all the grade compatibility tests here to mercury_grade.h.
> 
> compiler/compile_target_code.m:
>     Add a XXX.

...

> diff --git a/compiler/compile_target_code.m b/compiler/compile_target_code.m
> index f29b6bb..aec5ad1 100644
> --- a/compiler/compile_target_code.m
> +++ b/compiler/compile_target_code.m
> @@ -761,6 +761,15 @@ gather_grade_defines(Globals, GradeDefines) :-
>      globals.lookup_bool_option(Globals, low_level_debug, LL_Debug),
>      (
>          LL_Debug = yes,
> +        % XXX This is grade option is supposed to tell the C compiler
> +        % (a) to turn on the generation of debugging symbols and
> +        % (b) to disable the optimizations that would make the executable
> +        % harder to debug in a C debugger such as gdb. However, we don't
> +        % pass those options here. Instead, we rely on the --grade option
> +        % that we give to mgnuc and ml on their command lines; when they see
> +        % the ll_debug grade component, *they* will pass those options onto
> +        % the C compiler and linker. Since this is the case, why can't we
> +        % make them pass -DMR_LL_DEBUG as well?
>          LL_DebugOpt = "-DMR_LL_DEBUG "
>      ;
>          LL_Debug = no,

Passing the value of --cflags-for-debug is handled elsewhere in
compile_target_code.m.  The code for gathering together all the grade
#defines is separate so that we can implement the --output-grade-defines
option.  The expectation for that option is that it will return only
macro definitions, not other C compiler flags.

The rest of the diff looks fine.

Julien.