Re: Fix Fortran rounding issues, PR fortran/96983.
On 20.04.21 08:58, Richard Biener via Fortran wrote:
On Mon, Apr 19, 2021 at 9:40 PM Michael Meissner via Fortran
wrote:
Is there any reason to not only send the email to fortran@ _and_
gcc-patches@ but sending it to 13 Fortran maintainers explicitly? (Now
removed)
Fix Fortran rounding issues, PR fortran/96983.
Can I check this change into the GCC trunk?
The patch looks fine technically and is definitely an improvement since the
intermediate conversion looks odd. But it might be that the standard
requires such dance though the preceeding cases handled don't seem to
care. I'm thinking of a FP format where round(1.6) == 3 because of lack
of precision but using an intermediate FP format with higher precision
would "correctly" compute 2.
The patched build_round_expr is only called by ANINT / NINT;
NINT is real → integer; ANINT is real → real
[And the modified code is only called for NINT, reason: see comment far below.]
NINT (A[, KIND]) is described (F2018) as "Nearest integer":
* Result Characteristics. Integer. If KIND is present, the kind type parameter
is that specified by the value of KIND;
otherwise, the kind type parameter is that of default integer type.
* The result is the integer nearest A, or if there are two
integers equally near A, the result is whichever such integer has the greater
magnitude.
* Example. NINT (2.783) has the value 3.
ANINT (A[, KIND]) as "Nearest whole number":
* The result is of type real. If KIND is present, the kind type parameter is
that
specified by the value of KIND; otherwise, the kind type parameter is that of
A.
* The result is the integer nearest A, or if there are two integers equally
near A,
the result is whichever such integer has the greater magnitude.
* Examples. ANINT (2.783) has the value 3.0. ANINT (−2.783) has the value −3.0.
Of course the current code doesn't handle this correctly for the
case if llroundf either.
I've not contributed to the Fortran front end before. If the maintainers like
the patch, can somebody point out if I need to do additional things to commit
the patch?
Nothing special: a testcase already exists, committing is done as usual
and a PR to update you have as well.
gcc/fortran/
2021-04-19 Michael Meissner
PR gfortran/96983
* trans-intrinsic.c (build_round_expr): If int type is larger than
long long, do the round and convert to the integer type. Do not
try to find a floating point type the exact size of the integer
type.
LGTM. (Minor remark below.)
---
gcc/fortran/trans-intrinsic.c | 26 --
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 5e53d1162fa..cceef8f34ac 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -386,30 +386,20 @@ build_round_expr (tree arg, tree restype)
argprec = TYPE_PRECISION (argtype);
resprec = TYPE_PRECISION (restype);
- /* Depending on the type of the result, choose the int intrinsic
- (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. */
+ /* Depending on the type of the result, choose the int intrinsic (iround,
+ available only as a builtin, therefore cannot use it for __float128), long
+ int intrinsic (lround family) or long long intrinsic (llround). If we
+ don't have an appropriate function that converts directly to the integer
+ type (such as kind == 16), just use ROUND, and then convert the result to
+ an integer. We might also need to convert the result afterwards. */
I think that's fine.
Small comment for completeness:
"and convert the result to an integer"
Technically, this function can be called for both 'anint' and 'nint'
and for 'anint' it needs a real – and wouldn't convert to an integer.
However, as gfc_conv_intrinsic_aint only calls this function when
gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
is NULL_TREE. I think the new code can not be reached – and if, it
would give an ICE as the same call is used.
As effectively the comment is correct, you can leave it as is.
Tobias
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);
else if (resprec <= LONG_LONG_TYPE_SIZE)
fn = builtin_decl_for_precision (BUILT_IN_LLROUND, argprec);
- else if (resprec >= argprec && resprec == 128)
-{
- /* Search for a real kind suitable as temporary for conversion. */
- int kind = -1;
- for (int i = 0; kind < 0 && gfc_real_kinds[i].kind != 0; i++)
- if (gfc_real_kinds[i].mode_precision >= resprec)
- kind = gfc_real_kinds[i].kind;
- if (kind <
Re: static libgfortran linker warnings
On Wed, 21 Apr 2021 02:34:43 -0400 NightStrike via Fortran wrote: > When linking with -static-libgfortran, I get warnings from ld of the form > "ld: warning: -z ignore ignored" and "ld: warning: -z record ignored". I > can't find those -z options documented anywhere. Why is gfortran adding > them? -z are options to ld. Which linker do you use, on which OS? If you use binutils, you can pass -Wl,--verbose to the compiler to instruct the linker to dump the linker script while linking. This should show where the -z comes from: gfortran foo.f90 -static-libgfortran -Wl,--verbose
Re: static libgfortran linker warnings
On Wed, Apr 21, 2021 at 4:59 AM Bernhard Reutner-Fischer wrote: > On Wed, 21 Apr 2021 02:34:43 -0400 > NightStrike via Fortran wrote: > > > When linking with -static-libgfortran, I get warnings from ld of the form > > "ld: warning: -z ignore ignored" and "ld: warning: -z record ignored". I > > can't find those -z options documented anywhere. Why is gfortran adding > > them? > > -z are options to ld. > Which linker do you use, on which OS? binutils 2.32 (stock build from source) on CentOS 7. > If you use binutils, you can pass -Wl,--verbose to the compiler to > instruct the linker to dump the linker script while linking. This should > show where the -z comes from: > gfortran foo.f90 -static-libgfortran -Wl,--verbose I didn't do this per se, but I did gfortran -v, and saw that it was an option to collect2. I can try your method when the system is available again tomorrow.
Re: static libgfortran linker warnings
On Wed, 21 Apr 2021 05:29:28 -0400
NightStrike wrote:
> On Wed, Apr 21, 2021 at 4:59 AM Bernhard Reutner-Fischer
> wrote:
> > On Wed, 21 Apr 2021 02:34:43 -0400
> > NightStrike via Fortran wrote:
> >
> > > When linking with -static-libgfortran, I get warnings from ld of the form
> > > "ld: warning: -z ignore ignored" and "ld: warning: -z record ignored". I
> > > can't find those -z options documented anywhere. Why is gfortran adding
> > > them?
> >
> > -z are options to ld.
> > Which linker do you use, on which OS?
>
> binutils 2.32 (stock build from source) on CentOS 7.
>
> > If you use binutils, you can pass -Wl,--verbose to the compiler to
> > instruct the linker to dump the linker script while linking. This should
> > show where the -z comes from:
> > gfortran foo.f90 -static-libgfortran -Wl,--verbose
>
> I didn't do this per se, but I did gfortran -v, and saw that it was an
> option to collect2. I can try your method when the system is
> available again tomorrow.
gcc/configure.ac suggests that -z ignore / -z record is the native
solaris ld parlance for --as-needed / --no-as-needed
See "AC_CACHE_CHECK(linker --as-needed support".
No idea why your gcc build thought that the ignore/record tuple is the
correct thing to use for as-needed/no-as-needed. Maybe check your
config.log around the '--as-needed support' checks for clues.
I wouldn't suggest to hand edit your libgfortran.spec to use
*lib: %{static-libgfortran:--as-needed} -lquadmath
%{static-libgfortran:--no-as-needed} -lm %(libgcc) %(liborig)
but i guess that would silence your linker warnings, too.
HTH,
Re: static libgfortran linker warnings
On Wed, 21 Apr 2021 02:34:43 -0400
NightStrike via Fortran wrote:
When linking with -static-libgfortran, I get warnings from ld of the form
"ld: warning: -z ignore ignored" and "ld: warning: -z record ignored". I
can't find those -z options documented anywhere. Why is gfortran adding
them?
-z are options to ld.
libgfortran/acinclude.m4 contains:
dnl Check whether -Wl,--as-needed resp. -Wl,-zignore is supported
That's where -z could come from. – Could you check what is in the
installed file libgfortran.spec? On my Linux, it has:
%{static-libgfortran:--as-needed}
If you have a '-z ignore' there, it would be interesting to understand
why – and what 'libgfortran/config.log' shows.
Which linker do you use, on which OS?
binutils 2.32 (stock build from source) on CentOS 7.
Here: Ubuntu with binutils 2.34.
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
Re: static libgfortran linker warnings
On Wed, Apr 21, 2021 at 6:06 AM Bernhard Reutner-Fischer
wrote:
>
> On Wed, 21 Apr 2021 05:29:28 -0400
> NightStrike wrote:
>
> > On Wed, Apr 21, 2021 at 4:59 AM Bernhard Reutner-Fischer
> > wrote:
> > > On Wed, 21 Apr 2021 02:34:43 -0400
> > > NightStrike via Fortran wrote:
> > >
> > > > When linking with -static-libgfortran, I get warnings from ld of the
> > > > form
> > > > "ld: warning: -z ignore ignored" and "ld: warning: -z record ignored". I
> > > > can't find those -z options documented anywhere. Why is gfortran adding
> > > > them?
> > >
> > > -z are options to ld.
> > > Which linker do you use, on which OS?
> >
> > binutils 2.32 (stock build from source) on CentOS 7.
> >
> > > If you use binutils, you can pass -Wl,--verbose to the compiler to
> > > instruct the linker to dump the linker script while linking. This should
> > > show where the -z comes from:
> > > gfortran foo.f90 -static-libgfortran -Wl,--verbose
> >
> > I didn't do this per se, but I did gfortran -v, and saw that it was an
> > option to collect2. I can try your method when the system is
> > available again tomorrow.
>
> gcc/configure.ac suggests that -z ignore / -z record is the native
> solaris ld parlance for --as-needed / --no-as-needed
> See "AC_CACHE_CHECK(linker --as-needed support".
>
> No idea why your gcc build thought that the ignore/record tuple is the
> correct thing to use for as-needed/no-as-needed. Maybe check your
> config.log around the '--as-needed support' checks for clues.
>
> I wouldn't suggest to hand edit your libgfortran.spec to use
> *lib: %{static-libgfortran:--as-needed} -lquadmath
> %{static-libgfortran:--no-as-needed} -lm %(libgcc) %(liborig)
> but i guess that would silence your linker warnings, too.
> HTH,
FWIW:
./ld: supported targets: elf64-x86-64 elf32-i386 elf32-iamcu
elf32-x86-64 pei-i386 pei-x86-64 elf64-l1om elf64-k1om elf64-little
elf64-big elf32-little elf32-big plugin srec symbolsrec verilog tekhex
binary ihex
./ld: supported emulations: elf_x86_64 elf32_x86_64 elf_i386 elf_iamcu
elf_l1om elf_k1om
./ld: emulation specific options:
Re: static libgfortran linker warnings
On Wed, Apr 21, 2021 at 6:17 AM Tobias Burnus wrote:
>
>
> >> On Wed, 21 Apr 2021 02:34:43 -0400
> >> NightStrike via Fortran wrote:
> >>> When linking with -static-libgfortran, I get warnings from ld of the form
> >>> "ld: warning: -z ignore ignored" and "ld: warning: -z record ignored". I
> >>> can't find those -z options documented anywhere. Why is gfortran adding
> >>> them?
> >> -z are options to ld.
>
> libgfortran/acinclude.m4 contains:
>
> dnl Check whether -Wl,--as-needed resp. -Wl,-zignore is supported
>
> That's where -z could come from. – Could you check what is in the
> installed file libgfortran.spec? On my Linux, it has:
>
> %{static-libgfortran:--as-needed}
>
> If you have a '-z ignore' there, it would be interesting to understand
> why – and what 'libgfortran/config.log' shows.
>
> >> Which linker do you use, on which OS?
> > binutils 2.32 (stock build from source) on CentOS 7.
>
> Here: Ubuntu with binutils 2.34.
%rename lib liborig
*lib: %{static-libgfortran:-zignore} -lquadmath
%{static-libgfortran:-zrecord} -lm %(libgcc) %(liborig)
Clearly, I built it wrong. I just would like to know how I did so.
