[m-rev.] for review: Fix -O<n> options below default optimisation level.

Peter Wang novalazy at gmail.com
Mon Jun 7 14:13:41 AEST 2021


The default optimisation level was implemented by having the
Mercury.config file add a -O<n> option to the DEFAULT_MCFLAGS variable.
This allowed the user to override the default optimisation level
by setting the MERCURY_DEFAULT_OPT_LEVEL environment variable.

Commit 181ada0dbf2899b388ae9155083a3050dd027cc1 made it so that -O<n>
options do not reset previously set options. This had the unintended
consequence that any -O<n> options below the default optimisation level
had no effect when passed on the command line or in a Mercury.options
file, because a -O<n> option passed by the user would never undo the
options set by the default optimisation level.

compiler/options.m:
    Add new option `--default-opt-level' for use by Mercury.config.

tools/make_optimization_options_end:
    Add predicate to parse the value of `--default-opt-level' from the
    options table. The option value is a string because the value of the
    MERCURY_DEFAULT_OPT_LEVEL environment variable will be passed as
    the option value. The existence of the environment variable was
    (barely) documented, but the allowable values are not.
    Though the user might conceivably have set any Mercury compiler
    option in that environment variable, the value was probably only
    intended to be a string of the form "-O<int>", and that is all we
    will support.

tools/make_optimization_options_middle:
    Use `get_default_opt_level'.

compiler/optimization_options.m:
    Regenerate this file.

compiler/options_file.m:
    Do not automatically add "-O2" to MCFlags.
    The comment says this was to set a default optimisation level when
    calling the `mercury_compile' binary instead of the `mmc' script,
    but `get_default_opt_level' already defaults to optimisation level 2.

scripts/Mercury.config.in:
    Pass the value of MERCURY_DEFAULT_OPT_LEVEL using the
    `--default-opt-level` option.

diff --git a/compiler/optimization_options.m b/compiler/optimization_options.m
index 58467b851..6df3dd5fc 100644
--- a/compiler/optimization_options.m
+++ b/compiler/optimization_options.m
@@ -561,6 +561,7 @@
 :- import_module getopt.
 :- import_module int.
 :- import_module map.
+:- import_module string.
 
 %---------------------%
 
@@ -571,7 +572,7 @@ process_optimization_options(OptionTable, OptOptions, !:OptTuple) :-
         OptOptions, !OptTuple, not_seen_opt_level, MaybeSeenOptLevel),
     (
         MaybeSeenOptLevel = not_seen_opt_level,
-        DefaultOptLevel = 2,
+        get_default_opt_level(OptionTable, DefaultOptLevel),
         set_opts_upto_level(OptionTable, 0, DefaultOptLevel,
             !OptTuple, MaybeSeenOptLevel, _)
     ;
@@ -3632,6 +3633,21 @@ update_opt_tuple_int_procs_per_c_function(FromOptLevel, N, !OptTuple) :-
     ).
 
 
+:- pred get_default_opt_level(option_table::in, int::out) is det.
+
+get_default_opt_level(OptionTable, DefaultOptLevel) :-
+    % default_opt_level takes a "-O<n>" string for compatibility.
+    lookup_string_option(OptionTable, default_opt_level, Str0),
+    Str = string.strip(Str0),
+    ( if
+        string.remove_prefix("-O", Str, Suffix),
+        string.to_int(string.lstrip(Suffix), Int)
+    then
+        DefaultOptLevel = Int
+    else
+        DefaultOptLevel = 2
+    ).
+
 :- pred set_opts_upto_level(option_table::in, int::in, int::in,
     opt_tuple::in, opt_tuple::out,
     maybe_seen_opt_level::in, maybe_seen_opt_level::out) is det.
diff --git a/compiler/options.m b/compiler/options.m
index fd4cf2c5f..751c7637a 100644
--- a/compiler/options.m
+++ b/compiler/options.m
@@ -2,7 +2,7 @@
 % vim: ft=mercury ts=4 sw=4 et wm=0
 %---------------------------------------------------------------------------%
 % Copyright (C) 1994-2012 The University of Melbourne.
-% Copyright (C) 2013-2020 The Mercury team.
+% Copyright (C) 2013-2021 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.
 %---------------------------------------------------------------------------%
@@ -873,6 +873,7 @@
 
     % Special optimization options.
 
+    ;       default_opt_level
     ;       opt_level
     ;       opt_space                   % Default is to optimize time.
     ;       intermodule_optimization
@@ -1661,6 +1662,7 @@ optdef(oc_codegen, debug_class_init,                    bool(no)).
     % Special optimization options.
     % These ones are not affected by `-O<n>'.
 
+optdef(oc_spec_opt, default_opt_level,                  string("-O2")).
 optdef(oc_spec_opt, opt_level,                          int_special).
 optdef(oc_spec_opt, opt_space,                          special).
 optdef(oc_spec_opt, intermodule_optimization,           bool(no)).
@@ -2608,6 +2610,7 @@ long_option("debug-class-init",     debug_class_init).
 
 % optimization options
 
+long_option("default-opt-level",    default_opt_level).
 long_option("opt-level",            opt_level).
 long_option("optimization-level",   opt_level).
 long_option("optimisation-level",   opt_level).
@@ -5748,6 +5751,9 @@ options_help_code_generation(!IO) :-
 options_help_optimization(!IO) :-
     io.write_string("\nOptimization Options:\n", !IO),
     write_tabbed_lines([
+        % This is for use by Mercury.config only.
+    %   "--default-opt-level -O<n>",
+    %   "\tSet the default optimization level to <n>.",
         "-O <n>, --opt-level <n>, --optimization-level <n>",
         "\tSet optimization level to <n>.",
         "\tOptimization level -1 means no optimization",
diff --git a/compiler/options_file.m b/compiler/options_file.m
index 10795cd2f..f69b001a8 100644
--- a/compiler/options_file.m
+++ b/compiler/options_file.m
@@ -1267,10 +1267,8 @@ lookup_mmc_maybe_module_options(Variables, MaybeModuleName,
     list.map_foldl2(
         lookup_options_variable(Variables, MaybeModuleName),
         VariableTypes, VariableTypesMaybeValues, [], Specs, !IO),
-    % Default to `-O2', even when mercury_compile is called directly,
-    % not by the mmc script.
-    Result = ["-O2" | list.condense(
-        list.map(convert_to_mmc_options, VariableTypesMaybeValues))].
+    Result = list.condense(
+        list.map(convert_to_mmc_options, VariableTypesMaybeValues)).
 
 :- type options_variable_class
     --->    default
diff --git a/scripts/Mercury.config.in b/scripts/Mercury.config.in
index e63ead651..36ff98b24 100644
--- a/scripts/Mercury.config.in
+++ b/scripts/Mercury.config.in
@@ -5,7 +5,7 @@
 # @configure_input@
 #---------------------------------------------------------------------------#
 # Copyright (C) 2003-2007, 2010-2012 The University of Melbourne.
-# Copyright (C) 2013-2016, 2018-2019 The Mercury team.
+# Copyright (C) 2013-2016, 2018-2021 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.
 #---------------------------------------------------------------------------#
@@ -50,8 +50,6 @@ DEFAULT_MERCURY_LINKAGE=@DEFAULT_LINKAGE@
 
 DEFAULT_GRADEFLAGS=--grade "$(MERCURY_DEFAULT_GRADE)"
 
-# The default optimization level should be after
-# all the options that describe the machine configuration.
 DEFAULT_MCFLAGS=\
 		@ALL_LOCAL_C_INCL_DIR_MMC_OPTS@ \
 		@ALL_LOCAL_C_LIB_DIR_MMC_OPTS@ \
@@ -132,10 +130,10 @@ DEFAULT_MCFLAGS=\
 		--sync-term-size "@SYNC_TERM_SIZE@" \
 		--host-env-type "@HOST_ENV_TYPE@" \
 		--target-env-type "@TARGET_ENV_TYPE@" \
+		--default-opt-level "$(MERCURY_DEFAULT_OPT_LEVEL)" \
 		@RESTRICTED_COMMAND_LINE_OPT@ \
 		@HAVE_DELAY_SLOT@ \
 		@HAVE_BOXED_FLOATS@ \
 		@HAVE_BOXED_INT64S@ \
 		@MCFLAGS_FOR_CC@ \
-		$(MERCURY_DEFAULT_OPT_LEVEL) \
 		@MMC_USE_SYMLINKS_OPT@
diff --git a/tools/make_optimization_options_end b/tools/make_optimization_options_end
index 92af5c4a2..2c830507b 100644
--- a/tools/make_optimization_options_end
+++ b/tools/make_optimization_options_end
@@ -1,4 +1,19 @@
 
+:- pred get_default_opt_level(option_table::in, int::out) is det.
+
+get_default_opt_level(OptionTable, DefaultOptLevel) :-
+    % default_opt_level takes a "-O<n>" string for compatibility.
+    lookup_string_option(OptionTable, default_opt_level, Str0),
+    Str = string.strip(Str0),
+    ( if
+        string.remove_prefix("-O", Str, Suffix),
+        string.to_int(string.lstrip(Suffix), Int)
+    then
+        DefaultOptLevel = Int
+    else
+        DefaultOptLevel = 2
+    ).
+
 :- pred set_opts_upto_level(option_table::in, int::in, int::in,
     opt_tuple::in, opt_tuple::out,
     maybe_seen_opt_level::in, maybe_seen_opt_level::out) is det.
diff --git a/tools/make_optimization_options_middle b/tools/make_optimization_options_middle
index 45bc712e4..1d1dd2389 100755
--- a/tools/make_optimization_options_middle
+++ b/tools/make_optimization_options_middle
@@ -35,7 +35,6 @@
 #
 
 BEGIN {
-        default_opt_level = 2;
         next_bool_opt = 0;
         next_int_opt = 0;
         next_string_opt = 0;
@@ -165,6 +164,7 @@ END {
         printf(":- import_module getopt.\n");
         printf(":- import_module int.\n");
         printf(":- import_module map.\n");
+        printf(":- import_module string.\n");
 
         printf("\n%%---------------------%%\n\n");
 
@@ -175,7 +175,7 @@ END {
         printf("%-8sOptOptions, !OptTuple, not_seen_opt_level, MaybeSeenOptLevel),\n", "");
         printf("%-4s(\n", "");
         printf("%-8sMaybeSeenOptLevel = not_seen_opt_level,\n", "");
-        printf("%-8sDefaultOptLevel = %d,\n", "", default_opt_level);
+        printf("%-8sget_default_opt_level(OptionTable, DefaultOptLevel),\n", "");
         printf("%-8sset_opts_upto_level(OptionTable, 0, DefaultOptLevel,\n",
             "");
         printf("%-12s!OptTuple, MaybeSeenOptLevel, _)\n", "");
-- 
2.31.1



More information about the reviews mailing list