Re: [Patch, fortran] Use BUILT_IN_IROUND
On Fri, Mar 16, 2012 at 9:17 AM, Janne Blomqvist wrote: > On Thu, Mar 15, 2012 at 22:28, Janne Blomqvist > wrote: >> On Thu, Mar 15, 2012 at 22:14, Tobias Burnus wrote: >>> Janne Blomqvist wrote: since some time GCC has BUILT_IN_IROUND{F,,L}, similar to lround() and llround() but the result is returned as an integer. Regtested on x86_64-unknown-linux-gnu, Ok for trunk? >>> >>> >>> OK. Thanks for the patch! Nit: Could you check mathbuiltins.def - at least >>> in the diff, "iround" seems to be misaligned (one " " missing). >> >> Ah, a tab had sneaked in, fixed. Committed the fixed patch as r185442. >> Thanks for the quick review! >> >> -- >> Janne Blomqvist > > I realized there was a bug in the patch, and testing revealed my > suspicion as true. Namely as there is no library fallback for iround, > and the middle-end machinery takes care of converting it to lround > only for float, double, and long double, it doesn't work for > __float128. In the testcase in my original patch mail, changing x to > real(16) generated a call to iroundq, which doesn't exist. Committed > the patch below as obvious. This seems to have broken SPEC 2000 FPU quite a lot as we now get /gcc/spec/sb-vangelis-head-64/x86_64/install-201203152354/lib/gcc/x86_64-unknown-linux-gnu/4.8.0/../../../../lib64/libgfortran.so: undefined reference to `iroundq' collect2: error: ld returned 1 exit status specmake: *** [apsi] Error 1 though maybe it was Jakubs patch as libgfortran has the reference? Jakub? Richard. > Index: ChangeLog > === > --- ChangeLog (revision 185452) > +++ ChangeLog (working copy) > @@ -1,3 +1,8 @@ > +2012-03-16 Janne Blomqvist > + > + * trans-intrinsic.c (build_round_expr): Don't use BUILT_IN_IROUND > + for __float128. > + > 2012-03-15 Janne Blomqvist > > * f95-lang.c (gfc_init_builtin_functions): Initialize > Index: trans-intrinsic.c > === > --- trans-intrinsic.c (revision 185452) > +++ trans-intrinsic.c (working copy) > @@ -383,10 +383,11 @@ build_round_expr (tree arg, tree restype > resprec = TYPE_PRECISION (restype); > > /* Depending on the type of the result, choose the int intrinsic > - (iround, available only as a builtin), long int intrinsic > (lround > - family) or long long intrinsic (llround). We might also need to > - convert the result afterwards. */ > - if (resprec <= INT_TYPE_SIZE) > + (iround, available only as a builtin, therefore cannot use it > for > + __float128), long int intrinsic (lround family) or long long > + intrinsic (llround). We might also need to convert the result > + afterwards. */ > + if (resprec <= INT_TYPE_SIZE && argprec <= LONG_DOUBLE_TYPE_SIZE) > fn = builtin_decl_for_precision (BUILT_IN_IROUND, argprec); > else if (resprec <= LONG_TYPE_SIZE) > fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec); > > > > -- > Janne Blomqvist
Re: [Patch, fortran] Use BUILT_IN_IROUND
On Thu, Mar 15, 2012 at 22:28, Janne Blomqvist wrote: > On Thu, Mar 15, 2012 at 22:14, Tobias Burnus wrote: >> Janne Blomqvist wrote: >>> >>> since some time GCC has BUILT_IN_IROUND{F,,L}, similar to lround() and >>> llround() but the result is returned as an integer. >>> >>> Regtested on x86_64-unknown-linux-gnu, Ok for trunk? >> >> >> OK. Thanks for the patch! Nit: Could you check mathbuiltins.def - at least >> in the diff, "iround" seems to be misaligned (one " " missing). > > Ah, a tab had sneaked in, fixed. Committed the fixed patch as r185442. > Thanks for the quick review! > > -- > Janne Blomqvist I realized there was a bug in the patch, and testing revealed my suspicion as true. Namely as there is no library fallback for iround, and the middle-end machinery takes care of converting it to lround only for float, double, and long double, it doesn't work for __float128. In the testcase in my original patch mail, changing x to real(16) generated a call to iroundq, which doesn't exist. Committed the patch below as obvious. Index: ChangeLog === --- ChangeLog (revision 185452) +++ ChangeLog (working copy) @@ -1,3 +1,8 @@ +2012-03-16 Janne Blomqvist + + * trans-intrinsic.c (build_round_expr): Don't use BUILT_IN_IROUND + for __float128. + 2012-03-15 Janne Blomqvist * f95-lang.c (gfc_init_builtin_functions): Initialize Index: trans-intrinsic.c === --- trans-intrinsic.c (revision 185452) +++ trans-intrinsic.c (working copy) @@ -383,10 +383,11 @@ build_round_expr (tree arg, tree restype resprec = TYPE_PRECISION (restype); /* Depending on the type of the result, choose the int intrinsic - (iround, available only as a builtin), long int intrinsic (lround - family) or long long intrinsic (llround). We might also need to - convert the result afterwards. */ - if (resprec <= INT_TYPE_SIZE) + (iround, available only as a builtin, therefore cannot use it for + __float128), long int intrinsic (lround family) or long long + intrinsic (llround). We might also need to convert the result + afterwards. */ + if (resprec <= INT_TYPE_SIZE && argprec <= LONG_DOUBLE_TYPE_SIZE) fn = builtin_decl_for_precision (BUILT_IN_IROUND, argprec); else if (resprec <= LONG_TYPE_SIZE) fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec); -- Janne Blomqvist
Re: [Patch, fortran] Use BUILT_IN_IROUND
On Thu, Mar 15, 2012 at 22:14, Tobias Burnus wrote: > Janne Blomqvist wrote: >> >> since some time GCC has BUILT_IN_IROUND{F,,L}, similar to lround() and >> llround() but the result is returned as an integer. >> >> Regtested on x86_64-unknown-linux-gnu, Ok for trunk? > > > OK. Thanks for the patch! Nit: Could you check mathbuiltins.def - at least > in the diff, "iround" seems to be misaligned (one " " missing). Ah, a tab had sneaked in, fixed. Committed the fixed patch as r185442. Thanks for the quick review! -- Janne Blomqvist
Re: [Patch, fortran] Use BUILT_IN_IROUND
Janne Blomqvist wrote: since some time GCC has BUILT_IN_IROUND{F,,L}, similar to lround() and llround() but the result is returned as an integer. Regtested on x86_64-unknown-linux-gnu, Ok for trunk? OK. Thanks for the patch! Nit: Could you check mathbuiltins.def - at least in the diff, "iround" seems to be misaligned (one " " missing). Tobias 2012-03-15 Janne Blomqvist * f95-lang.c (gfc_init_builtin_functions): Initialize BUILT_IN_IROUND. * mathbuiltins.def: Add IROUND. * trans-intrinsic.c (build_round_expr): Use BUILT_IN_IROUND if type size matches. (gfc_build_intrinsic_lib_fndecls): Build iround functions.
[Patch, fortran] Use BUILT_IN_IROUND
Hi, since some time GCC has BUILT_IN_IROUND{F,,L}, similar to lround() and llround() but the result is returned as an integer. As there is no corresponding libc function, this builtin is expanded to lround{f,l} except when -ffast-math is used, in which case it enables slightly shorter and faster code to be generated inline. Attached patch enables this builtin in the gfortran frontend. For the testcase function my_nint(x) implicit none real :: x integer :: my_nint my_nint = nint(x) end function my_nint compiled with "-O2 -ffast-math" on x86-64, the difference between the assembler output of trunk and trunk+patch: --- iround.trunk.O2fastmath.s 2012-03-15 20:12:40.045069324 +0200 +++ iround.s2012-03-15 20:24:19.501320278 +0200 @@ -12,7 +12,7 @@ my_nint_: movss .LC0(%rip), %xmm0 orps%xmm1, %xmm0 addss %xmm2, %xmm0 - cvttss2siq %xmm0, %rax + cvttss2si %xmm0, %eax ret .cfi_endproc .LFE0: Comparing the size of the object files, in the patched version the text section is one byte shorter. Regtested on x86_64-unknown-linux-gnu, Ok for trunk? (As an aside, there were some recent problems with __builtin_iround(), see PR 52592, but they seem to be fixed now so I think this should be safe) 2012-03-15 Janne Blomqvist * f95-lang.c (gfc_init_builtin_functions): Initialize BUILT_IN_IROUND. * mathbuiltins.def: Add IROUND. * trans-intrinsic.c (build_round_expr): Use BUILT_IN_IROUND if type size matches. (gfc_build_intrinsic_lib_fndecls): Build iround functions. -- Janne Blomqvist diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c index 05b598f..3f28e67 100644 --- a/gcc/fortran/f95-lang.c +++ b/gcc/fortran/f95-lang.c @@ -773,7 +773,11 @@ gfc_init_builtin_functions (void) gfc_define_builtin ("__builtin_fmodf", mfunc_float[1], BUILT_IN_FMODF, "fmodf", ATTR_CONST_NOTHROW_LEAF_LIST); - /* lround{f,,l} and llround{f,,l} */ + /* iround{f,,l}, lround{f,,l} and llround{f,,l} */ + ftype = build_function_type_list (integer_type_node, +float_type_node, NULL_TREE); + gfc_define_builtin("__builtin_iroundf", ftype, BUILT_IN_IROUNDF, + "iroundf", ATTR_CONST_NOTHROW_LEAF_LIST); ftype = build_function_type_list (long_integer_type_node, float_type_node, NULL_TREE); gfc_define_builtin ("__builtin_lroundf", ftype, BUILT_IN_LROUNDF, @@ -783,6 +787,10 @@ gfc_init_builtin_functions (void) gfc_define_builtin ("__builtin_llroundf", ftype, BUILT_IN_LLROUNDF, "llroundf", ATTR_CONST_NOTHROW_LEAF_LIST); + ftype = build_function_type_list (integer_type_node, +double_type_node, NULL_TREE); + gfc_define_builtin("__builtin_iround", ftype, BUILT_IN_IROUND, + "iround", ATTR_CONST_NOTHROW_LEAF_LIST); ftype = build_function_type_list (long_integer_type_node, double_type_node, NULL_TREE); gfc_define_builtin ("__builtin_lround", ftype, BUILT_IN_LROUND, @@ -792,6 +800,10 @@ gfc_init_builtin_functions (void) gfc_define_builtin ("__builtin_llround", ftype, BUILT_IN_LLROUND, "llround", ATTR_CONST_NOTHROW_LEAF_LIST); + ftype = build_function_type_list (integer_type_node, +long_double_type_node, NULL_TREE); + gfc_define_builtin("__builtin_iroundl", ftype, BUILT_IN_IROUNDL, + "iroundl", ATTR_CONST_NOTHROW_LEAF_LIST); ftype = build_function_type_list (long_integer_type_node, long_double_type_node, NULL_TREE); gfc_define_builtin ("__builtin_lroundl", ftype, BUILT_IN_LROUNDL, diff --git a/gcc/fortran/mathbuiltins.def b/gcc/fortran/mathbuiltins.def index b0bcc1f..f6d9586 100644 --- a/gcc/fortran/mathbuiltins.def +++ b/gcc/fortran/mathbuiltins.def @@ -64,6 +64,7 @@ OTHER_BUILTIN (FMOD, "fmod", 2, true) OTHER_BUILTIN (FREXP, "frexp", frexp, false) OTHER_BUILTIN (LLROUND, "llround", llround, true) OTHER_BUILTIN (LROUND,"lround",lround, true) +OTHER_BUILTIN (IROUND, "iround",iround, true) OTHER_BUILTIN (NEXTAFTER, "nextafter", 2, true) OTHER_BUILTIN (POW, "pow", 1, true) OTHER_BUILTIN (ROUND, "round", 1, true) diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index ac9f507..5e54d8e 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -376,28 +376,24 @@ build_round_expr (tree arg, tree restype) { tree argtype; tree fn; - bool longlong; int argprec, resprec; argtype = TREE_TYPE (arg); argprec = TYPE_PRECISION (argtype); resprec = TYPE_PRECISION (restype); - /* Depending on the type of the result, choose the long int intrinsic - (lround family) or long long intrinsic (llround). We might also - need to convert the result afterwards. */ - if (res