[m-rev.] for review: pack small args next to local secondary tags

[Julien Fischer]

Hi Zoltan,

On Sat, 14 Jul 2018, Zoltan Somogyi wrote:

> On Sat, 14 Jul 2018 12:44:14 -0400 (EDT), Julien Fischer <jfischer at opturion.com> wrote:
>
>> On Thu, 12 Jul 2018, Zoltan Somogyi wrote:
>>
>>> On Tue, 3 Jul 2018 21:14:22 -0400 (EDT), Julien Fischer <jfischer at opturion.com> wrote:
>>>> Specifically, in asm_fast.gc.decldebug.trseg.stseg, the following
>>>> fail on my system (x86-64,linux):
>>>>
>>>>     hard_coded/arith_int16
>>>>     hard_coded/arith_int32
>>>>     hard_coded/arith_int8
>>>>     hard_coded/pack_int32
>>>>
>>>> The packing one is most likely failing bcause the arithmetic ones are.
>>>> (This looks like a regression of the issue fixed in commit b765dae,
>>>> but it isn't since the test case for that passes.)
>>>
>>> The attached diff fixes these failures.
>>
>> As of rotd-2018-07-14 the last one of those is still failing for me.
>>
>> diff -u pack_int32.exp pack_int32.out
>> --- pack_int32.exp      2018-05-16 20:06:06.000000000 -0400
>> +++ pack_int32.out      2018-07-14 12:39:16.000000000 -0400
>> @@ -1,3 +1,3 @@
>> -struct(-2147483648, 2147483647, -32768, 32767)
>> +struct(-2147483648, -32768, 990059265, 25)
>>   struct(-2147483648, 2147483647, -32768, 32767)
>>   struct(-2147483648, 2147483647, -32768, 32767)
>>
>> (Same grade and system as above.)
>
> It passed for me in asm_fast.gc.decldebug.stseg, i.e. NOT trseg,
> but trailing should have no bearing on this.
>
> Did the failure happen with allow_packing_* enabled or disabled?
> And can you confirm that you were generating 64 bit code,
> not 32 bit?

I'm not sure why it is working on your machine;  (Note that the problem
only shows up if --comon-struct is enabled; so at -O2 by default).
However, there is a problem here and thse has been with every ROTD at
least as far back as rotd-2017-10-22 (which is the earliest one I still
have installed on my machine).

The following diff fixes the problem.

Julien.

---------------------------------------

Fix the failure of tests/hard_coded/pack_int32 in decldebug grades.

The above test was failing decldebug grades at -O2 and above.  The problem was
that the part of the code generator responsible for generating ground terms had
been updated to handle sub-word sized integers.  (64-bit integer on 32-bit
machines were also not being handled correctly.)

compiler/unify_gen_construct.m:
      Make the above fix.

diff --git a/compiler/unify_gen_construct.m b/compiler/unify_gen_construct.m
index 6354399..6eaf2f6 100644
--- a/compiler/unify_gen_construct.m
+++ b/compiler/unify_gen_construct.m
@@ -872,10 +872,12 @@ generate_ground_term(TermVar, Goal, !CI, !CLD) :-
                  get_module_info(!.CI, ModuleInfo),
                  get_exprn_opts(!.CI, ExprnOpts),
                  UnboxedFloats = get_unboxed_floats(ExprnOpts),
+                UnboxedInt64s = get_unboxed_int64s(ExprnOpts),
                  get_static_cell_info(!.CI, StaticCellInfo0),
                  map.init(ActiveMap0),
                  generate_ground_term_conjuncts(ModuleInfo, Conjuncts,
-                    UnboxedFloats, StaticCellInfo0, StaticCellInfo,
+                    UnboxedFloats, UnboxedInt64s,
+                    StaticCellInfo0, StaticCellInfo,
                      ActiveMap0, ActiveMap),
                  map.to_assoc_list(ActiveMap, ActivePairs),
                  ( if ActivePairs = [TermVar - GroundTerm] then
@@ -899,27 +901,27 @@ generate_ground_term(TermVar, Goal, !CI, !CLD) :-

  :- type active_ground_term_map == map(prog_var, typed_rval).

-:- pred generate_ground_term_conjuncts(module_info::in,
-    list(hlds_goal)::in, have_unboxed_floats::in,
+:- pred generate_ground_term_conjuncts(module_info::in, list(hlds_goal)::in,
+    have_unboxed_floats::in, have_unboxed_int64s::in,
      static_cell_info::in, static_cell_info::out,
      active_ground_term_map::in, active_ground_term_map::out) is det.

  generate_ground_term_conjuncts(_ModuleInfo, [],
-        _UnboxedFloats, !StaticCellInfo, !ActiveMap).
+        _UnboxedFloats, _UnboxedInt64s, !StaticCellInfo, !ActiveMap).
  generate_ground_term_conjuncts(ModuleInfo, [Goal | Goals],
-        UnboxedFloats, !StaticCellInfo, !ActiveMap) :-
+        UnboxedFloats, UnboxedInt64s, !StaticCellInfo, !ActiveMap) :-
      generate_ground_term_conjunct(ModuleInfo, Goal, UnboxedFloats,
-        !StaticCellInfo, !ActiveMap),
+        UnboxedInt64s, !StaticCellInfo, !ActiveMap),
      generate_ground_term_conjuncts(ModuleInfo, Goals, UnboxedFloats,
-        !StaticCellInfo, !ActiveMap).
+        UnboxedInt64s, !StaticCellInfo, !ActiveMap).

-:- pred generate_ground_term_conjunct(module_info::in,
-    hlds_goal::in, have_unboxed_floats::in,
+:- pred generate_ground_term_conjunct(module_info::in, hlds_goal::in,
+    have_unboxed_floats::in, have_unboxed_int64s::in,
      static_cell_info::in, static_cell_info::out,
      active_ground_term_map::in, active_ground_term_map::out) is det.

  generate_ground_term_conjunct(ModuleInfo, Goal, UnboxedFloats,
-        !StaticCellInfo, !ActiveMap) :-
+        UnboxedInt64s, !StaticCellInfo, !ActiveMap) :-
      Goal = hlds_goal(GoalExpr, _GoalInfo),
      ( if
          GoalExpr = unify(_, _, _, Unify, _),
@@ -930,18 +932,18 @@ generate_ground_term_conjunct(ModuleInfo, Goal, UnboxedFloats,
          associate_cons_id_args_with_widths(ModuleInfo, ConsId,
              ArgVars, ArgVarsWidths),
          generate_ground_term_conjunct_tag(Var, ConsTag, ArgVarsWidths,
-            UnboxedFloats, !StaticCellInfo, !ActiveMap)
+            UnboxedFloats, UnboxedInt64s, !StaticCellInfo, !ActiveMap)
      else
          unexpected($pred, "malformed goal")
      ).

  :- pred generate_ground_term_conjunct_tag(prog_var::in, cons_tag::in,
      list(arg_and_width(prog_var))::in, have_unboxed_floats::in,
-    static_cell_info::in, static_cell_info::out,
+    have_unboxed_int64s::in, static_cell_info::in, static_cell_info::out,
      active_ground_term_map::in, active_ground_term_map::out) is det.

  generate_ground_term_conjunct_tag(Var, ConsTag, ArgVarsWidths,
-        UnboxedFloats, !StaticCellInfo, !ActiveMap) :-
+        UnboxedFloats, UnboxedInt64s, !StaticCellInfo, !ActiveMap) :-
      % The code of this predicate is very similar to the code of
      % generate_const_struct_arg_tag. Any changes here may also
      % require similar changes there.
@@ -953,7 +955,32 @@ generate_ground_term_conjunct_tag(Var, ConsTag, ArgVarsWidths,
          ;
              ConsTag = int_tag(IntTag),
              int_tag_to_const_and_int_type(IntTag, Const, IntType),
-            Type = lt_int(IntType)
+            (
+                ( IntType = int_type_int64
+                ; IntType = int_type_uint64
+                ),
+                (
+                    UnboxedInt64s = have_unboxed_int64s,
+                    Type = lt_int(IntType)
+                ;
+                    UnboxedInt64s = do_not_have_unboxed_int64s,
+                    Type = lt_data_ptr
+                )
+            ;
+                ( IntType = int_type_int
+                ; IntType = int_type_int8
+                ; IntType = int_type_int16
+                ; IntType = int_type_int32
+                ),
+                Type = lt_int(int_type_int)
+            ;
+                ( IntType = int_type_uint
+                ; IntType = int_type_uint8
+                ; IntType = int_type_uint16
+                ; IntType = int_type_uint32
+                ),
+                Type = lt_int(int_type_uint)
+            )
          ;
              ConsTag = dummy_tag,
              Const = llconst_int(0),