[m-rev.] for review: fix float precision bug

[Fergus Henderson]

Hmm, no review comments so far ;-)

Pete's change to io__write_float and string__float_to_string
also fixes the same bug.  However, I propose to still commit this change.
As well as fixing the bug (which is no longer necessary), it does three things:

	- adds a test case
	- adds some comments
	- centralizes the code which handles output of floating point literals

These three things are all still useful.  The code reorganization which
centralizes the floating point literal handling will be useful in fixing
the bug where we output infs and nans incorrectly.

So, any objections to my committing this one?

On 06-Nov-2002, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> This fixes a bug reported by Peter Moulder.
> 
> Estimated hours taken: 4
> Branches: main
> 
> Reorganize the code in the compiler to centralize the code which handles
> output of floating point literals, and fix a bug where we were not outputting
> them with sufficient precision.
> 
> compiler/c_util.m:
> 	Add new routines make_float_literal and output_float_literal.
> 	These output to 17 digits precision.
> 
> compiler/mlds_to_c.m:
> compiler/llds_out.m:
> compiler/mlds_to_java.m:
> compiler/ilasm.m:
> 	Use the new routines.
> 
> tests/hard_coded/Mmakefile:
> tests/hard_coded/float_consistency.m:
> tests/hard_coded/float_consistency.exp:
> 	A regression test.
> 
> Workspace: /home/ceres/fjh/mercury
> Index: compiler/c_util.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/c_util.m,v
> retrieving revision 1.11
> diff -u -d -r1.11 c_util.m
> --- compiler/c_util.m	6 Aug 2002 00:30:46 -0000	1.11
> +++ compiler/c_util.m	22 Oct 2002 05:50:48 -0000
> @@ -8,8 +8,10 @@
>  % Main author: fjh.
>  
>  % This module defines utility routines that are useful when
> -% generating and/or emitting C code.  Changes to this module may require
> -% changes to be made to java_util.m
> +% generating and/or emitting C code.  Some of these routines are
> +% also useful with other languages whose syntax is similar to C.
> +
> +% NOTE: changes to this module may require changes to be made to java_util.m.
>  
>  %-----------------------------------------------------------------------------%
>  
> @@ -19,6 +21,9 @@
>  :- import_module backend_libs__builtin_ops.
>  
>  %-----------------------------------------------------------------------------%
> +%
> +% Line numbering.
> +%
>  
>  	% set_line_num(FileName, LineNum):
>  	%	emit a #line directive to set the specified filename & linenum
> @@ -34,6 +39,9 @@
>  :- mode c_util__reset_line_num(di, uo) is det.
>  
>  %-----------------------------------------------------------------------------%
> +%
> +% String and character handling.
> +%
>  
>  	% Print out a string suitably escaped for use as a C string literal.
>  	% This doesn't actually print out the enclosing double quotes --
> @@ -69,6 +77,22 @@
>  
>  %-----------------------------------------------------------------------------%
>  %
> +% Float literals.
> +%
> +
> +	% Convert a float to a string suitable for use as a C (or Java, or IL)
> +	% floating point literal.
> +:- func c_util__make_float_literal(float) = string.
> +
> +	% As above, but write the string to the current output stream
> +	% rather than returning it.
> +:- pred c_util__output_float_literal(float::in, io__state::di, io__state::uo)
> +	is det.
> +
> +%-----------------------------------------------------------------------------%
> +%
> +% Operators.
> +%
>  % The following predicates all take as input an operator,
>  % check if it is an operator of the specified kind,
>  % and if so, return the name of the corresponding C operator
> @@ -117,6 +141,9 @@
>  :- import_module list, bool.
>  
>  %-----------------------------------------------------------------------------%
> +%
> +% Line numbering.
> +%
>  
>  c_util__set_line_num(File, Line) -->
>  	globals__io_lookup_bool_option(line_numbers, LineNumbers),
> @@ -156,6 +183,12 @@
>  	).
>  
>  %-----------------------------------------------------------------------------%
> +%
> +% String and character handling.
> +%
> +% XXX we should check to ensure that we don't accidentally generate
> +%     trigraph sequences in string literals.
> +%
>  
>  c_util__output_quoted_string(S0) -->
>  	c_util__output_quoted_multi_string(string__length(S0), S0).
> @@ -251,6 +284,8 @@
>  :- mode escape_any_char(in, out) is det.
>  
>          % Convert a character to the corresponding C octal escape code.
> +	% XXX This assumes that the target language compiler's representation
> +	%     of characters is the same as the Mercury compiler's.
>  escape_any_char(Char, EscapeCodeChars) :-
>          char__to_int(Char, Int),
>          string__int_to_base_string(Int, 8, OctalString0),
> @@ -258,6 +293,28 @@
>          EscapeCodeChars = ['\\' | string__to_char_list(OctalString)].
>  
>  %-----------------------------------------------------------------------------%
> +%
> +% Floating point literals.
> +%
> +% XXX These routines do not yet handle infinities and NaNs properly.
> +
> +	% This is used by the C, Java, and IL back-ends,
> +	% so the output must be valid syntax in all three languages.
> +	%
> +	% We output literals using 17 digits of precision.
> +	% This is the minimum needed to be able to convert IEEE
> +	% double-precision floating point values to strings and
> +	% back again without losing precision.
> +	%
> +make_float_literal(Float) = string__format("%#.17g", [f(Float)]).
> +
> +output_float_literal(Float) -->
> +	io__write_string(make_float_literal(Float)).
> +
> +%-----------------------------------------------------------------------------%
> +%
> +% Operators.
> +%
>  
>  c_util__unary_prefix_op(mktag,			"MR_mktag").
>  c_util__unary_prefix_op(tag,			"MR_tag").
> Index: compiler/ilasm.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ilasm.m,v
> retrieving revision 1.34
> diff -u -d -r1.34 ilasm.m
> --- compiler/ilasm.m	20 Mar 2002 12:36:22 -0000	1.34
> +++ compiler/ilasm.m	22 Oct 2002 05:47:40 -0000
> @@ -274,6 +274,7 @@
>  :- import_module char, string, pprint, getopt.
>  :- import_module require, int, term_io, varset, bool.
>  :- import_module libs__globals, libs__options, hlds__error_util.
> +:- import_module backend_libs__c_util. % for output_float_literal
>  
>  
>  	% Some versions of the IL assembler enforce a rule that if you output 
> @@ -1144,10 +1145,10 @@
>  		io__write_int(IntConst)
>  	; { Type = float32, Const = f(FloatConst) } ->
>  		io__write_string("ldc.r4\t"),
> -		io__write_float(FloatConst)
> +		c_util__output_float_literal(FloatConst)
>  	; { Type = float64, Const = f(FloatConst) } ->
>  		io__write_string("ldc.r8\t"),
> -		io__write_float(FloatConst)
> +		c_util__output_float_literal(FloatConst)
>  	;
>  	 	{ error("Inconsistent arguments in ldc instruction") }
>  	).
> Index: compiler/llds_out.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/llds_out.m,v
> retrieving revision 1.201
> diff -u -d -r1.201 llds_out.m
> --- compiler/llds_out.m	29 Sep 2002 10:30:41 -0000	1.201
> +++ compiler/llds_out.m	6 Nov 2002 03:32:15 -0000
> @@ -2257,8 +2257,8 @@
>  			;
>  				{ decl_set_insert(DeclSet0, FloatLabel,
>  					DeclSet) },
> -				{ string__float_to_string(FloatVal,
> -					FloatString) },
> +				{ FloatString = c_util__make_float_literal(
> +					FloatVal) },
>  				output_indent(FirstIndent, LaterIndent, N0),
>  				{ N = N0 + 1 },
>  				io__write_strings([
> @@ -2422,7 +2422,7 @@
>  	% value of the float const, with "pt" instead
>  	% of ".", "plus" instead of "+", and "neg" instead of "-".
>  	%
> -	string__float_to_string(Float, FloatName0),
> +	FloatName0 = c_util__make_float_literal(Float),
>  	string__replace_all(FloatName0, ".", "pt", FloatName1),
>  	string__replace_all(FloatName1, "+", "plus", FloatName2),
>  	string__replace_all(FloatName2, "-", "neg", FloatName).
> @@ -4037,7 +4037,7 @@
>  	% do arithmetic in `float' rather than `double'
>  	% if `Float' is `float' not `double'.
>  	output_llds_type_cast(float),
> -	io__write_float(FloatVal).
> +	c_util__output_float_literal(FloatVal).
>  output_rval_const(string_const(String)) -->
>  	io__write_string("MR_string_const("""),
>  	output_c_quoted_string(String),
> @@ -4113,7 +4113,7 @@
>  output_rval_static_const(int_const(N)) -->
>  	io__write_int(N).
>  output_rval_static_const(float_const(FloatVal)) -->
> -	io__write_float(FloatVal).
> +	c_util__output_float_literal(FloatVal).
>  output_rval_static_const(string_const(String)) -->
>  	io__write_string("MR_string_const("""),
>  	output_c_quoted_string(String),
> @@ -4427,6 +4427,9 @@
>  	(
>  		string__first_char(String, Char, Rest)
>  	->
> +		% XXX This will cause ABI incompatibilities between
> +		%     compilers which are built in grades that have
> +		%     different character representations.
>  		char__to_int(Char, Code),
>  		string__int_to_string(Code, CodeString),
>  		string__append("_", CodeString, ThisCharString),
> Index: compiler/mlds_to_c.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mlds_to_c.m,v
> retrieving revision 1.139
> diff -u -d -r1.139 mlds_to_c.m
> --- compiler/mlds_to_c.m	4 Oct 2002 10:02:30 -0000	1.139
> +++ compiler/mlds_to_c.m	4 Nov 2002 05:04:33 -0000
> @@ -3387,7 +3387,7 @@
>  	% do arithmetic in `float' rather than `double'
>  	% if `MR_Float' is `float' not `double'.
>  	io__write_string("(MR_Float) "),
> -	io__write_float(FloatVal).
> +	c_util__output_float_literal(FloatVal).
>  mlds_output_rval_const(string_const(String)) -->
>  	% the cast avoids the following gcc warning
>  	% "assignment discards qualifiers from pointer target type"
> Index: compiler/mlds_to_java.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mlds_to_java.m,v
> retrieving revision 1.32
> diff -u -d -r1.32 mlds_to_java.m
> --- compiler/mlds_to_java.m	1 Aug 2002 11:52:20 -0000	1.32
> +++ compiler/mlds_to_java.m	21 Oct 2002 08:21:01 -0000
> @@ -60,8 +60,10 @@
>  
>  :- import_module ml_backend__ml_util.
>  :- import_module ml_backend__java_util. 
> -:- import_module backend_libs__c_util.	% XXX needed for c_util__output_quoted_string
> -				% c_util_output_quoted_multi_string
> +:- import_module backend_libs__c_util.
> +	% XXX needed for c_util__output_quoted_string,
> +	%     c_util__output_quoted_multi_string, and
> +	%     c_util__make_float_literal.
>  :- import_module ll_backend__llds_out.	% XXX needed for llds_out__name_mangle,
>  				% llds_out__sym_name_mangle,
>  				% llds_out__make_base_typeclass_info_name,
> @@ -2932,7 +2934,7 @@
>  	io__write_int(N).
>  
>  output_rval_const(float_const(FloatVal)) -->
> -	io__write_float(FloatVal).
> +	c_util__output_float_literal(FloatVal).
>  
>  output_rval_const(string_const(String)) -->
>  	io__write_string(""""),
> Index: tests/hard_coded/Mmakefile
> ===================================================================
> RCS file: /home/mercury1/repository/tests/hard_coded/Mmakefile,v
> retrieving revision 1.173
> diff -u -d -r1.173 Mmakefile
> --- tests/hard_coded/Mmakefile	23 Oct 2002 13:41:52 -0000	1.173
> +++ tests/hard_coded/Mmakefile	6 Nov 2002 03:21:27 -0000
> @@ -50,6 +50,7 @@
>  	export_test \
>  	factt \
>  	failure_unify \
> +	float_consistency \
>  	float_field \
>  	float_map \
>  	float_reg \
> Index: tests/hard_coded/float_consistency.exp
> ===================================================================
> RCS file: tests/hard_coded/float_consistency.exp
> diff -N tests/hard_coded/float_consistency.exp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/hard_coded/float_consistency.exp	6 Nov 2002 03:20:11 -0000
> @@ -0,0 +1,2 @@
> +Calc_one     = Lit_one:     true
> +Calc_one/9.0 = Lit_one/9.0: true
> Index: tests/hard_coded/float_consistency.m
> ===================================================================
> RCS file: tests/hard_coded/float_consistency.m
> diff -N tests/hard_coded/float_consistency.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/hard_coded/float_consistency.m	6 Nov 2002 03:29:37 -0000
> @@ -0,0 +1,23 @@
> +% This test case tests that the compiler's constant propagation for
> +% floating point division produce the same value as that produced
> +% by doing the division at runtime.
> +% This is a regression test; versions of the compiler prior to Nov 2002
> +% failed this test at -O3 and higher.
> +:- module float_consistency.
> +:- interface.
> +:- import_module io.
> +:- pred main(state::di, state::uo) is det.
> +
> +:- implementation.
> +:- import_module float, string.
> +main -->
> +        { Lit_one = 1.0 },
> +        { Calc_one = same_as(Lit_one) },
> +        print("Calc_one     = Lit_one:     " ++ 
> +	     (if Calc_one = Lit_one then "true" else "false")), nl,
> +        print("Calc_one/9.0 = Lit_one/9.0: " ++
> +	     (if Calc_one/9.0 = Lit_one/9.0 then "true" else "false")), nl.
> +
> +:- pragma no_inline(same_as/1).
> +:- func same_as(float) = float.
> +same_as(X) = X.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------