[m-rev.] for review: Add constant structure support to Java backend

[Julien Fischer]

Hi Paul,

On Wed, 29 Jan 2014, Paul Bone wrote:

> Add constant structure support to Java backend
>
> This change enables constant structure support for the Java backend for the
> structures introduced by the polymorphism pass.
>
> compiler/const_struct.m:
> compiler/ml_unify_gen.m:
>    Enable constant structure support
>
> java/runtime/TypeInfo_Struct.java:
>    Add a constructor to the TypeInfo structure for Java.  This constructor
>    has the interface expected by the polymorphism code.
>
>    Never expect to be passed the arity to the varargs constructor.
>
> compiler/polymorphism.m:
> compiler/ml_unify_gen.m:
>    Change how we avoid creating TypeInfos with an explicit arity for Java

s/arity/arity argument/


>    We never specify the arity for a TypeInfo_Struct in a Java grade.  There
>    are two reasons: this is a little difficult to handle due to overloading
>    rules and it is not necessary.
>
>    This is handled by removing these arguments in the MLDS backend.
>    However, the current solution does not work when we create TypeInfos as
>    constant structures.  Therefore this change removes the special case
>    from the MLDS backend and instead never creates the arity arguments
>    during the polymorphism pass.  rtti_to_mlds.m didn't need updating as it
>    already had the correct behaviour.
>
> compiler/rtti_to_mlds.m:
>    Update a comment.

...

> diff --git a/compiler/const_struct.m b/compiler/const_struct.m
> index a9def75..812ad15 100644
> --- a/compiler/const_struct.m
> +++ b/compiler/const_struct.m
> @@ -1,7 +1,7 @@
> -        ; Target = target_java
>         ; Target = target_x86_64
>         ; Target = target_erlang
>         ),
> @@ -186,6 +167,38 @@ const_struct_db_init(Globals, Db) :-
>     Db = const_struct_db(PolyEnabled, GroundTermEnabled, 0,
>         map.init, map.init, map.init, map.init).
>
> +    % Test if constant structures are enabled for polymorphism structures
> +    % and from ground term contexts.  The latter is only enabled if tracing

s/from ground term/from_ground_term/

> +    % does not require procedure bodies to be preserved.  The caller must
> +    % also check if the compilation grade supports constant structures.

Is there a good reaons why this predicate does not check that itself?

> +:- pred can_enable_const_struct(globals::in, bool::out, bool::out) is det.
> +
> +can_enable_const_struct(Globals, PolyEnabled, GroundTermEnabled) :-
> +    globals.lookup_bool_option(Globals, enable_const_struct,
> +        OptionEnabled),
> +    PolyEnabled = OptionEnabled,
> +
> +    globals.get_trace_level(Globals, TraceLevel),
> +    globals.get_trace_suppress(Globals, TraceSuppress),
> +    Bodies = trace_needs_proc_body_reps(TraceLevel, TraceSuppress),
> +    (
> +        Bodies = no,
> +        GroundTermEnabled = OptionEnabled
> +    ;
> +        Bodies = yes,
> +        % We generate representations of procedure bodies for the
> +        % declarative debugger and for the profiler. When
> +        % traverse_primitives in browser/declarative_tree.m looks for the
> +        % Nth argument of variable X and X is built with a unification such
> +        % as X = ground_term_const(...), it crashes. It should be taught not
> +        % to do that, but in the meantime, we prevent the situation from
> +        % arising in the first place. (We never look for the original
> +        % sources of type infos and typeclass infos, so we can use constant
> +        % structures for them.)
> +        GroundTermEnabled = no
> +    ).
> +
> lookup_insert_const_struct(ConstStruct, ConstNum, !Db) :-
>     const_struct_db_get_poly_enabled(!.Db, Enabled),

...


> diff --git a/compiler/polymorphism.m b/compiler/polymorphism.m
> index 13f24ae..3765d88 100644
> --- a/compiler/polymorphism.m
> +++ b/compiler/polymorphism.m
> @@ -1,7 +1,7 @@
> %-----------------------------------------------------------------------------%
> % vim: ft=mercury ts=4 sw=4 et
> %-----------------------------------------------------------------------------%
> -% Copyright (C) 1995-2012 The University of Melbourne.
> +% Copyright (C) 1995-2012, 2014 The University of Melbourne.
> % This file may only be copied under the terms of the GNU General
> % Public License - see the file COPYING in the Mercury distribution.
> %-----------------------------------------------------------------------------%
> @@ -3280,7 +3280,8 @@ polymorphism_construct_type_info(Type, TypeCtor, TypeArgs, TypeCtorIsVarArity,
>     %
>     %   TypeInfoVar = type_info(TypeCtorVar, ArgTypeInfoVars...)
>     %
> -    % However, if TypeCtorIsVarArity is true, then it will be of the form
> +    % However, if TypeCtorIsVarArity is true and we are not compiling for
> +    % the Java backend, then it will be of the form
>     %
>     %   TypeInfoVar = type_info(TypeCtorVar, Arity, ArgTypeInfoVars...)
>     %
> @@ -3294,11 +3295,39 @@ polymorphism_construct_type_info(Type, TypeCtor, TypeArgs, TypeCtorIsVarArity,
>     % The returned Var will be bound to the type_info cell of Type if such
>     % a cell had to be allocated, and to the type_ctor_info of Type's only
>     % type constructor if it didn't.
> +    %
> +    % NOTE: the special handling for the java backend must be kept
> +    % conisistent with:

s/conisistent/consistent/

> +    %   rtti_to_mlds.gen_type_info_defn/6
> +    %   java/runtime/TypeInfo_Struct.java
> +
> +    % Determine if we need to explicitly pass the arity

Add full stop.

> +    (
> +        TypeCtorIsVarArity = no,
> +        PassArity = no
> +    ;
> +        TypeCtorIsVarArity = yes,
> +        poly_info_get_module_info(!.Info, ModuleInfo),
> +        module_info_get_globals(ModuleInfo, Globals),
> +        get_target(Globals, Target),
> +        (
> +            Target = target_java,
> +            PassArity = no
> +        ;
> +            ( Target = target_c
> +            ; Target = target_il
> +            ; Target = target_csharp
> +            ; Target = target_x86_64
> +            ; Target = target_erlang
> +            ),
> +            PassArity = yes
> +        )
> +    ),
>
>                     no, Var, TypeInfoGoal,
>                     VarSet1, VarSet, VarTypes1, VarTypes,

...

> diff --git a/java/runtime/TypeInfo_Struct.java
> b/java/runtime/TypeInfo_Struct.java
> index 20524f4..e3a3162 100644
> --- a/java/runtime/TypeInfo_Struct.java
> +++ b/java/runtime/TypeInfo_Struct.java

...

> 	// because such overloadings are not allowed under Java 1.7.  (Previous
> -	// versions of Java incorrectly allowed them.)
> +	// versions of Java incorrectly allowed them.)  This is okay because
> +	// init above simply checks the number of items in the array to
> +	// determine the arity.
> +	//
> 	// If you change this you will also need to update the code in
> -	// compiler/rtti_to_mlds.m.
> +	// compiler/rtti_to_mlds.m and compiler/polymorphism.m
> 	//
> -	public TypeInfo_Struct(TypeInfo_Struct ti, Object... as)
> +	public TypeInfo_Struct(Object a1, Object... as)
> 	{
> -		init(ti.type_ctor, as);
> +		if (a1 instanceof TypeInfo_Struct) {
> +			TypeInfo_Struct ti = (TypeInfo_Struct)a1;
> +			init(ti.type_ctor, as);
> +		} else if (a1 instanceof TypeCtorInfo_Struct) {
> +			init((TypeCtorInfo_Struct)a1, as);
> +		} else {
> +			throw new ClassCastException(
> +				"Couldn't cast argument to a TypeInfo or " +
> +				"TypeCtorInfo");

s/Couldn't/cannot/

That looks okay otherwise.

Cheers,
Julien.