[m-rev.] problems with abs for int{8,16,32}

[Julien Fischer]

On Thu, 15 Feb 2018, Zoltan Somogyi wrote:

> On Thu, 15 Feb 2018 14:53:16 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>> They are *not* fine in asm_fast.gc, you weren't trying hard enough ;-)
>> (Try compiling with -O0.)
>
> :-)
>
>>> I checked, and the abs operation on e.g. min_int8 throws
>>> an exception, as expected, when invoked directly from main,
>>> but the exception vanishes when invoked from within a try block.
>>>
>>> Julien, I think fixing this is up to you. I am attaching my cut-down
>>> test case to save you a bit of work.
>>
>> The issue here is the handling of sub-word sized types in scalar common
>> data; the runtime machinery for doing looks up in common data cannot
>> handle fields being less than a word in size.  The fix here is to ensure
>> that an field in static data are at least a word in size (which is
>> indeed precisely how the MLDS backend handles it).
>>
>> I'll post a fix for this in the next couple of days.
>
> I am probably more familiar with the common data system than you are.
> If you can describe the problem to me in more detail, I can maybe fix it

The attached program (a very cut-down version of the arith_int8 test
case), contains this:

     main(!IO) :-
         As = [-128i8],
         foo(As, !IO).

In the generated C code for it we have this:

     struct mercury_type_1 {
         int8_t f1;
         MR_Word * f2;
     };
     MR_STATIC_LINKAGE const struct mercury_type_1 mercury_common_1[];

     ...

     static const struct mercury_type_1 mercury_common_1[1] =
     {
     {
       INT8_C(-128),
      MR_tbmkword(0, 0)
     },
     };

The type of the field 'f1' in the struct above needs to be an MR_Word.
(That's fine since 8-, 16- and 32-bit integers will always fit in an
MR_Word.)

Uncommenting the code in the diff below fixes the int8 case.  (Extending
it to the other affected types is trivial.)  In practice, it would be
nice to emit a cast to MR_Word before the int8_t constant as well.

diff --git a/compiler/llds_out_global.m b/compiler/llds_out_global.m
index 2d9c0cf..72ff6ae 100644
--- a/compiler/llds_out_global.m
+++ b/compiler/llds_out_global.m
@@ -526,6 +526,8 @@ output_cons_arg_types([Type | Types], Indent, ArgNum, !IO) :-
      ( if Type = lt_float then
          % Ensure float structure members are word-aligned.
          io.write_string("MR_Float_Aligned", !IO)
+    %else if Type = lt_int(int_type_int8) then
+    %    io.write_string("MR_Word", !IO)
      else
          output_llds_type(Type, !IO)
      ),

> while waiting for Peter's review of my change to du_type_layout.m.
> Or, if Peter can't do the review, I can trade the fix for a review by you.

I will take a look this evening, assuming Peter hasn't got to it by
then.

Julien.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arith_int8.m
Type: application/vnd.wolfram.mathematica.package
Size: 1661 bytes
Desc: 
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20180215/2fd969ac/attachment.bin>