[m-rev.] for review: make unchecked shifts by uint amounts builtin, step 1

Julien Fischer jfischer at opturion.com
Thu Apr 30 17:06:27 AEST 2020


Hi Zoltan,

On Thu, 30 Apr 2020, Zoltan Somogyi wrote:

> For review by anyone. I particularly would like feedback on
> the changes to mlds_to_{cs,java}_data.m, since I don't know
> C# or Java that well.

> Step one of adding unchecked shifts by uint amounts.
> 
> compiler/builtin_ops.m:
>     Parameterize the unchecked left and right shift builtin ops
>     by whether the shift amount is an int or an uint. So far,
>     the shift amount has always been an int; allowing the shift amount
>     to be a uint is new.
>
>     Recognize the Mercury functions unchecked_{left,right}_ushift
>     as being builtins implemented by the new variants of the unchecked
>     shift builtin ops mentioned above. These Mercury functions do not
>     exist yet. They will be added in step two of this diff, *after* this
>     change has been installed. (Making something a builtin, and *then*
>     defining it, is easier than defining it, and *then* making it a builtin,
>     because in the latter case, the stage 1 and stage 2 compilers disagree
>     on whether the function in question needs to have a definition.)
> 
> compiler/options.m:
>     Provide a way to check whether an installed compiler has this diff.
>     (This is needed for step 2.)
> 
> compiler/lookup_switch.m:
> compiler/ml_lookup_switch.m:
> compiler/ml_unify_gen_util.m:
> compiler/unify_gen_util.m:
>     When generating references to unchecked shift ops, specify that the
>     shift amount is an int.
> 
> compiler/erl_call_gen.m:
>     Don't treat unchecked shifts by uint amounts as builtins, since I (zs)
>     don't know how this should be done in Erlang.

I wouldn't worry too much about the Erlang backend; it's currently lacking
a lot of support for non-int integer types anyway.

...

> diff --git a/compiler/llds_out_data.m b/compiler/llds_out_data.m
> index 92ba641fe..d0148ae2c 100644
> --- a/compiler/llds_out_data.m
> +++ b/compiler/llds_out_data.m
> @@ -1178,16 +1178,22 @@ output_rval(Info, Rval, !IO) :-
>              output_rval_as_type(Info, SubRvalB, lt_int(int_type_int), !IO),
>              io.write_string(")", !IO)
>          ;
> -            % The second operand of the shift operatators always has type
> -            % `int'.
> -            ( Op = unchecked_left_shift(IntType), OpStr = "<<"
> -            ; Op = unchecked_right_shift(IntType), OpStr = ">>"
> +            % The second operand of the shift operators always has type
> +            % `int' in C, but Mercury also allows it to be uint.

The second operand of a shift in C can have any integral type.  (Obviously,
for the behaviour to be defined the second operand must have a value that
is non-negative and within the bit size of the first operand.)  Or do
you mean that we always generate shift expressions so that their second
operand has type int?

> +            ( Op = unchecked_left_shift(IntType, ShiftType), OpStr = "<<"
> +            ; Op = unchecked_right_shift(IntType, ShiftType), OpStr = ">>"
>              ),
>              io.write_string("(", !IO),
>              output_rval_as_type(Info, SubRvalA, lt_int(IntType), !IO),
>              io.write_string(" ", !IO),
>              io.write_string(OpStr, !IO),
>              io.write_string(" ", !IO),
> +            (
> +                ShiftType = shift_by_int
> +            ;
> +                ShiftType = shift_by_uint,
> +                io.write_string("(int) ", !IO)
> +            ),
>              output_rval_as_type(Info, SubRvalB, lt_int(int_type_int), !IO),
>              io.write_string(")", !IO)

The diff looks fine.

Julien.






More information about the reviews mailing list