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

Julien Fischer jfischer at opturion.com
Mon Feb 12 11:43:23 AEDT 2018


For review by anyone.

Note that the test case for this one is already committed.

-------------------------------------------

Fix bug #499 - switches on 64-bit integers in the Java grade.

Switches on 64-bit integers in the Java grade are causing us to generate
invalid Java code because Java's switch statement does not work with 64-bit
integers.  (This limitation is apparently built into the JVM.)

Fix this for now by generating if-then-else chains for switches on 64-bit
integers.   We can almost certainly do better, but that is future work.

compiler/switch_util.m:
     Add a separate switch category for switches on 64-bit integer types.

compiler/switch_gen.m:
     For the low-level C backend, treat switches on 64-bit integers as
     other switches on atomic types.


compiler/ml_target_util.m:
     Add a function that returns whether the target language supports
     switching on 64-bit integers.   Add another that allows us to
     test whether switches on a given integer type are supported by
     a given target language.

compiler/ml_switch_gen.m:
compiler/ml_simplify_switch.m:
     Use the above functions to only generate switches on 64-bit integers
     as target language switches if the target language supports switches
     on 64-bit integers (C# and C do; Java does not).

Julien.

diff --git a/compiler/ml_simplify_switch.m b/compiler/ml_simplify_switch.m
index 9d4cb75..b8beba2 100644
--- a/compiler/ml_simplify_switch.m
+++ b/compiler/ml_simplify_switch.m
@@ -2,6 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 et
  %---------------------------------------------------------------------------%
  % Copyright (C) 2000-2001, 2003-2011 The University of Melbourne.
+% Copyright (C) 2015-2018 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %---------------------------------------------------------------------------%
@@ -66,13 +67,13 @@ ml_simplify_switch(Stmt0, Stmt, !Info) :-

          % Is this an int switch?
          Stmt0 = ml_stmt_switch(Type, Rval, Range, Cases, Default, Context),
-        is_integral_type(Type) = yes,
+        is_integral_type(Type, IntType),

          % Does the target want us to convert dense int switches
          % into computed gotos?
          globals_target_supports_computed_goto(Globals) = yes,
          not (
-            globals_target_supports_int_switch(Globals) = yes,
+            globals_target_supports_int_type_switch(Globals, IntType) = yes,
              globals.lookup_bool_option(Globals, prefer_switch, yes)
          ),

@@ -96,9 +97,9 @@ ml_simplify_switch(Stmt0, Stmt, !Info) :-
          % unless the target prefers switches.

          Stmt0 = ml_stmt_switch(Type, Rval, _Range, Cases, Default, Context),
-        is_integral_type(Type) = yes,
+        is_integral_type(Type, IntType),
          not (
-            globals_target_supports_int_switch(Globals) = yes,
+            globals_target_supports_int_type_switch(Globals, IntType) = yes,
              globals.lookup_bool_option(Globals, prefer_switch, yes)
          )
      then
@@ -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
+    ;
+        MLDSType = mlds_native_uint_type,
+        IntType = int_type_uint
      ;
          ( MLDSType = mlds_mercury_array_type(_)
          ; MLDSType = mlds_cont_type(_)
@@ -146,15 +149,16 @@ is_integral_type(MLDSType) = IsIntegral :-
          ; MLDSType = mlds_tabling_type(_)
          ; MLDSType = mlds_unknown_type
          ),
-        IsIntegral = no
+        fail
      ;
          MLDSType = mercury_type(_, CtorCat, _),
-        (
-            ( CtorCat = ctor_cat_builtin(cat_builtin_int(_))
-            ; CtorCat = ctor_cat_builtin(cat_builtin_char)
+        require_complete_switch [CtorCat] (
+            ( CtorCat = ctor_cat_builtin(cat_builtin_char)
              ; CtorCat = ctor_cat_enum(cat_enum_mercury)
              ),
-            IsIntegral = yes
+            IntType = int_type_int
+        ;
+            CtorCat = ctor_cat_builtin(cat_builtin_int(IntType))
          ;
              ( CtorCat = ctor_cat_builtin(cat_builtin_string)
              ; CtorCat = ctor_cat_builtin(cat_builtin_float)
@@ -170,12 +174,12 @@ is_integral_type(MLDSType) = IsIntegral :-
              ; CtorCat = ctor_cat_user(cat_user_direct_dummy)
              ; CtorCat = ctor_cat_user(cat_user_abstract_dummy)
              ),
-            IsIntegral = no
+            fail
          ;
              CtorCat = ctor_cat_enum(cat_enum_foreign),
              % XXX We can switch on foreign enumerations in C, but this may
              % not be the case for the other target languages.
-            IsIntegral = no
+            fail
          )
      ).

diff --git a/compiler/ml_switch_gen.m b/compiler/ml_switch_gen.m
index 3f7770a..3d6899d 100644
--- a/compiler/ml_switch_gen.m
+++ b/compiler/ml_switch_gen.m
@@ -2,6 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 et
  %---------------------------------------------------------------------------%
  % Copyright (C) 1994-2012 The University of Melbourne.
+% Copyright (C) 2013-2018 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %---------------------------------------------------------------------------%
@@ -189,6 +190,11 @@ ml_gen_switch(SwitchVar, CanFail, Cases, CodeModel, Context, GoalInfo,
                  Stmts, !Info),
              Decls = []
          ;
+            SwitchCategory = int64_switch,
+            ml_gen_smart_int64_switch(SwitchVar, CanFail, TaggedCases,
+                CodeModel, Context, Stmts, !Info),
+            Decls = []
+        ;
              SwitchCategory = string_switch,
              ml_gen_smart_string_switch(SwitchVar, CanFail, TaggedCases,
                  CodeModel, Context, GoalInfo, Decls, Stmts, !Info)
@@ -261,6 +267,26 @@ ml_gen_smart_atomic_switch(SwitchVar, SwitchVarType, CanFail,
              SwitchVar, CodeModel, CanFail, Context, Stmts, !Info)
      ).

+:- pred ml_gen_smart_int64_switch(prog_var::in,
+    can_fail::in, list(tagged_case)::in, code_model::in, prog_context::in,
+    list(mlds_stmt)::out,
+    ml_gen_info::in, ml_gen_info::out) is det.
+
+ml_gen_smart_int64_switch(SwitchVar, CanFail,
+        TaggedCases, CodeModel, Context, Stmts, !Info) :-
+    ml_gen_info_get_module_info(!.Info, ModuleInfo),
+    module_info_get_globals(ModuleInfo, Globals),
+    Int64SwitchSupported = globals_target_supports_int64_switch(Globals),
+    (
+        Int64SwitchSupported = yes,
+        ml_switch_generate_mlds_switch(TaggedCases, SwitchVar,
+            CodeModel, CanFail, Context, Stmts, !Info)
+    ;
+        Int64SwitchSupported = no,
+        ml_switch_generate_if_then_else_chain(TaggedCases,
+            SwitchVar, CodeModel, CanFail, Context, Stmts, !Info)
+    ).
+
  :- pred ml_gen_smart_string_switch(prog_var::in, can_fail::in,
      list(tagged_case)::in, code_model::in, prog_context::in,
      hlds_goal_info::in, list(mlds_local_var_defn)::out, list(mlds_stmt)::out,
diff --git a/compiler/ml_target_util.m b/compiler/ml_target_util.m
index df16124..5b76e7c 100644
--- a/compiler/ml_target_util.m
+++ b/compiler/ml_target_util.m
@@ -2,6 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 et
  %---------------------------------------------------------------------------%
  % Copyright (C) 2011 The University of Melbourne.
+% Copyright (C) 2013-2015, 2017-2018 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %---------------------------------------------------------------------------%
@@ -18,18 +19,28 @@

  :- import_module libs.
  :- import_module libs.globals.
+:- import_module parse_tree.
+:- import_module parse_tree.prog_data.

  :- import_module bool.

+%---------------------------------------------------------------------------%
+
      % Return `yes' iff the target language supports the specified construct.
      %
+    % Note that for int switches one of our target languages (Java) does not
+    % support switching on 64-bit values, so we treat that case separately.
+    %
  :- func globals_target_supports_int_switch(globals) = bool.
+:- func globals_target_supports_int_type_switch(globals, int_type) = bool.
+:- func globals_target_supports_int64_switch(globals) = bool.
  :- func globals_target_supports_string_switch(globals) = bool.
  :- func globals_target_supports_goto(globals) = bool.
  :- func globals_target_supports_computed_goto(globals) = bool.
  :- func globals_target_supports_break_and_continue(globals) = bool.

  :- func target_supports_int_switch(compilation_target) = bool.
+:- func target_supports_int64_switch(compilation_target) = bool.
  :- func target_supports_string_switch(compilation_target) = bool.
  :- func target_supports_goto(compilation_target) = bool.
  :- func target_supports_computed_goto(compilation_target) = bool.
@@ -50,6 +61,30 @@ globals_target_supports_int_switch(Globals) = SupportsIntSwitch :-
      globals.get_target(Globals, Target),
      SupportsIntSwitch = target_supports_int_switch(Target).

+globals_target_supports_int64_switch(Globals) = SupportsInt64Switch :-
+    globals.get_target(Globals, Target),
+    SupportsInt64Switch = target_supports_int64_switch(Target).
+
+globals_target_supports_int_type_switch(Globals, IntType)
+        = SupportsIntTypeSwitch :-
+    (
+        ( IntType = int_type_int
+        ; IntType = int_type_uint
+        ; IntType = int_type_int8
+        ; IntType = int_type_uint8
+        ; IntType = int_type_int16
+        ; IntType = int_type_uint16
+        ; IntType = int_type_int32
+        ; IntType = int_type_uint32
+        ),
+        SupportsIntTypeSwitch = globals_target_supports_int_switch(Globals)
+    ;
+        ( IntType = int_type_int64
+        ; IntType = int_type_uint64
+        ),
+        SupportsIntTypeSwitch = globals_target_supports_int64_switch(Globals)
+    ).
+
  globals_target_supports_string_switch(Globals) = SupportsStringSwitch :-
      globals.get_target(Globals, Target),
      SupportsStringSwitch = target_supports_string_switch(Target).
@@ -74,6 +109,12 @@ target_supports_int_switch(target_java) = yes.
  target_supports_int_switch(target_erlang) =
      unexpected($module, $pred, "target erlang").

+target_supports_int64_switch(target_c) = yes.
+target_supports_int64_switch(target_csharp) = yes.
+target_supports_int64_switch(target_java) = no.
+target_supports_int64_switch(target_erlang) =
+    unexpected($module, $pred, "target erlang").
+
  target_supports_string_switch(target_c) = no.
  target_supports_string_switch(target_csharp) = yes.
  target_supports_string_switch(target_java) = yes.
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
@@ -2,6 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 et
  %-----------------------------------------------------------------------------%
  % Copyright (C) 1994-2012 The University of Melbourne.
+% Copyright (C) 2013-2018 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %-----------------------------------------------------------------------------%
@@ -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 =
diff --git a/compiler/switch_util.m b/compiler/switch_util.m
index d52aac0..be5f4a3 100644
--- a/compiler/switch_util.m
+++ b/compiler/switch_util.m
@@ -2,6 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 et
  %-----------------------------------------------------------------------------%
  % Copyright (C) 2000-2012 The University of Melbourne.
+% Copyright (C) 2013-2018 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %-----------------------------------------------------------------------------%
@@ -75,7 +76,14 @@
  %

  :- type switch_category
-    --->    atomic_switch   % a switch on int/char/enum
+    --->    atomic_switch
+            % A switch on int, char, enum or a 8-, 16 or 32-bit signed
+            % or unsigned ineger.
+
+    ;       int64_switch
+            % A switch on a 64-bit integer.
+            % These require special treatment on the Java backend.
+
      ;       string_switch
      ;       tag_switch
      ;       float_switch.
@@ -506,8 +514,26 @@ num_cons_ids_in_tagged_cases_loop([TaggedCase | TaggedCases],

  type_ctor_cat_to_switch_cat(CtorCat) = SwitchCat :-
      (
+        CtorCat = ctor_cat_builtin(cat_builtin_int(IntType)),
+        (
+            ( IntType = int_type_int
+            ; IntType = int_type_uint
+            ; IntType = int_type_int8
+            ; IntType = int_type_uint8
+            ; IntType = int_type_int16
+            ; IntType = int_type_uint16
+            ; IntType = int_type_int32
+            ; IntType = int_type_uint32
+            ),
+            SwitchCat = atomic_switch
+        ;
+            ( IntType = int_type_int64
+            ; IntType = int_type_uint64
+            ),
+            SwitchCat = int64_switch
+        )
+    ;
          ( CtorCat = ctor_cat_enum(_)
-        ; CtorCat = ctor_cat_builtin(cat_builtin_int(_))
          ; CtorCat = ctor_cat_builtin(cat_builtin_char)
          ),
          SwitchCat = atomic_switch
@@ -633,6 +659,11 @@ is_smart_indexing_allowed_for_category(Globals, SwitchCategory) = Allowed :-
      ;
          SwitchCategory = float_switch,
          globals.lookup_bool_option(Globals, smart_float_indexing, Allowed)
+    ;
+        SwitchCategory = int64_switch,
+        % We do not have a separate option for controlling smart indexing
+        % of 64-bit integers.
+        globals.lookup_bool_option(Globals, smart_atomic_indexing, Allowed)
      ).

  %-----------------------------------------------------------------------------%


More information about the reviews mailing list