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

Julien Fischer jfischer at opturion.com
Sat Feb 17 13:15:20 AEDT 2018

On Sat, 17 Feb 2018, Zoltan Somogyi wrote:

> On Fri, 16 Feb 2018 20:32:18 +1100 (AEDT), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
>> I am pretty sure the bug lies in the fact that generate_const_struct
>> ignores the cs_term_type field of ConstStruct. It should look up the
>> representation of the type, and use that info in its task.
> I am now pretty sure that the above is wrong :-)
> The malformed constant structure is in fact generated by
> the following chain of calls implementing As = [-128i8]:
> generate_construction_2
> construct_cell
> assign_cell_to_var
> var_locn_assign_cell_to_var
> cell_is_constant
> rval_type_as_arg
> natural_type
> The problem is that the last two in that chain were designed
> to decide only the TYPE of an rval in a cell. They were not designed
> to decide the SIZE of that rval, because when they were written,
> every rval was exactly the same size: the size of MR_Word.
> This assumption pervades pretty much the whole of global_data.m.
> While some of the predicates in that module take an argument whose
> type is arg_width (whose function symbols are full_word, double_word,
> partial_word_first and partial_word_shifted), the last two functors
> do not appear in global_data.m at all, and double_word appears
> only in two occurrences of the test "ArgWidth \= double_word"

We get away with the double_word case because that is currently only
used with floats and floats are handled specially throughout the
code generator anyway.


> What I am saying is that the right way to fix this is to wait
> for any problems with the new du_type_layout.m to show up,
> fix them if they occur, and *then* fix this bug, in a compiler
> that will by then allow e.g. int8s to occur not just as the only
> thing in a word but also as one of several arguments packed
> together in a word. This will push things out to (probably)
> thursday or friday.
> Julien, would you be ok with that?

That's fine by me.


More information about the reviews mailing list