[m-rev.] for review: fix bug #499 - switches on 64-bit integers in the Java grade

Julien Fischer jfischer at opturion.com
Tue Feb 13 12:18:59 AEDT 2018


On Tue, 13 Feb 2018, Peter Wang wrote:

> On Mon, 12 Feb 2018 11:43:23 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>> @@ -117,15 +118,17 @@ ml_simplify_switch(Stmt0, Stmt, !Info) :-
>>           Stmt = Stmt0
>>       ).
>>
>> -:- func is_integral_type(mlds_type) = bool.
>> +:- pred is_integral_type(mlds_type::in, int_type::out) is semidet.
>>
>> -is_integral_type(MLDSType) = IsIntegral :-
>> -    (
>> +is_integral_type(MLDSType, IntType) :-
>> +    require_complete_switch [MLDSType] (
>>           ( MLDSType = mlds_native_int_type
>> -        ; MLDSType = mlds_native_uint_type
>>           ; MLDSType = mlds_native_char_type
>>           ),
>> -        IsIntegral = yes
>> +        IntType = int_type_int
>
> I suggest the usual formatting with the opening paren on a new line.

Done.

>> diff --git a/compiler/switch_gen.m b/compiler/switch_gen.m
>> index d919709..84a7ab3 100644
>> --- a/compiler/switch_gen.m
>> +++ b/compiler/switch_gen.m
>> @@ -154,7 +155,11 @@ generate_switch(CodeModel, SwitchVar, CanFail, Cases, GoalInfo, Code,
>>           MayUseSmartIndexing = may_use_smart_indexing,
>>           module_info_get_globals(ModuleInfo, Globals),
>>           (
>> -            SwitchCategory = atomic_switch,
>> +            % When generating we do not require any special treatment of
>> +            % switches involving 64-bit integers.
>> +            ( SwitchCategory = atomic_switch
>> +            ; SwitchCategory = int64_switch
>> +            ),
>>               num_cons_ids_in_tagged_cases(TaggedCases, NumConsIds, NumArms),
>>               ( if
>>                   MaybeIntSwitchInfo =
>
> It's not immediately clear what special information the comment is
> trying to impart. Maybe just delete it.

It was meant to say "When generating C ..."; the intention to was to
contrast it with the MLDS backend where a separation between the two
categories of switch is necessary.  I've deleted it.

> Looks fine otherwise.

Thanks for that.

Julien.


More information about the reviews mailing list