[m-rev.] for post-commit review: group *int_least* types together

[Julien Fischer]

Hi Zoltan,

On Tue, 20 Feb 2018, Zoltan Somogyi wrote:

> On Sun, 18 Feb 2018 04:40:18 -0500 (EST), Julien Fischer <jfischer at opturion.com> wrote:
>
>> Have I missed something here?
>
> I phrased myself badly. What I want to do is to commit the following
> diff:
>
> --- a/compiler/llds_out_global.m
> +++ b/compiler/llds_out_global.m
> @@ -743,10 +743,11 @@ ok_int_const(N, LLDSType) :-
>             IntLeastType = uint_least16,
>             0 =< N, N < 65536
>         ;
> -            ( IntLeastType = int_least32
> -            ; IntLeastType = uint_least32
> -            )
> -            % XXX BUG: The type int may be 64 bit.
> +            IntLeastType = int_least32,
> +            -2147483647 =< N, N < 2147483647

That's (obviously) not problematic ...

> +        ;
> +            IntLeastType = uint_least32,
> +            0 =< N, N < 4294967295

... but that is.  The compiler only accepts decimal int literals up to
int.max_int (which the second test there comfortably exceeds!)

The compiler will accept larger non-decimal int literals, but that won't
help you since it's a signed comparison operation anyway and it wouldn't
do the right thing here.

I suggest making the test for the uint_least32 case something like:

      0 =< N,
      ( int.bits_per_int = 32
      ; int64.from_int(N) < 4294967295_i64
      )

>     ;
>         LLDSType = lt_int(IntType),
> @@ -764,10 +765,11 @@ ok_int_const(N, LLDSType) :-
>             IntType = int_type_uint16,
>             0 =< N, N < 65536
>         ;
> -            ( IntType = int_type_int32
> -            ; IntType = int_type_uint32
> -            )
> -            % XXX BUG: The type int may be 64 bit.
> +            IntType = int_type_int32,
> +            -2147483647 =< N, N < 214748364
> +        ;
> +            IntType = int_type_uint32,
> +            0 =< N, N < 4294967295

Ditto here.

>         ;
>             ( IntType = int_type_int64
>             ; IntType = int_type_uint64
>
> I know that the references to 4294967295 will work
> on 64 bit machines. What I want to know is: will they work
> on 32 bit machines as well?
> (Where, ironically, they are
> not needed.) I cannot test it, since I don't have
> any 32 bit machines. I asked you because you were
> the one to last touch the relevant parts of the compiler,
> as part of adding support for 64 bit unboxed integers.

I think I have answered that above.

Julien.