[gfortran,patch] Add option to raise FPE

François-Xavier Coudert fxcoudert@gmail.com
Sun Aug 21 13:28:00 GMT 2005


>> +  int * const opt_exception[] = { &(gfc_option.fpe_invalid),
>> +                                  &(gfc_option.fpe_denormal),
>> +                                  &(gfc_option.fpe_zero),
>> +                                  &(gfc_option.fpe_overflow),
>> +                                  &(gfc_option.fpe_underflow),
>> +                                  &(gfc_option.fpe_precision) };
> 
> Need/want static.  Why the extra parenthesis?

I'm not very good at understanding where static is needed. Will this
not mess things up if the function is called more than once? (right
now, it is not, but one never knows)

As for the extra parenthesis, I do find that more readable, but if
that's again coding standards, I will remove them.

>> +  gfor_fndecl_set_fpe =
>> +    gfc_build_library_function_decl (get_identifier (PREFIX("set_fpe")),
>> +                                 void_type_node, 6,
>> +                                 gfc_c_int_type_node, gfc_c_int_type_node,
>> +                                 gfc_c_int_type_node, gfc_c_int_type_node,
>> +                                 gfc_c_int_type_node, gfc_c_int_type_node);
> 
> Why 6 arguments and not one bitmask?

Why one bitmask and not 6 arguments? The code seemed pretty readable
in that way. Plus, it enables us to have more than two states for each
exception, which might be useful at some point.

>> +#define _FPU_GETCW(cw) __asm__ __volatile__ ("fnstcw %0" : "=m" (*&cw))
> 
> "*&" doesn't make sense, wherever you copied that from.

It's coming straight from a linux /usr/include/fpu_control.h header
file. I was intrigued by it, and as far as i could test, it works
pretty well without it too.

> Why separate i386/sse code?  You might as well just detect sse
> at runtime.  There are pleanty of examples of this in the code
> base already.

Could you point me to one such occurence? A grep of the GCC codebase
did not help me much...

> IMO you should have special code for glibc using feenableexcept,
> and use that preferentially on all gnu systems.  libgfortran has
> a fairly tight binding to libm already, so that should be no
> problem.

OK. That is a nice point, I guess we could have a configure test on
the presence of feenableexcept, with FPU-dependent assembler code used
when the first is not available.

>> -  {"GFORTRAN_FPU_INVALID", 1, &options.fpu_invalid, init_boolean,
>> +  {"GFORTRAN_FPU_INVALID", -1, &options.fpu_invalid, init_boolean,
>>     show_boolean,
>>     "Raise a floating point exception on invalid FP operation.", 0},
> 
> Surely you want the command-line argument instead of an environment
> variable, and not some tricky interaction between them?

I personnaly favor both the command-line argument and environment
variable (the interaction is not tricky: environment has precedence
over compile-time options), and the code for reading these environment
variables was already present in the library. That said, if the
general opinion is to remove these environment variables, I can do
that easily.

Thanks for your comments. Any other (especially on the last point) welcome.
FX



More information about the Gcc-patches mailing list