[m-rev.] for review: new runtime option for setting fp rounding mode

[Paul Bone]

On Tue, Oct 28, 2008 at 07:07:44PM +1100, Julien Fischer wrote:
> Index: runtime/mercury_conf.h.in
> ===================================================================
> RCS file: 
> /home/mercury/mercury1/repository/mercury/runtime/mercury_conf.h.in,v
> retrieving revision 1.61
> diff -u -r1.61 mercury_conf.h.in
> --- runtime/mercury_conf.h.in	22 Jan 2008 02:36:37 -0000	1.61
> +++ runtime/mercury_conf.h.in	28 Oct 2008 06:56:42 -0000
> @@ -133,6 +133,7 @@
>  ** 	MR_HAVE_PTHREAD_H	we have <pthread.h>
>  **	MR_HAVE_TIME_H		we have <time.h>
>  ** 	MR_HAVE_SPAWN_H		we have <spawn.h>
> +**	MR_HAVE_FENV_H		we have <fenv.h>
>  */
>  #undef	MR_HAVE_SYS_SIGINFO_H
>  #undef	MR_HAVE_SYS_SIGNAL_H
> @@ -159,6 +160,7 @@
>  #undef	MR_HAVE_PTHREAD_H
>  #undef	MR_HAVE_TIME_H
>  #undef	MR_HAVE_SPAWN_H
> +#undef  MR_HAVE_FENV_H

The indenting looks different here.  Should it use tabs rather than
spaces?

> 
>  /*
>  ** MR_HAVE_POSIX_TIMES is defined if we have the POSIX
> @@ -256,6 +258,7 @@
>  **	MR_HAVE_PUTENV		we have the putenv() function.
>  **	MR_HAVE__PUTENV		we have the _putenv() function.
>  **	MR_HAVE_POSIX_SPAWN	we have the posix_spawn() function.
> +**	MR_HAVE_FESETROUND	we have the fesetround() function.
>  */
>  #undef	MR_HAVE_GETPID
>  #undef	MR_HAVE_SETPGID
> @@ -316,6 +319,7 @@
>  #undef	MR_HAVE_PUTENV
>  #undef	MR_HAVE__PUTENV
>  #undef	MR_HAVE_POSIX_SPAWN
> +#undef  MR_HAVE_FESETROUND
> 

And here.

> Index: runtime/mercury_wrapper.c
> ===================================================================
> RCS file: 
> /home/mercury/mercury1/repository/mercury/runtime/mercury_wrapper.c,v
> retrieving revision 1.193
> diff -u -r1.193 mercury_wrapper.c
> --- runtime/mercury_wrapper.c	16 Sep 2008 07:52:27 -0000	1.193
> +++ runtime/mercury_wrapper.c	28 Oct 2008 07:38:46 -0000
> @@ -1899,6 +1905,70 @@
>  #endif
>                  break;
> 
> +            case MR_FP_ROUNDING_MODE:
> +#if defined(MR_HAVE_FENV_H) && defined(MR_HAVE_FESETROUND)
> +                {
> +                    int     rounding_mode;
> + 
> +                    /*
> +                    ** Particular rounding modes are only supported if the
> +                    ** corresponding FE_* macro is defined.  The four 
> below are
> +                    ** the ones from C99.  C99 says that these macros
> +                    ** should expand to a nonnegative value, so we use a 
> negative value
> +                    ** to indicate that the selected rounding mode is not 
> supported by
> +                    ** the system.
> +                    */
> +                    if (MR_streq(MR_optarg, "downward")) {
> +                        #if defined(FE_DOWNWARD)
> +                            rounding_mode = FE_DOWNWARD;
> +                        #else
> +                            rounding_mode = -1;
> +                        #endif
> +                    } else if (MR_streq(MR_optarg, "upward")) {
> +                        #if defined(FE_UPWARD)
> +                            rounding_mode = FE_UPWARD;
> +                        #else
> +                            rounding_mode = -1;
> +                        #endif 
> +                    } else if (MR_streq(MR_optarg, "toward_zero")) {
> +                        #if defined(FE_TOWARDZERO)
> +                            rounding_mode = FE_TOWARDZERO;
> +                        #else
> +                            rounding_mode = -1;
> +                        #endif
> +                    } else if (MR_streq(MR_optarg, "to_nearest")) {
> +                        #if defined(FE_TONEAREST)
> +                            rounding_mode = FE_TONEAREST;
> +                        #else
> +                            rounding_mode = -1;
> +                        #endif
> +                    } else {
> +                        MR_usage();
> +                    }
> +
> +                    if (rounding_mode < 0) {
> +                        printf("Mercury runtime: the selected rounding 
> mode is "
> +                            "not supported by this system.\n");
> +                        fflush(stdout);
Do you need to flush stdout before exiting anyway?  doesn't the C
library handle this?  or does our runtime system prevent the C library
from doing this.

Should these errors go to stderr instead?

> +                        exit(1);
> +                    } else {
> +                        if (fesetround(rounding_mode) != 0) {
> +                            printf("Mercury runtime: could not establish 
> selected "
> +                                "rounding mode.\n");
> +                            fflush(stdout);
And here.

> +                            exit(1);
> +                        }
> +                   }
> +                }
> +#else
> +                printf("Mercury runtime: `--fp-rounding-mode' is specified 
> "
> +                    "in MERCURY_OPTIONS\n");
> +                printf("but the rounding mode cannot be changed on this 
> platform.\n");
> +                fflush(stdout);

And here.


Otherwise the patch is fine.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20081103/37918a48/attachment.sig>